From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id A65BC45ED4 for ; Mon, 17 Apr 2023 17:14:45 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5847068BE4E; Mon, 17 Apr 2023 20:14:43 +0300 (EEST) Received: from iq.passwd.hu (unknown [217.27.212.140]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EFD56689FE5 for ; Mon, 17 Apr 2023 20:14:36 +0300 (EEST) Received: from localhost (localhost [127.0.0.1]) by iq.passwd.hu (Postfix) with ESMTP id 2DEBEE8A94 for ; Mon, 17 Apr 2023 19:13:56 +0200 (CEST) X-Virus-Scanned: amavisd-new at passwd.hu Received: from iq.passwd.hu ([127.0.0.1]) by localhost (iq.passwd.hu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id h-ZZnY1_Nibq for ; Mon, 17 Apr 2023 19:13:52 +0200 (CEST) Received: from iq (iq [217.27.212.140]) by iq.passwd.hu (Postfix) with ESMTPS id 49CA1E89E9 for ; Mon, 17 Apr 2023 19:13:52 +0200 (CEST) Date: Mon, 17 Apr 2023 19:13:52 +0200 (CEST) From: Marton Balint To: FFmpeg development discussions and patches In-Reply-To: <20230417113434.GJ275832@pb2> Message-ID: <15b07f-8522-7dea-a980-3f4b62f4d6e3@passwd.hu> References: <20230416222518.21308-1-michael@niedermayer.cc> <9c9cb310-7c50-a400-4165-51468f5a9af5@gmail.com> <20230417112623.GI275832@pb2> <4ed9d14d-70f7-0f52-e87e-7c6815e75194@gmail.com> <20230417113434.GJ275832@pb2> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 1/5] avcodec/pcm_rechunk_bsf: unref packet before putting a new one in X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Mon, 17 Apr 2023, Michael Niedermayer wrote: > On Mon, Apr 17, 2023 at 08:28:37AM -0300, James Almer wrote: >> On 4/17/2023 8:26 AM, Michael Niedermayer wrote: >>> On Sun, Apr 16, 2023 at 07:57:00PM -0300, James Almer wrote: >>>> On 4/16/2023 7:25 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 >>>>> --- >>>>> 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 108d9e90b99..3f43934fe9a 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 could be pointing to a bug in the above code, and unreffing here like >>>> this would result in data loss. >>>> >>>> Does the following change also fix the memleak? >>>> >>>>> diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c >>>>> index 108d9e90b9..032f914916 100644 >>>>> --- a/libavcodec/pcm_rechunk_bsf.c >>>>> +++ b/libavcodec/pcm_rechunk_bsf.c >>>>> @@ -151,7 +151,8 @@ static int rechunk_filter(AVBSFContext *ctx, AVPacket *pkt) >>>>> av_packet_move_ref(pkt, s->in_pkt); >>>>> return send_packet(s, nb_samples, pkt); >>>>> } >>>>> - } >>>>> + } else >>>>> + av_packet_unref(s->in_pkt); >>>>> >>>>> ret = ff_bsf_get_packet_ref(ctx, s->in_pkt); >>>>> if (ret == AVERROR_EOF && s->out_pkt->size) { >>>> >>>> If it does then a zero sized packet with either only side data or that went >>>> through the av_packet_make_refcounted() in av_bsf_send_packet() that left it >>>> with an allocated padding buffer was fed to this bsf. >>> >>> yes this works too >>> and this memleak is open since a year, its the 2nd time i submited this >> >> Yes, i had a sense of deja vu. >> >>> patch, last time the discussions didnt lead to any consensus on what to do >>> I hope this time some fix from someone ends up in git >>> >>> thx >> >> Just apply the suggested change i made above. > > ok That is fine by me as well to fix the leak. However I still wonder if it is valid to pass empty packets around. AFAIK the only documented case is when some final side data is passed at the end of the encoding, and these fuzzing issues are typically not that, but that some demuxer generates 0-sized packets for corrupt files. Regards, Marton _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".