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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux