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.cadhal...@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
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
