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

From: Joe Perches
Date: Mon Jul 17 2017 - 21:22:18 EST


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 <jacobvonchorus@xxxxxxxxxx>
> ---
> 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];