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 741EC429C2 for ; Sun, 10 Apr 2022 14:14:24 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 84C6A68B1FB; Sun, 10 Apr 2022 17:14:21 +0300 (EEST) Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6BDF968AF09 for ; Sun, 10 Apr 2022 17:14:15 +0300 (EEST) Received: from localhost (213-47-68-29.cable.dynamic.surfer.at [213.47.68.29]) (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id 7F7A61BF204 for ; Sun, 10 Apr 2022 14:14:14 +0000 (UTC) Date: Sun, 10 Apr 2022 16:14:13 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220410141413.GC2829255@pb2> References: <20220329212459.23440-1-michael@niedermayer.cc> <62c40964-d827-d044-cec3-233bfc04aaf1@gmail.com> <20220330105802.GO2829255@pb2> MIME-Version: 1.0 In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] 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-Type: multipart/mixed; boundary="===============2128020703256681243==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============2128020703256681243== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="6+zbkgZlzZNNdywa" Content-Disposition: inline --6+zbkgZlzZNNdywa Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Apr 09, 2022 at 08:56:05PM +0200, Marton Balint wrote: >=20 >=20 > On Wed, 30 Mar 2022, Michael Niedermayer wrote: >=20 > > 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 > > > >=20 > > > > 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(+) > > > >=20 > > > > 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, AV= Packet *pkt) > > > > } > > > > } > > > > + av_packet_unref(s->in_pkt); > > >=20 > > > This looks to me like it revealed a bug in the code above, which is m= eant to > > > ensure s->in_pkt will be blank at this point. It should be fixed there > > > instead. > >=20 > > IIRC the problem was a input packet with size 0 > > the code seems to assume 0 meaning no packet >=20 > 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. where exactly is this written ? >=20 > 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 produc= es > the 0 sized packets should be fixed. There is no encoder or demuxer. There is just the fuzzer which excercies the whole space of allowed parameters of the BSFs and either such zero packets are valid or they are not. if not, then a check could be added to av_bsf_send_packet() that feels a bit broad though. i can add a check to pcm_rechunk_bsf but it feels a bit odd if these are valid and just not supposed to come out of the encoders do you see some problem with these packets ?=20 that makes it better to just reject them ? (error you enountered a packet which makes no difference seems a bit odd in its own too. That probably should only be a warning) =20 thx diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c index 42cc1b5ab0..ae16112285 100644 --- a/libavcodec/bsf.c +++ b/libavcodec/bsf.c @@ -212,6 +212,11 @@ int av_bsf_send_packet(AVBSFContext *ctx, AVPacket *pk= t) return 0; } =20 + if (pkt->size =3D=3D 0 && pkt->side_data_elems =3D=3D 0) { + av_log(ctx, AV_LOG_ERROR, "Zero packet is not allowed.\n"); + return AVERROR(EINVAL); + } + if (bsfi->eof) { av_log(ctx, AV_LOG_ERROR, "A non-NULL packet sent after an EOF.\n"= ); return AVERROR(EINVAL); [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data --6+zbkgZlzZNNdywa Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYlLmMgAKCRBhHseHBAsP q5p9AJoDST1/11bnChpheDJhH3jzWJE9UACeJ8hfmwASjc0VLJso0BO9yeQoxEI= =XhF7 -----END PGP SIGNATURE----- --6+zbkgZlzZNNdywa-- --===============2128020703256681243== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============2128020703256681243==--