Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1689416

Re: [PATCH] staging: gs_fpgaboot: add buffer overflow checks

From Dan Carpenter <dan.carpenter@oracle.com>
Newsgroups linux.kernel
Subject Re: [PATCH] staging: gs_fpgaboot: add buffer overflow checks
Date 2017-07-17 22:00 +0200
Message-ID <u4klr-SQ-11@gated-at.bofh.it> (permalink)
References <u42eR-6dG-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Sun, Jul 16, 2017 at 08:38:41PM -0400, Jacob von Chorus wrote:
> diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
> index 19b550fff0..2aafd769b8 100644
> --- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
> +++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
> @@ -47,7 +47,7 @@ static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
>  	*offset += rdsize;
>  }
>  
> -static void readinfo_bitstream(char *bitdata, char *buf, int *offset)
> +static int readinfo_bitstream(char *bitdata, char *buf, int n, int *offset)

Choose a better name than "n" like "size".

>  {
>  	char tbuf[64];
>  	s32 len;
> @@ -59,9 +59,15 @@ static void readinfo_bitstream(char *bitdata, char *buf, int *offset)
>  	read_bitstream(bitdata, tbuf, offset, 2);
>  
>  	len = tbuf[0] << 8 | tbuf[1];

Since tbuf is char then, on x86 and arm, that means it's signed and it
means "len" can be negative.  Declare tbuf as u8.  Which will require
other changes as well...

> +	if (len + 1 > n) {

It's more idiomatic to say "if (len >= n)".  Plus that's a good habbit
if you want to avoid integer overflows.

> +		pr_err("error: readinfo buffer too small\n");
> +		return -1;

-1 is not a correct error code.

regards,
dan carpenter

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] staging: gs_fpgaboot: add buffer overflow checks Jacob von Chorus <jacobvonchorus@cwphoto.ca> - 2017-07-17 02:40 +0200
  Re: [PATCH] staging: gs_fpgaboot: add buffer overflow checks Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-17 14:30 +0200
  Re: [PATCH] staging: gs_fpgaboot: add buffer overflow checks Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-17 22:00 +0200
    Re: [PATCH] staging: gs_fpgaboot: add buffer overflow checks Jacob von Chorus <jacobvonchorus@cwphoto.ca> - 2017-07-18 02:30 +0200
      Re: [PATCH] staging: gs_fpgaboot: add buffer overflow checks Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-18 10:00 +0200

csiph-web