From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100])
	by master.gitmailbox.com (Postfix) with ESMTP id 513654AFCB
	for <ffmpegdev@gitmailbox.com>; Tue, 25 Jun 2024 23:27:47 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
	by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E8AF668D636;
	Wed, 26 Jun 2024 02:27:44 +0300 (EEST)
Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net
 [217.70.183.193])
 by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3E90F68C75C
 for <ffmpeg-devel@ffmpeg.org>; Wed, 26 Jun 2024 02:27:38 +0300 (EEST)
Received: by mail.gandi.net (Postfix) with ESMTPSA id 95A93240004
 for <ffmpeg-devel@ffmpeg.org>; Tue, 25 Jun 2024 23:27:37 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc;
 s=gm1; t=1719358057;
 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=m6baTBU2GgK8sUG8DcCf8J9wqwdSdyAeG6NxOteYd5o=;
 b=O+HQau56K0P6LSMEHVnNRGFUvWm7E5SqclOu0HyKmpOFbBZfOj6zZ2BEV38am1TaUQSeBl
 sNmCh9oAYrqq8k7IrMakkVCcQ36gybvx/4Yz1buVFIKe7m4gBIjnQB6m19dGflyIgqJxhh
 5Xd2P1AJX9cqP611JMGqRoggbYPktprxGnXGcnUpeA2btfYM88KIcQHgqBCB4FTtsLHzR1
 rGzZYV+A3z4zIC7wuZ1lpwRoJeM6cPyVScmzOzJ3/SU+JYnLbyYIDFqq3FNC+ihllDFkby
 hFg82O3t3RYYxTQLLTn2IdNuPiBS9LpD8bF3RDxzCDzvYUEOacUrYRIRK4ptDw==
Date: Wed, 26 Jun 2024 01:27:36 +0200
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <20240625232736.GO4991@pb2>
References: <20240623230137.1749178-1-michael@niedermayer.cc>
 <20240623230137.1749178-2-michael@niedermayer.cc>
 <171930595471.21847.5930175805645108289@lain.khirnov.net>
MIME-Version: 1.0
In-Reply-To: <171930595471.21847.5930175805645108289@lain.khirnov.net>
X-GND-Sasl: michael@niedermayer.cc
Subject: Re: [FFmpeg-devel] [PATCH 2/5] avcodec/hevc/hevcdec: Do not allow
 changes to parameter sets between slices
X-BeenThere: ffmpeg-devel@ffmpeg.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
 <mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
 <mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Content-Type: multipart/mixed; boundary="===============5082799689641643064=="
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/20240625232736.GO4991@pb2/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>


--===============5082799689641643064==
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="pndui+VhQ7yNUqd0"
Content-Disposition: inline


--pndui+VhQ7yNUqd0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Jun 25, 2024 at 10:59:14AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2024-06-24 01:01:34)
> > The slice code detects changes by comparing the pps index.
> > That way the code cannot detect changes if the sets can change.
> >=20
> > Fixes: out of array access
> > Fixes: 69585/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuz=
zer-6389604524556288
> > Fixes: 69601/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuz=
zer-5069068613255168
> > Fixes: 69621/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuz=
zer-6187334182174720
> >=20
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz=
/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/hevc/hevcdec.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >=20
> > diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
> > index 1d2e53afc32..d68d454537a 100644
> > --- a/libavcodec/hevc/hevcdec.c
> > +++ b/libavcodec/hevc/hevcdec.c
> > @@ -3221,17 +3221,20 @@ static int decode_nal_unit(HEVCContext *s, cons=
t H2645NAL *nal)
> >          ret =3D ff_hevc_decode_nal_vps(&gb, s->avctx, &s->ps);
> >          if (ret < 0)
> >              goto fail;
> > +        s->sh.pps_id =3D -1;
>=20
> This is backwards. If the problem is that the current check does not
> detect the change, then the check should be fixed.

The specification explcitly doesnt allow the active *PS to change

"Any PPS NAL unit containing the value of pps_pic_parameter_set_id for the =
active PPS RBSP for a coded picture (and
consequently for the layer containing the coded picture) shall have the sam=
e content as that of the active PPS RBSP for the
coded picture, unless it follows the last VCL NAL unit of the coded picture=
 and precedes the first VCL NAL unit of another
coded picture."

"Any SPS NAL unit with nuh_layer_id equal to 0 containing the value of sps_=
seq_parameter_set_id for the active SPS
RBSP for the base layer for a CVS shall have the same content as that of th=
e active SPS RBSP for the base layer for the
CVS, unless it follows the last access unit of the CVS and precedes the fir=
st VCL NAL unit and the first SEI NAL unit
containing an active parameter sets SEI message (when present) of another C=
VS."

"Any SPS NAL unit with any nuh_layer_id value containing the value of sps_s=
eq_parameter_set_id for the active SPS
RBSP for a particular layer shall have the same content as that of the acti=
ve SPS RBSP for the particular layer unless it
follows the access unit auA containing the last coded picture for which the=
 active SPS RBSP for the particular layer is
required to be active for the particular layer and precedes the first NAL u=
nit succeeding auA in decoding order that activates
an SPS RBSP with the same value of seq_parameter_set_id."

So if anything fancy is wanted, the way to go is not to parse it in the fir=
st place.
Because if it matches then the parsing was a waste of time. If it mismatche=
s, that
would be invalid and would just lead to failure anyway

But really the primary goal is to fix the out of array access and be easy to
backport. I am not trying to fix this in a fancy way.
The simpler and more robust the fix is the better.

thx

[...]
--=20
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin

--pndui+VhQ7yNUqd0
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZntSZAAKCRBhHseHBAsP
q0W4AKCZSr/am/PmDTwMc6KlXUEwG0GaNgCeOFempLfJbJecsuO+K1Or+eKQa/8=
=DJ/a
-----END PGP SIGNATURE-----

--pndui+VhQ7yNUqd0--

--===============5082799689641643064==
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".

--===============5082799689641643064==--