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 19EA441D10 for ; Thu, 17 Feb 2022 15:07:24 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8F5CE68B337; Thu, 17 Feb 2022 17:07:22 +0200 (EET) Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8228C68AFCD for ; Thu, 17 Feb 2022 17:07:16 +0200 (EET) 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 A947F2000B for ; Thu, 17 Feb 2022 15:07:15 +0000 (UTC) Date: Thu, 17 Feb 2022 16:07:14 +0100 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220217150714.GP2829255@pb2> References: <20220216084016.1979611-1-jiasheng@iscas.ac.cn> MIME-Version: 1.0 In-Reply-To: <20220216084016.1979611-1-jiasheng@iscas.ac.cn> Subject: Re: [FFmpeg-devel] [PATCH v2] avformat/nutdec: Add check for avformat_new_stream 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="===============8226390410721761366==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============8226390410721761366== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="asGm4nUSqZaI6CQG" Content-Disposition: inline --asGm4nUSqZaI6CQG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 16, 2022 at 04:40:16PM +0800, Jiasheng Jiang wrote: > As the potential failure of the memory allocation, > the avformat_new_stream() could return NULL pointer. > Therefore, it should be better to check it and return > error if fails. > Also, the caller, nut_read_header(), needs to deal with > the return value of the decode_main_header() and return > error if memory allocation fails. > To avoid mishandling the invalid 'time_base_count', another > check for the 'time_base_count' is needed and return different > error if fails. >=20 > Fixes: 619d8e2e58 ("updating nut demuxer to latest spec no muxing yet no = index yet no seeking yet libnuts crcs dont match mine (didnt investigate ye= t) samplerate is stored wrong by libnut (demuxer has a workaround) code is = not clean or beautifull yet, but i thought its better to commit early befor= e someone unneccesarily wastes his time duplicating the work demuxer split = =66rom muxer") > Signed-off-by: Jiasheng Jiang > --- > Changelog: >=20 > v1 -> v2 >=20 > * Change 1. Add the error handling for ENOMEM from decode_main_header() > in nut_read_header(). > * Change 2. Check for the 'time_base_count'. > --- > libavformat/nutdec.c | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) >=20 > diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c > index 0a8a700acf..4cbccb20d9 100644 > --- a/libavformat/nutdec.c > +++ b/libavformat/nutdec.c > @@ -220,6 +220,10 @@ static int decode_main_header(NUTContext *nut) > } > =20 > GET_V(nut->time_base_count, tmp > 0 && tmp < INT_MAX / sizeof(AVRati= onal) && tmp < length/2); > + > + if (nut->time_base_count > NUT_MAX_STREAMS) > + return AVERROR_INVALIDDATA; the code already checks against length/2. If you want to add to that that should be done at the same level and such a change should explain why the existing check is insufficent as well as why the new is correct and it should be in a patch seperate from other changes also a file with NUT_MAX_STREAMS streams could use more timebases in princi= ple timebases need a lot less space than streams so they could have a slightly higher limit > + > nut->time_base =3D av_malloc_array(nut->time_base_count, sizeof(AVRa= tional)); > if (!nut->time_base) > return AVERROR(ENOMEM); > @@ -351,8 +355,13 @@ static int decode_main_header(NUTContext *nut) > ret =3D AVERROR(ENOMEM); > goto fail; > } > - for (i =3D 0; i < stream_count; i++) > - avformat_new_stream(s, NULL); > + for (i =3D 0; i < stream_count; i++) { > + if (!avformat_new_stream(s, NULL)) { > + av_free(nut->stream); freeing something and not clearing the pointer is a bad idea in general thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin --asGm4nUSqZaI6CQG Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYg5knwAKCRBhHseHBAsP q5oyAKCW/ToBcqhvCvy1qQ8BTjh0Mo1tYgCeLljx1eU3ueFFjqREDfNq3KDMKnY= =elz3 -----END PGP SIGNATURE----- --asGm4nUSqZaI6CQG-- --===============8226390410721761366== 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". --===============8226390410721761366==--