linux-kernel - Re: [PATCH 1/2] media: coda: jpeg: support optimized huffman tables

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]

Date:   Tue, 24 Mar 2020 10:11:25 +0100
From:   Philipp Zabel <pza@...gutronix.de>
To:     Adrian Ratiu <adrian.ratiu@...labora.com>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Ezequiel Garcia <ezequiel@...labora.com>, kernel@...labora.com,
        kernel@...gutronix.de, Tim Harvey <tharvey@...eworks.com>
Subject: Re: [PATCH 1/2] media: coda: jpeg: support optimized huffman tables

Hi Adrian,

On Mon, Mar 23, 2020 at 03:09:36PM +0200, Adrian Ratiu wrote:
> Each jpeg can have the huffman tables optimized for its specific content
> meaning that the table lenghts and values don't match the standard table
> of substitutions so there's no reason to hardcode and expect the sandard
> lenghts, otherwise we just end up rejecting optimized jpegs altogether.

Thank you, that is a great improvement. There's one issue remaining
below:

> Tested on CODA960.
> 
> Signed-off-by: Adrian Ratiu <adrian.ratiu@...labora.com>
> ---
>  drivers/media/platform/coda/coda-jpeg.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/coda/coda-jpeg.c b/drivers/media/platform/coda/coda-jpeg.c
> index 6a11b64efb6b..162ba28a6b95 100644
> --- a/drivers/media/platform/coda/coda-jpeg.c
> +++ b/drivers/media/platform/coda/coda-jpeg.c
> @@ -343,7 +343,8 @@ int coda_jpeg_decode_header(struct coda_ctx *ctx, struct vb2_buffer *vb)
>  			v4l2_err(&dev->v4l2_dev, "missing Huffman table\n");
>  			return -EINVAL;
>  		}
> -		if (huffman_tables[i].length != ((i & 2) ? 178 : 28)) {
> +		if (huffman_tables[i].length < 17 ||
> +		    huffman_tables[i].length > 178) {

The maximum length of the DC tables is 16 + 12, so this should
still be checked for a maximum length of 28 if (i & 2) == 0.

>  			v4l2_err(&dev->v4l2_dev,
>  				 "invalid Huffman table %d length: %zu\n",
>  				 i, huffman_tables[i].length);
> @@ -357,10 +358,12 @@ int coda_jpeg_decode_header(struct coda_ctx *ctx, struct vb2_buffer *vb)
>  			return -ENOMEM;
>  		ctx->params.jpeg_huff_tab = huff_tab;
>  	}
> -	memcpy(huff_tab->luma_dc, huffman_tables[0].start, 16 + 12);
> -	memcpy(huff_tab->chroma_dc, huffman_tables[1].start, 16 + 12);
> -	memcpy(huff_tab->luma_ac, huffman_tables[2].start, 16 + 162);
> -	memcpy(huff_tab->chroma_ac, huffman_tables[3].start, 16 + 162);
> +
> +	memset(huff_tab, 0, sizeof(*huff_tab));
> +	memcpy(huff_tab->luma_dc, huffman_tables[0].start, huffman_tables[0].length);
> +	memcpy(huff_tab->chroma_dc, huffman_tables[1].start, huffman_tables[1].length);

Otherwise these two might overflow the luma_dc and chroma_dc arrays.

regards
Philipp