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@xxxxxxxxxxxxx>
> ---
> 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
