[FFmpeg-devel] [PATCH] pnmdec: make sure v is capped by maxval

Michael Niedermayer michael at niedermayer.cc
Fri Nov 11 03:03:20 EET 2016


On Thu, Nov 10, 2016 at 08:52:29PM +0100, Andreas Cadhalpun wrote:
> On 10.11.2016 02:26, Michael Niedermayer wrote:
> > On Wed, Nov 09, 2016 at 10:46:03PM +0100, Andreas Cadhalpun wrote:
> >>  pnmdec.c |    4 ++++
> >>  1 file changed, 4 insertions(+)
> >> a970cb981be02ea692d0bf2e68976077f14f2de3  0001-pnmdec-make-sure-v-is-capped-by-maxval.patch
> >> From f315a3cfe35377a2638dc2294200a288408dc784 Mon Sep 17 00:00:00 2001
> >> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> Date: Wed, 9 Nov 2016 01:09:35 +0100
> >> Subject: [PATCH] pnmdec: make sure v is capped by maxval
> >>
> >> Otherwise put_bits can be called with a value that doesn't fit in the
> >> sample_len, causing an assertion failure.
> >> ---
> >>  libavcodec/pnmdec.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c
> >> index ca97cc3..0f6a895 100644
> >> --- a/libavcodec/pnmdec.c
> >> +++ b/libavcodec/pnmdec.c
> >> @@ -144,6 +144,10 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data,
> >>                      } else {
> >>                          /* read a sequence of digits */
> >>                          do {
> >> +                            if (v > (INT_MAX - c) / 10 || 10 * v + c > s->maxval) {
> >> +                                av_log(avctx, AV_LOG_ERROR, "value 10 * %d + %d larger than maxval %d\n", v, c, s->maxval);
> >> +                                return AVERROR_INVALIDDATA;
> >> +                            }
> > 
> > this test should nt be inside the loop
> > you can try to benchmark with START/STOP_TIMER to see what effect
> > this has (i didnt try but i expect it to be bad), this is the
> > innermost loop of the decoder
> > 
> > you can just unroll the loop by 5, thats the max number of iterations
> > i think, which avoids the overflow
> > it also should make the code faster
> 
> Done, new patch attached.
> 
> > iam reading in man ppm and others:
> > "The maximum color value (Maxval), again in ASCII decimal.  Must be less than 65536"
> 
> Indeed, I'll send a separate match limiting maxval.
> 
> Best regards,
> Andreas
> 

>  pnmdec.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 0dd61abddc422dd2ac37356f8271822d7e801b8e  0001-pnmdec-make-sure-v-is-capped-by-maxval.patch
> From 2d402d8a38223b8c3c58d3e71e734932c9bdadae Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Wed, 9 Nov 2016 01:09:35 +0100
> Subject: [PATCH] pnmdec: make sure v is capped by maxval
> 
> Otherwise put_bits can be called with a value that doesn't fit in the
> sample_len, causing an assertion failure.

LGTM

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- 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/20161111/35e24650/attachment.sig>


More information about the ffmpeg-devel mailing list