[FFmpeg-devel] [PATCH] avcodec: Avoid splitting side data repeatedly

[Michael Niedermayer]

On Thu, May 04, 2017 at 12:41:49PM -0300, James Almer wrote:
> On 5/4/2017 12:15 PM, Michael Niedermayer wrote:
> > Fixes Timeout
> > Fixes: 508/clusterfuzz-testcase-6245747678773248
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/avpacket.c | 24 ++++++++++++++++++++++++
> >  libavcodec/decode.c   |  9 +++++++--
> >  libavcodec/internal.h |  4 ++++
> >  3 files changed, 35 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> > index d97400eaef..de12a1f1ce 100644
> > --- a/libavcodec/avpacket.c
> > +++ b/libavcodec/avpacket.c
> > @@ -463,6 +463,30 @@ int av_packet_split_side_data(AVPacket *pkt){
> >      return 0;
> >  }
> >  
> > +int ff_packet_split_and_drop_side_data(AVPacket *pkt){
> > +    if (!pkt->side_data_elems && pkt->size >12 && AV_RB64(pkt->data + pkt->size - 8) == FF_MERGE_MARKER){
> > +        int i;
> > +        unsigned int size;
> > +        uint8_t *p;
> > +
> > +        p = pkt->data + pkt->size - 8 - 5;
> > +        for (i=1; ; i++){
> > +            size = AV_RB32(p);
> > +            if (size>INT_MAX - 5 || p - pkt->data < size)
> > +                return 0;
> > +            if (p[4]&128)
> > +                break;
> > +            if (p - pkt->data < size + 5)
> > +                return 0;
> > +            p-= size+5;
> > +        }
> > +        pkt->size = p - pkt->data - size;
> > +        av_assert0(pkt->size >= 0);
> > +        return 1;
> > +    }
> > +    return 0;
> > +}
> > +
> >  #endif
> >  
> >  uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size)
> > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > index 708071fd07..43ba04550a 100644
> > --- a/libavcodec/decode.c
> > +++ b/libavcodec/decode.c
> > @@ -392,7 +392,9 @@ static int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame)
> >      tmp = *pkt;
> >  #if FF_API_MERGE_SD
> >  FF_DISABLE_DEPRECATION_WARNINGS
> > -    did_split = av_packet_split_side_data(&tmp);
> > +    did_split = avci->compat_decode_partial_size ?
> > +                ff_packet_split_and_drop_side_data(&tmp) :
> > +                av_packet_split_side_data(&tmp);
> 
> Eh, this is all going away as soon as we bump major and will not make it
> to any upcoming release, so guess it's fine for now.
> 
> >  
> >      if (did_split) {
> >          ret = extract_packet_props(avctx->internal, &tmp);
> > @@ -961,6 +963,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
> >                               AVPacket *avpkt)
> >  {
> >      int i, ret = 0;
> > +    AVCodecInternal *avci = avctx->internal;
> >  
> >      if (!avpkt->data && avpkt->size) {
> >          av_log(avctx, AV_LOG_ERROR, "invalid packet: NULL data, size != 0\n");
> > @@ -981,7 +984,9 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
> >          AVPacket tmp = *avpkt;
> >  #if FF_API_MERGE_SD
> >  FF_DISABLE_DEPRECATION_WARNINGS
> > -        int did_split = av_packet_split_side_data(&tmp);
> > +        int did_split = avci->compat_decode_partial_size ?
> > +                        ff_packet_split_and_drop_side_data(&tmp) :
> > +                        av_packet_split_side_data(&tmp);
> >          //apply_param_change(avctx, &tmp);
> >  
> >          if (did_split) {
> > diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> > index e30d4aa73d..f3d10a14f1 100644
> > --- a/libavcodec/internal.h
> > +++ b/libavcodec/internal.h
> > @@ -356,6 +356,10 @@ int ff_set_sar(AVCodecContext *avctx, AVRational sar);
> >  int ff_side_data_update_matrix_encoding(AVFrame *frame,
> >                                          enum AVMatrixEncoding matrix_encoding);
> >  
> > +#if FF_API_MERGE_SD_API
> > +int ff_packet_split_and_drop_side_data(AVPacket *pkt);
> > +#endif
> 
> This function will not be used by anything after the ABI part of this
> functionality is dropped, so maybe make it depend on FF_API_MERGE_SD
> instead, here and in avpacket.c

ok, will apply with these changes

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued
-------------- 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/20170506/39cb2356/attachment.sig>