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

[Dan Carpenter]

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