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 289AB40466 for ; Fri, 24 Jun 2022 00:03:49 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 575BA68B6F6; Fri, 24 Jun 2022 03:03:46 +0300 (EEST) Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6702368B6BB for ; Fri, 24 Jun 2022 03:03:40 +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 9B5F3100003 for ; Fri, 24 Jun 2022 00:03:38 +0000 (UTC) Date: Fri, 24 Jun 2022 02:03:36 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220624000336.GO396728@pb2> References: <20220429221826.31666-1-michael@niedermayer.cc> <20220429221826.31666-2-michael@niedermayer.cc> MIME-Version: 1.0 In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH 2/2] avcodec/h2645_parse: Check HEVC NAL size 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="===============7028297142857728557==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============7028297142857728557== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="rrRB3ZuPW+qualkU" Content-Disposition: inline --rrRB3ZuPW+qualkU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 22, 2022 at 01:05:22PM +0200, Andreas Rheinhardt wrote: > Andreas Rheinhardt: > > Michael Niedermayer: > >> Fixes: Assertion failure > >> Fixes: 46662/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fu= zzer-4947860854013952 > >> > >> This also results in more frames to be decoded from fate samples > >> > >> Found-by: continuous fuzzing process https://github.com/google/oss-fuz= z/tree/master/projects/ffmpeg > >> Signed-off-by: Michael Niedermayer > >> --- > >> libavcodec/h2645_parse.c | 2 +- > >> .../ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 | 10 ++++++++++ > >> tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 | 3 +++ > >> 3 files changed, 14 insertions(+), 1 deletion(-) > >> > >> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c > >> index 03780680c6..78ab22b76e 100644 > >> --- a/libavcodec/h2645_parse.c > >> +++ b/libavcodec/h2645_parse.c > >> @@ -292,7 +292,7 @@ static int hevc_parse_nal_header(H2645NAL *nal, vo= id *logctx) > >> { > >> GetBitContext *gb =3D &nal->gb; > >> =20 > >> - if (get_bits1(gb) !=3D 0) > >> + if (get_bits_left(gb) < 16 || get_bits1(gb) !=3D 0) > >> return AVERROR_INVALIDDATA; > >> =20 > >> nal->type =3D get_bits(gb, 6); > >> diff --git a/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 b= /tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 > >> index 0c930f6556..3283925e38 100644 > >> --- a/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 > >> +++ b/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 > >> @@ -25,6 +25,16 @@ > >> 0, 19, 19, 1, 599040, 0x4227009b > >> 0, 20, 20, 1, 599040, 0x1bda8be4 > >> 0, 21, 21, 1, 599040, 0xd1d5dcb4 > >> +0, 22, 22, 1, 599040, 0x58b2edb3 > >> +0, 23, 23, 1, 599040, 0xd1f795d8 > >> +0, 24, 24, 1, 599040, 0x3331d5e6 > >> +0, 25, 25, 1, 599040, 0x5e5ec2c9 > >> +0, 26, 26, 1, 599040, 0x3b907bf5 > >> +0, 27, 27, 1, 599040, 0xefcbf471 > >> +0, 28, 28, 1, 599040, 0x2769a578 > >> +0, 29, 29, 1, 599040, 0x812ce986 > >> +0, 30, 30, 1, 599040, 0xf07c212c > >> +0, 31, 31, 1, 599040, 0xb5476890 > >> 0, 32, 32, 1, 599040, 0x00a0249f > >> 0, 33, 33, 1, 599040, 0x7263f7cf > >> 0, 34, 34, 1, 599040, 0x47054be4 > >> diff --git a/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 b/tests/re= f/fate/hevc-conformance-RAP_B_Bossen_1 > >> index e661ff245e..776267b59c 100644 > >> --- a/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 > >> +++ b/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 > >> @@ -70,6 +70,9 @@ > >> 0, 64, 64, 1, 149760, 0x3362678b > >> 0, 65, 65, 1, 149760, 0x6e7fc851 > >> 0, 66, 66, 1, 149760, 0x33f96449 > >> +0, 67, 67, 1, 149760, 0xd9d05007 > >> +0, 75, 75, 1, 149760, 0x477f2cf2 > >> +0, 76, 76, 1, 149760, 0xe1f9ccd0 > >> 0, 77, 77, 1, 149760, 0xb3ba8cfb > >> 0, 78, 78, 1, 149760, 0x64787995 > >> 0, 79, 79, 1, 149760, 0xc10de4c4 > >=20 > > get_bit_length currently presumes every NALU to contain > > rbsp_trailing_bits. Yet this is not true for the End of > > Sequence/Bitstream units which are just headers without RBSP. For these > > units, get_bit_length might truncate them -- it does so for end of > > sequence units in H.264. It would not be a serious issue for H.265, as > > the semantics of nuh_temporal_id_plus1 require nuh_temporal_id_plus1 to > > be 1 for End of Sequence/Bitstream units. Nevertheless I think this > > should be coupled with a patch that does not truncate the NAL unit if it > > is just a header. > >=20 >=20 > 1. I just sent a patch implementing the above: > https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297923.html > Please confirm that it actually fixes the testcase its commit message > claims to fix. confirmed! thx > 2. The RAP_B_Bossen_1 and NoOutPrior_A_Qualcomm_1 (where the testcases > change due to your patch) contain completely fine end of sequence NALUs. > Because they are valid, stripping them (as your patch does) is not ok > (e.g. these units would even be discarded when using hevc_metadata). > There are two bugs with these units: > a) Our parser puts them at the beginning of their NALUs, yet they should > be at the end of the (preceding) NALU. > b) When remuxing the samples to Matroska with mkvmerge (which puts these > units at the end of their packets), the output is the same as with raw > input, i.e. decoder still misses some frames. So somehow these units > confuse the decoder. [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Old school: Use the lowest level language in which you can solve the problem conveniently. New school: Use the highest level language in which the latest supercomputer can solve the problem without the user falling asleep waiting. --rrRB3ZuPW+qualkU Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYrT/VQAKCRBhHseHBAsP q+2xAJ0bpE0FWDiB3gNcoXGCtw+87HF6/QCePGiFH1n4EhjnLjkmNRj69TxFmio= =i81K -----END PGP SIGNATURE----- --rrRB3ZuPW+qualkU-- --===============7028297142857728557== 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". --===============7028297142857728557==--