linux-kernel - Re: [PATCH v2 2/2] staging: gs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1500340928.25934.18.camel@perches.com>
Date:   Mon, 17 Jul 2017 18:22:08 -0700
From:   Joe Perches <joe@...ches.com>
To:     Jacob von Chorus <jacobvonchorus@...hoto.ca>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Insop Song <insop.song@...nspeed.com>
Cc:     devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        dan.carpenter@...cle.com
Subject: Re: [PATCH v2 2/2] staging: gs_fpgaboot: change char to u8

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@...hoto.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];