Re: [PATCH v2 2/2] staging: gs_fpgaboot: change char to u8

[Joe Perches]

On Mon, 2017-07-17 at 20:47 -0400, Jacob von Chorus wrote:
> The bitstream storage variables were changed from char to u8 arrays to
> prevent issues such as negative lengths. This change makes the code
> compatible with the "data" field in "struct firmware" which is of type
> u8.
> 
> Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
> ---
>  drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 24 ++++++++++++------------
>  drivers/staging/gs_fpgaboot/gs_fpgaboot.h |  2 +-
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c 
> b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
> index 008ef99f05..467a0ad81f 100644
> --- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
> +++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
> @@ -41,16 +41,16 @@ static char       *file = "xlinx_fpga_firmware.bit";
>  module_param(file, charp, 0444);
>  MODULE_PARM_DESC(file, "Xilinx FPGA firmware file.");
>  
> -static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
> +static void read_bitstream(u8 *bitdata, u8 *buf, int *offset, int rdsize)
>  {
>       memcpy(buf, bitdata + *offset, rdsize);
>       *offset += rdsize;
>  }
>  
> -static int readinfo_bitstream(char *bitdata, char *buf, int size, int 
> *offset)
> +static int readinfo_bitstream(u8 *bitdata, u8 *buf, int size, int *offset)
>  {
> -     char tbuf[64];
> -     s32 len;
> +     u8 tbuf[64];
> +     u16 len;
>  
>       /* read section char */
>       read_bitstream(bitdata, tbuf, offset, 1);

read_bitstream takes an int rdsize, not a u16.
and this function will overflow tbuf if len > 64

static void readinfo_bitstream(char *bitdata, char *buf, int *offset)
{
        char tbuf[64];
        s32 len;

        /* read section char */
        read_bitstream(bitdata, tbuf, offset, 1);

        /* read length */
        read_bitstream(bitdata, tbuf, offset, 2);

        len = tbuf[0] << 8 | tbuf[1];

        read_bitstream(bitdata, buf, offset, len);
        buf[len] = '\0';
}

len is up to 64k but tbuf is 64 bytes.

        len = get_unaligned_le16(tbuf)

might be nicer than

        len = tbuf[0] << 8 | tbuf[1];

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel