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

[Andreas Cadhalpun]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-pnmdec-make-sure-v-is-capped-by-maxval.patch
Type: text/x-diff
Size: 1856 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161110/2b470d91/attachment.patch>