From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 78CCC44B33 for ; Thu, 17 Jul 2025 22:33:58 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 3FC5968F249; Fri, 18 Jul 2025 01:33:55 +0300 (EEST) Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id 6A4DA68ECED for ; Fri, 18 Jul 2025 01:33:48 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 4768843200 for ; Thu, 17 Jul 2025 22:33:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1752791627; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=RNLtCg6z8AOg228aeRA58g5kk6Zo5zFREKSmY1JW1eg=; b=DQWSanfY2Tm8syj0sgcNhA5wy96nr/Yq/3HVq6aAN2kIBiNOszcRl3sA/2l8l6AKR/vEQu q9JdpD5Il7ULFynp30eAWpfM1COV4Ea0r5sag9I4mTzb3cijoCod7omvvscn/DNseVdyxN otNW8MJdaYzq3ZytUeK/ft2I5kD5BNq1Jv4O38ndpc4Unr/3uEMVE6JejL/ORBbFd4niIK 7wA9wR2TBPEiCvgde9mrcbEVnNpiv4G3gFujckQBt/PnGQtxcFHN/ldSEoXppGuwX1c8s+ lh9gCBgAZi6vGUphMmpiJrxAreqn+zIw1rKv/JKcoCeufZ/Id/ID8W4c3k6NNQ== Date: Fri, 18 Jul 2025 00:33:46 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20250717223346.GW29660@pb2> References: <20250713011030.1156550-1-michael@niedermayer.cc> <20250714192133.GL29660@pb2> <71ce56e6-5d83-41f7-9872-e81e0a342cf9@rothenpieler.org> <20250714220140.GP29660@pb2> <067d6931-29bd-4be3-9e87-336383897e65@rothenpieler.org> MIME-Version: 1.0 In-Reply-To: <067d6931-29bd-4be3-9e87-336383897e65@rothenpieler.org> X-GND-State: clean X-GND-Score: -90 X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdefgdeiudejlecutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfitefpfffkpdcuggftfghnshhusghstghrihgsvgenuceurghilhhouhhtmecufedtudenucesvcftvggtihhpihgvnhhtshculddquddttddmnegfrhhlucfvnfffucdluddtmdenucfjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomhepofhitghhrggvlhcupfhivgguvghrmhgrhigvrhcuoehmihgthhgrvghlsehnihgvuggvrhhmrgihvghrrdgttgeqnecuggftrfgrthhtvghrnheptefggedvffeiueffvefhiedtgfefjedukeefgeetgeevgeejgeekvdevjeelveeknecuffhomhgrihhnpehgihhthhhusgdrtghomhenucfkphepgedurdeiiedrieehrddujeeinecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepgedurdeiiedrieehrddujeeipdhhvghloheplhhotggrlhhhohhsthdpmhgrihhlfhhrohhmpehmihgthhgrvghlsehnihgvuggvrhhmrgihvghrrdgttgdpnhgspghrtghpthhtohepuddprhgtphhtthhopehffhhmphgvghdquggvvhgvlhesfhhfmhhpvghgrdhorhhg X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH 1/5] avformat/flvdec: Check for EOF in AudioPacketTypeMultichannelConfig 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="===============5728898218200969627==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============5728898218200969627== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="+LLUX+HKNUlgu62C" Content-Disposition: inline --+LLUX+HKNUlgu62C Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jul 15, 2025 at 12:28:04AM +0200, Timo Rothenpieler wrote: > On 7/15/2025 12:01 AM, Michael Niedermayer wrote: > > On Mon, Jul 14, 2025 at 10:00:19PM +0200, Timo Rothenpieler wrote: > > > On 7/14/2025 9:21 PM, Michael Niedermayer wrote: > > > > On Sun, Jul 13, 2025 at 01:42:28PM +0200, Timo Rothenpieler wrote: > > > > > On 7/13/2025 3:10 AM, Michael Niedermayer wrote: > > > > > > Fixes: Infinite loop > > > > > > Fixes: 427538726/clusterfuzz-testcase-minimized-ffmpeg_dem_FLV_= fuzzer-6582567304495104 > > > > > >=20 > > > > > > Found-by: continuous fuzzing process https://github.com/google/= oss-fuzz/tree/master/projects/ffmpeg > > > > > > Signed-off-by: Michael Niedermayer > > > > > > --- > > > > > > libavformat/flvdec.c | 3 +++ > > > > > > 1 file changed, 3 insertions(+) > > > > > >=20 > > > > > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c > > > > > > index ac681954cb7..a4fa0157512 100644 > > > > > > --- a/libavformat/flvdec.c > > > > > > +++ b/libavformat/flvdec.c > > > > > > @@ -1715,6 +1715,9 @@ retry_duration: > > > > > > av_log(s, AV_LOG_DEBUG, "Set channel data f= rom MultiChannel info.\n"); > > > > > > + if (avio_feof(s->pb)) > > > > > > + return AVERROR_EOF; > > > > > > + > > > > > > goto next_track; > > > > > > } > > > > > > } else if (stream_type =3D=3D FLV_STREAM_TYPE_VIDEO= ) { > > > > >=20 > > > > > I don't think just returning from here is correct. > > > > > The goto next_track right after it already checks for EOF. > > > >=20 > > > > > I do not see how between here and the eof check there there'd be = any way to > > > > > infinite loop. > > > >=20 > > > > avio_skip() with a negative value will reset the EOF flag > > > >=20 > > > >=20 > > > > >=20 > > > > > It returns FFERROR_REDO there, which is important to drain queued= up > > > > > packages. > > > >=20 > > > > I think the state becomes corrupted once it reads into EOF > > > > that is the size accounting goes "oops" as the code keeps running > > > > things that read and keeps accounting for these reads but in reality > > > > nothing is read as its at EOF > > > > and then it seeks back all these "not reads" and thats where it hit= s the > > > > infinite loop as theres a mismatch what the code thinks it moved fo= rward > > > > and what it actually moved forward. > > > > Thats how it looked to me at least, i have not verified every step = of this > > > >=20 > > > > ill mail you the testcase, then you can check if my analysis is rig= ht > > > > and fix the code in a way that can recover queued packets in such t= runcated > > > > packet at EOF case. > > > > Also please make sure its not forgotten that whatever fix this gets= is backported > > > I'm unable to reproduce any infinite loops, even with the sample. > > > But the code there definitely is sub-optimal, given the seek can go t= he > > > wrong way, and even when going the right way can potentially reset th= e EOF > > > flag. > > >=20 > > > Proposed patch is attached. > >=20 > > > flvdec.c | 12 ++++++++++-- > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > 681dde0e1e99d4e0cee0f4eec92eb3dc229a25d4 0001-avformat-flvdec-don-t-= skip-backwards-or-at-EOF.patch > > > From 7ff394e1ecab504a4cb0fda4bd0f25d88ee4f6fe Mon Sep 17 00:00:00 20= 01 > > > From: Timo Rothenpieler > > > Date: Mon, 14 Jul 2025 21:54:35 +0200 > > > Subject: [PATCH] avformat/flvdec: don't skip backwards or at EOF > > >=20 > > > --- > > > libavformat/flvdec.c | 12 ++++++++++-- > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > >=20 > > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c > > > index b90ed34b1c..de5e688822 100644 > > > --- a/libavformat/flvdec.c > > > +++ b/libavformat/flvdec.c > > > @@ -1860,8 +1860,16 @@ retry_duration: > > > next_track: > > > if (track_size) { > > > av_log(s, AV_LOG_WARNING, "Track size mismatch: %d!\n",= track_size); > > > - avio_skip(s->pb, track_size); > > > - size -=3D track_size; > > > + if (!avio_feof(s->pb)) { > > > + if (track_size > 0) { > > > + avio_skip(s->pb, track_size); > > > + size -=3D track_size; > > > + } else { > > > + /* We have somehow read more than the track had = to offer, leave and re-sync */ > > > + ret =3D FFERROR_REDO; > > > + goto leave; > > > + } > > > + } > > > } > >=20 > > i think this is not correct > >=20 > > if a corrupted packet pushes you 1gb forward into EOF you must seek back > > and by extension (if that logic is correct) we also can require a seek = back > > without EOF >=20 > If a corrupt package makes this avio_skip skip gigabytes ahead, reading w= ill > just EOF anyway a few lines down (or continue and try to re-sync from the= re, > if the file is big enough). i think re sync should happen from end of last good packet + minimum packet size or so not from random position Otherwise resync is not very robust thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle=20 --+LLUX+HKNUlgu62C Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCaHl6RgAKCRBhHseHBAsP qwZPAJ9Cp0mfiWhuoKqE11YkRSeXyKVlwwCfVVc5RvZSgHBaSlJswjk8alJRW10= =6xKV -----END PGP SIGNATURE----- --+LLUX+HKNUlgu62C-- --===============5728898218200969627== 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". --===============5728898218200969627==--