[FFmpeg-devel] [PATCH] libavcodec/h264dec: avoid arithmetic on null pointers

James Almer jamrial at gmail.com
Thu Mar 2 13:37:25 EET 2023


On 3/2/2023 8:33 AM, James Almer wrote:
> On 3/2/2023 6:05 AM, Anton Khirnov wrote:
>> Quoting Jeremy Dorfman (2023-03-01 19:50:08)
>>> null pointer arithmetic is undefined behavior in C.
>>> ---
>>>   libavcodec/h264dec.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
>>> index 2d691731c5..ef698f2630 100644
>>> --- a/libavcodec/h264dec.c
>>> +++ b/libavcodec/h264dec.c
>>> @@ -912,8 +912,8 @@ static int finalize_frame(H264Context *h, AVFrame 
>>> *dst, H264Picture *out, int *g
>>>               av_log(h->avctx, AV_LOG_DEBUG, "Duplicating field %d to 
>>> fill missing\n", field);
>>>               for (p = 0; p<4; p++) {
>>> -                dst_data[p] = f->data[p] + (field^1)*f->linesize[p];
>>> -                src_data[p] = f->data[p] +  field   *f->linesize[p];
>>> +                dst_data[p] = f->data[p] ? f->data[p] + 
>>> (field^1)*f->linesize[p] : NULL;
>>> +                src_data[p] = f->data[p] ? f->data[p] +  field   
>>> *f->linesize[p] : NULL;
>>
>> Why would that be NULL? Seems like something that should not happen.
> 
> None of the supported pixel formats in this decoder use four planes, so 
> at least the last one will always be NULL. FF_PTR_ADD() is what we did 
> in similar situations, like in sws_receive_slice(), when we don't use 
> some helper to get the exact number of used planes from the pixfmt 
> descriptor.

http://coverage.ffmpeg.org/index.h264dec.c.8820c603e94612cd02689417231bc605.html#l912

The ubsan fate instance would have detected this long ago if we had a 
sample that covers this path.
Do you happen to have one you can make public to be added to the FATE 
suite, Jeremy? Or was this problem found using some static analyzer?


More information about the ffmpeg-devel mailing list