[FFmpeg-devel] [PATCH] mpegpicture: use coded_width/coded_height to allocate frame

Michael Niedermayer michael at niedermayer.cc
Thu Nov 24 18:45:38 EET 2016


On Thu, Nov 24, 2016 at 02:14:59AM +0100, Andreas Cadhalpun wrote:
> On 23.11.2016 15:01, Michael Niedermayer wrote:
> > On Mon, Nov 07, 2016 at 10:32:29PM +0100, Andreas Cadhalpun wrote:
> >> This fixes a heap-buffer-overflow in ff_er_frame_end when decoding mss2 with
> >> coded_width/coded_height larger than width/height.
> >>
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>  libavcodec/mpegpicture.c | 12 ++++++------
> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
> >> index 6748fc2..70b4d3c 100644
> >> --- a/libavcodec/mpegpicture.c
> >> +++ b/libavcodec/mpegpicture.c
> >> @@ -108,15 +108,15 @@ static int alloc_frame_buffer(AVCodecContext *avctx,  Picture *pic,
> >>          avctx->codec_id != AV_CODEC_ID_VC1IMAGE  &&
> >>          avctx->codec_id != AV_CODEC_ID_MSS2) {
> >>          if (edges_needed) {
> >> -            pic->f->width  = avctx->width  + 2 * EDGE_WIDTH;
> >> -            pic->f->height = avctx->height + 2 * EDGE_WIDTH;
> >> +            pic->f->width  = avctx->coded_width  + 2 * EDGE_WIDTH;
> >> +            pic->f->height = avctx->coded_height + 2 * EDGE_WIDTH;
> >>          }
> >>  
> >>          r = ff_thread_get_buffer(avctx, &pic->tf,
> >>                                   pic->reference ? AV_GET_BUFFER_FLAG_REF : 0);
> >>      } else {
> >> -        pic->f->width  = avctx->width;
> >> -        pic->f->height = avctx->height;
> >> +        pic->f->width  = avctx->coded_width;
> >> +        pic->f->height = avctx->coded_height;
> >>          pic->f->format = avctx->pix_fmt;
> >>          r = avcodec_default_get_buffer2(avctx, pic->f, 0);
> >>      }
> >> @@ -135,8 +135,8 @@ static int alloc_frame_buffer(AVCodecContext *avctx,  Picture *pic,
> >>                           (EDGE_WIDTH >> (i ? chroma_x_shift : 0));
> >>              pic->f->data[i] += offset;
> >>          }
> >> -        pic->f->width  = avctx->width;
> >> -        pic->f->height = avctx->height;
> >> +        pic->f->width  = avctx->coded_width;
> >> +        pic->f->height = avctx->coded_height;
> >>      }
> > 
> > why would the error concealment code need a differently sized output
> > than the decoder itself ?
> 
> This code is rather convoluted, so it's not quite obvious, however the
> needed buffer size for the error concealment is determined by the
> MpegEncContext.width/height, which is set via:
> mss2_decode_init
> |-> wmv9_init
>     |-> ff_msmpeg4_decode_init
>         |-> ff_h263_decode_init
>             |-> ff_mpv_decode_init:
>     s->width           = avctx->coded_width;
>     s->height          = avctx->coded_height;
> 
> Thus the buffer needs the same size.

Is it correct that your cases uses
decode_wmv9() -> vc1_decode_i_blocks() ?
these decode a rectangele up to end_mb_y, end_mb_x
does this mismatch with what later code accesses ?

would using end_mb_* in the EC code fix this ? (or disabling EC if
they mismatch)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri
-------------- 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/20161124/ad7d1509/attachment.sig>


More information about the ffmpeg-devel mailing list