[FFmpeg-devel] [PATCH] mjpegdec: consider chroma subsampling in size check

Michael Niedermayer michaelni at gmx.at
Sun Dec 6 22:18:39 CET 2015


On Sun, Dec 06, 2015 at 06:56:35PM +0100, Andreas Cadhalpun wrote:
> On 05.12.2015 04:02, Michael Niedermayer wrote:
> > On Fri, Dec 04, 2015 at 03:14:21PM +0100, Andreas Cadhalpun wrote:
> >> On 03.12.2015 15:48, Michael Niedermayer wrote:
> >>> On Wed, Dec 02, 2015 at 10:00:13PM +0100, Andreas Cadhalpun wrote:
> >>>> @@ -1293,14 +1296,16 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah,
> >>>>                  v = s->v_scount[i];
> >>>>                  x = 0;
> >>>>                  y = 0;
> >>>> +                h_shift = c ? chroma_h_shift: 0;
> >>>> +                v_shift = c ? chroma_v_shift: 0;
> >>>>                  for (j = 0; j < n; j++) {
> >>>>                      block_offset = (((linesize[c] * (v * mb_y + y) * 8) +
> >>>>                                       (h * mb_x + x) * 8 * bytes_per_pixel) >> s->avctx->lowres);
> >>>>  
> >>>>                      if (s->interlaced && s->bottom_field)
> >>>>                          block_offset += linesize[c] >> 1;
> >>>> -                    if (   8*(h * mb_x + x) < s->width
> >>>> -                        && 8*(v * mb_y + y) < s->height) {
> >>>> +                    if (   8*(h * mb_x + x) < (s->width  + (1 << h_shift) - 1) >> h_shift
> >>>> +                        && 8*(v * mb_y + y) < (s->height + (1 << v_shift) - 1) >> v_shift) {
> >>>
> >>> please move the w/h computation out of the block loop
> >>> it stays the same for a component and does not need to be
> >>> recalculated
> >>> theres a loop above that fills data[] that can probably be used to
> >>> fill w/h arrays
> >>
> >> OK, but since there are only two possible values, I don't think filling
> >> arrays is necessary. Attached is an updated patch.
> > 
> > couldnt there be a alpha plane too ?
> 
> Yes, fixed patch attached.
> I also tested filling w/h arrays, but that turned out to be slightly slower.
> 
> Best regards,
> Andreas
> 

>  mjpegdec.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> a294ce9a780fdd710d3661bc201b0c72d30786d3  0001-mjpegdec-consider-chroma-subsampling-in-size-check.patch
> From 7788195340e1d0e1206660f12f003f952da750a6 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Wed, 2 Dec 2015 21:52:23 +0100
> Subject: [PATCH] mjpegdec: consider chroma subsampling in size check
> 
> If the chroma components are subsampled, smaller buffers are allocated
> for them. In that case the maximal block_offset for the chroma
> components is not as large as for the luma component.
> 
> This fixes out of bounds writes causing segmentation faults or memory
> corruption.
> 

LGTM

thx


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151206/74f420c2/attachment.sig>


More information about the ffmpeg-devel mailing list