[FFmpeg-devel] [PATCH 1/2] avcodec/h264: keep SPS and PPS bitstream data

Michael Niedermayer michaelni at gmx.at
Thu Oct 1 19:56:41 CEST 2015


On Thu, Oct 01, 2015 at 07:26:47PM +0200, wm4 wrote:
> On Thu, 1 Oct 2015 19:08:21 +0200
> Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > On Thu, Oct 01, 2015 at 06:13:20PM +0200, wm4 wrote:
> > > Needed for the following VideotoolBox commit.
> > > ---
> > >  libavcodec/h264.h    |  4 ++++
> > >  libavcodec/h264_ps.c | 20 ++++++++++++++++----
> > >  2 files changed, 20 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/libavcodec/h264.h b/libavcodec/h264.h
> > > index 7356288..eeb2aaf 100644
> > > --- a/libavcodec/h264.h
> > > +++ b/libavcodec/h264.h
> > > @@ -229,6 +229,8 @@ typedef struct SPS {
> > >      int residual_color_transform_flag;    ///< residual_colour_transform_flag
> > >      int constraint_set_flags;             ///< constraint_set[0-3]_flag
> > >      int new;                              ///< flag to keep track if the decoder context needs re-init due to changed SPS
> > > +    uint8_t data[1 << 16];
> > > +    int data_size;
> > >  } SPS;
> > >  
> > >  /**
> > > @@ -254,6 +256,8 @@ typedef struct PPS {
> > >      uint8_t scaling_matrix8[6][64];
> > >      uint8_t chroma_qp_table[2][QP_MAX_NUM+1];  ///< pre-scaled (with chroma_qp_index_offset) version of qp_table
> > >      int chroma_qp_diff;
> > > +    uint8_t data[1 << 16];
> > > +    int data_size;
> > >  } PPS;
> > >  
> > >  /**
> > > diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> > > index 52d235c..fd16a95 100644
> > > --- a/libavcodec/h264_ps.c
> > > +++ b/libavcodec/h264_ps.c
> > > @@ -307,6 +307,15 @@ int ff_h264_decode_seq_parameter_set(H264Context *h, int ignore_truncation)
> > >      int i, log2_max_frame_num_minus4;
> > >      SPS *sps;
> > >  
> > > +    sps = av_mallocz(sizeof(SPS));
> > > +    if (!sps)
> > > +        return AVERROR(ENOMEM);
> > > +
> > > +    sps->data_size = h->gb.buffer_end - h->gb.buffer;
> > 
> > the subtraction could overflow the 32bit int range leading to a
> > truncated or negative data_size
> > 
> > [...]
> > 
> 
> This is impossible. The C type used for NALs is int.

yes as long as all callers of this global function use such int based
sizes to initialize the pointers
if a single one is changed or one is added to initialize them from
a int64_t then this could overflow.


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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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/20151001/7f612285/attachment.sig>


More information about the ffmpeg-devel mailing list