[FFmpeg-devel] [PATCH] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in

[Marton Balint]

On Wed, 30 Mar 2022, Michael Niedermayer wrote:

> On Tue, Mar 29, 2022 at 06:33:06PM -0300, James Almer wrote:
>> On 3/29/2022 6:24 PM, Michael Niedermayer wrote:
>>> Fixes: memleak
>>> Fixes: 45982/clusterfuzz-testcase-minimized-ffmpeg_BSF_PCM_RECHUNK_fuzzer-5562089618407424
>>>
>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>   libavcodec/pcm_rechunk_bsf.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c
>>> index 108d9e90b9..3f43934fe9 100644
>>> --- a/libavcodec/pcm_rechunk_bsf.c
>>> +++ b/libavcodec/pcm_rechunk_bsf.c
>>> @@ -153,6 +153,7 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt)
>>>               }
>>>           }
>>> +        av_packet_unref(s->in_pkt);
>>
>> This looks to me like it revealed a bug in the code above, which is meant to
>> ensure s->in_pkt will be blank at this point. It should be fixed there
>> instead.
>
> IIRC the problem was a input packet with size 0
> the code seems to assume 0 meaning no packet

Is that valid here? The docs says that the encoders can generate 0 sized 
packets if there is side data in them. However - the PCM rechunk BSF using 
PCM packets - I am not sure this is intentional here.

So overall it looks to me that the PCM rechunk BSF should reject 0 sized 
packets with AVERROR_INVALIDDATA, and the encoder or demuxer which 
produces the 0 sized packets should be fixed.

Regards,
Marton

>
> I can check more explicitly for that after ff_bsf_get_packet_ref()
> and unref there, its more code through
>
> thx
>
>
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Its not that you shouldnt use gotos but rather that you should write
> readable code and code with gotos often but not always is less readable
>