Re: [FFmpeg-devel] [PATCH] avcodec/h264_ps: Fix copying oversized pps data

[Michael Niedermayer]

On Fri, Oct 02, 2015 at 11:04:10PM +0200, Hendrik Leppkes wrote:
> On Fri, Oct 2, 2015 at 9:49 PM, wm4  wrote:
> > On Fri, 2 Oct 2015 21:41:59 +0200
> > Michael Niedermayer  wrote:
> >
> >> On Fri, Oct 02, 2015 at 09:23:04PM +0200, wm4 wrote:
> >> > On Fri,  2 Oct 2015 21:14:57 +0200
> >> > Michael Niedermayer  wrote:
> >> >
> >> > > From: Michael Niedermayer 
> >> > >
> >> > > Fixes: https://trac.ffmpeg.org/attachment/ticket/685/movie.264
> >> > >
> >> > > In the available testcase the actual PPS only uses a few bits
> >> > > while there are 7kbyte of apparently random data after it
> >> > >
> >> > > Signed-off-by: Michael Niedermayer 
> >> > > ---
> >> > >  libavcodec/h264_ps.c |4 ++--
> >> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> >> > >
> >> > > diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> >> > > index fd16a95..0dca5f0 100644
> >> > > --- a/libavcodec/h264_ps.c
> >> > > +++ b/libavcodec/h264_ps.c
> >> > > @@ -611,8 +611,8 @@ int 
> >> > > ff_h264_decode_picture_parameter_set(H264Context *h, int bit_length)
> >> > >  return AVERROR(ENOMEM);
> >> > >  pps->data_size = h->gb.buffer_end - h->gb.buffer;
> >> > >  if (pps->data_size > sizeof(pps->data)) {
> >> > > -ret = AVERROR_INVALIDDATA;
> >> > > -goto fail;
> >> > > +av_log(h->avctx, AV_LOG_WARNING, "Truncating likely oversized 
> >> > > PPS\n");
> >> > > +pps->data_size = sizeof(pps->data);
> >> > >  }
> >> > >  memcpy(pps->data, h->gb.buffer, pps->data_size);
> >> > >  pps->sps_id = get_ue_golomb_31(>gb);
> >> >
> >> > That was quick.
> >> >
> >>
> >> > Should the same be done for SPS?
> >>
> >> i dont know
> >>
> >
> > I'd say it would be good for symmetry.
> >
> 
> Its a new error condition which wouldn't affect 99% of all users, so
> its probably best to switch it to a warning as well and hope for the
> best.

ok, switched the 2nd and applied

thanks

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

Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avcodec/h264_ps: Fix copying oversized pps data

[Michael Niedermayer]

From: Michael Niedermayer 

Fixes: https://trac.ffmpeg.org/attachment/ticket/685/movie.264

In the available testcase the actual PPS only uses a few bits
while there are 7kbyte of apparently random data after it

Signed-off-by: Michael Niedermayer 
---
 libavcodec/h264_ps.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index fd16a95..0dca5f0 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -611,8 +611,8 @@ int ff_h264_decode_picture_parameter_set(H264Context *h, 
int bit_length)
 return AVERROR(ENOMEM);
 pps->data_size = h->gb.buffer_end - h->gb.buffer;
 if (pps->data_size > sizeof(pps->data)) {
-ret = AVERROR_INVALIDDATA;
-goto fail;
+av_log(h->avctx, AV_LOG_WARNING, "Truncating likely oversized PPS\n");
+pps->data_size = sizeof(pps->data);
 }
 memcpy(pps->data, h->gb.buffer, pps->data_size);
 pps->sps_id = get_ue_golomb_31(>gb);
-- 
1.7.9.5

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_ps: Fix copying oversized pps data

[Michael Niedermayer]

On Fri, Oct 02, 2015 at 09:23:04PM +0200, wm4 wrote:
> On Fri,  2 Oct 2015 21:14:57 +0200
> Michael Niedermayer  wrote:
> 
> > From: Michael Niedermayer 
> > 
> > Fixes: https://trac.ffmpeg.org/attachment/ticket/685/movie.264
> > 
> > In the available testcase the actual PPS only uses a few bits
> > while there are 7kbyte of apparently random data after it
> > 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/h264_ps.c |4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> > index fd16a95..0dca5f0 100644
> > --- a/libavcodec/h264_ps.c
> > +++ b/libavcodec/h264_ps.c
> > @@ -611,8 +611,8 @@ int ff_h264_decode_picture_parameter_set(H264Context 
> > *h, int bit_length)
> >  return AVERROR(ENOMEM);
> >  pps->data_size = h->gb.buffer_end - h->gb.buffer;
> >  if (pps->data_size > sizeof(pps->data)) {
> > -ret = AVERROR_INVALIDDATA;
> > -goto fail;
> > +av_log(h->avctx, AV_LOG_WARNING, "Truncating likely oversized 
> > PPS\n");
> > +pps->data_size = sizeof(pps->data);
> >  }
> >  memcpy(pps->data, h->gb.buffer, pps->data_size);
> >  pps->sps_id = get_ue_golomb_31(>gb);
> 
> That was quick.
> 

> Should the same be done for SPS?

i dont know

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

What does censorship reveal? It reveals fear. -- Julian Assange


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_ps: Fix copying oversized pps data

[wm4]

On Fri,  2 Oct 2015 21:14:57 +0200
Michael Niedermayer  wrote:

> From: Michael Niedermayer 
> 
> Fixes: https://trac.ffmpeg.org/attachment/ticket/685/movie.264
> 
> In the available testcase the actual PPS only uses a few bits
> while there are 7kbyte of apparently random data after it
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/h264_ps.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> index fd16a95..0dca5f0 100644
> --- a/libavcodec/h264_ps.c
> +++ b/libavcodec/h264_ps.c
> @@ -611,8 +611,8 @@ int ff_h264_decode_picture_parameter_set(H264Context *h, 
> int bit_length)
>  return AVERROR(ENOMEM);
>  pps->data_size = h->gb.buffer_end - h->gb.buffer;
>  if (pps->data_size > sizeof(pps->data)) {
> -ret = AVERROR_INVALIDDATA;
> -goto fail;
> +av_log(h->avctx, AV_LOG_WARNING, "Truncating likely oversized 
> PPS\n");
> +pps->data_size = sizeof(pps->data);
>  }
>  memcpy(pps->data, h->gb.buffer, pps->data_size);
>  pps->sps_id = get_ue_golomb_31(>gb);

That was quick.

Should the same be done for SPS?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_ps: Fix copying oversized pps data

[wm4]

On Fri, 2 Oct 2015 21:41:59 +0200
Michael Niedermayer  wrote:

> On Fri, Oct 02, 2015 at 09:23:04PM +0200, wm4 wrote:
> > On Fri,  2 Oct 2015 21:14:57 +0200
> > Michael Niedermayer  wrote:
> > 
> > > From: Michael Niedermayer 
> > > 
> > > Fixes: https://trac.ffmpeg.org/attachment/ticket/685/movie.264
> > > 
> > > In the available testcase the actual PPS only uses a few bits
> > > while there are 7kbyte of apparently random data after it
> > > 
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/h264_ps.c |4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> > > index fd16a95..0dca5f0 100644
> > > --- a/libavcodec/h264_ps.c
> > > +++ b/libavcodec/h264_ps.c
> > > @@ -611,8 +611,8 @@ int ff_h264_decode_picture_parameter_set(H264Context 
> > > *h, int bit_length)
> > >  return AVERROR(ENOMEM);
> > >  pps->data_size = h->gb.buffer_end - h->gb.buffer;
> > >  if (pps->data_size > sizeof(pps->data)) {
> > > -ret = AVERROR_INVALIDDATA;
> > > -goto fail;
> > > +av_log(h->avctx, AV_LOG_WARNING, "Truncating likely oversized 
> > > PPS\n");
> > > +pps->data_size = sizeof(pps->data);
> > >  }
> > >  memcpy(pps->data, h->gb.buffer, pps->data_size);
> > >  pps->sps_id = get_ue_golomb_31(>gb);
> > 
> > That was quick.
> > 
> 
> > Should the same be done for SPS?
> 
> i dont know
> 

I'd say it would be good for symmetry.

Or we could just switch back to allocating 64K for the data, or maybe
even dynamically allocate it. (I wanted to avoid the latter because
it's a bit messy, but maybe it's the best solution?)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_ps: Fix copying oversized pps data

[Hendrik Leppkes]

On Fri, Oct 2, 2015 at 9:49 PM, wm4  wrote:
> On Fri, 2 Oct 2015 21:41:59 +0200
> Michael Niedermayer  wrote:
>
>> On Fri, Oct 02, 2015 at 09:23:04PM +0200, wm4 wrote:
>> > On Fri,  2 Oct 2015 21:14:57 +0200
>> > Michael Niedermayer  wrote:
>> >
>> > > From: Michael Niedermayer 
>> > >
>> > > Fixes: https://trac.ffmpeg.org/attachment/ticket/685/movie.264
>> > >
>> > > In the available testcase the actual PPS only uses a few bits
>> > > while there are 7kbyte of apparently random data after it
>> > >
>> > > Signed-off-by: Michael Niedermayer 
>> > > ---
>> > >  libavcodec/h264_ps.c |4 ++--
>> > >  1 file changed, 2 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
>> > > index fd16a95..0dca5f0 100644
>> > > --- a/libavcodec/h264_ps.c
>> > > +++ b/libavcodec/h264_ps.c
>> > > @@ -611,8 +611,8 @@ int ff_h264_decode_picture_parameter_set(H264Context 
>> > > *h, int bit_length)
>> > >  return AVERROR(ENOMEM);
>> > >  pps->data_size = h->gb.buffer_end - h->gb.buffer;
>> > >  if (pps->data_size > sizeof(pps->data)) {
>> > > -ret = AVERROR_INVALIDDATA;
>> > > -goto fail;
>> > > +av_log(h->avctx, AV_LOG_WARNING, "Truncating likely oversized 
>> > > PPS\n");
>> > > +pps->data_size = sizeof(pps->data);
>> > >  }
>> > >  memcpy(pps->data, h->gb.buffer, pps->data_size);
>> > >  pps->sps_id = get_ue_golomb_31(>gb);
>> >
>> > That was quick.
>> >
>>
>> > Should the same be done for SPS?
>>
>> i dont know
>>
>
> I'd say it would be good for symmetry.
>

Its a new error condition which wouldn't affect 99% of all users, so
its probably best to switch it to a warning as well and hope for the
best.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel