From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web10.102796.1674828879383268265 for ; Fri, 27 Jan 2023 06:14:40 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=hbIIn2En; spf=pass (domain: posteo.de, ip: 185.67.36.65, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 48C1D240125 for ; Fri, 27 Jan 2023 15:14:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1674828877; bh=SG9B6HH8XKyULycXQr5lSBilpfSpXnVTva4mwt51DOo=; h=Subject:From:Date:Cc:To:From; b=hbIIn2En+28lIfiTBgAY98aeH3l0NAycEaeqgAobEn1jrs57wPCkyFF3rP3VlgYbF NSsdFZ9ur5N2kbLRhThUBCJwHu2gv2miSlu5PzN3sBc+T6yA08mrdrnUOTRiFZ6hU6 otYRCzABvCfKWnX/bMtEH1P62tGohmKRZPYyO67rCftsjPXnDoWN5pBkwB6g4RY/XG LrQ0QjbT3k4biCjclMOYqwJlmikbh8ReXmGGuPcxJ2B2fdDBG2epGyof8ZIOd537jt R2x/5jpmwzM40mI885Ei/p/B7gJQiecNqPybfzDacBk5ECl2j6W+bn4EcQoBfPLqej dNX5pd7hihyeg== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4P3KMc1f9Sz9rxG; Fri, 27 Jan 2023 15:14:36 +0100 (CET) Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.400.51.1.1\)) Subject: Re: [edk2-platforms][PATCH v3 11/11] Ext4Pkg: Filter out directory entry names containing \0 as invalid From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= In-Reply-To: Date: Fri, 27 Jan 2023 14:14:25 +0000 Cc: Savva Mitrofanov , edk2-devel-groups-io , Vitaly Cheptsov Message-Id: References: <20230127092945.94389-1-savvamtr@gmail.com> <20230127092945.94389-12-savvamtr@gmail.com> To: Pedro Falcato Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > On 27. Jan 2023, at 15:09, Pedro Falcato = wrote: >=20 > On Fri, Jan 27, 2023 at 10:04 AM Marvin H=C3=A4user = wrote: >>=20 >> On 27. Jan 2023, at 10:29, Savva Mitrofanov = wrote: >>>=20 >>> The directory entry name conventions forbid having null-terminator >>> symbols in its body and can lead to undefined behavior conditions >>> and crashes >>>=20 >>> Cc: Marvin H=C3=A4user >>> Cc: Pedro Falcato >>> Cc: Vitaly Cheptsov >>> Fixes: 89b2bb0db263 ("Ext4Pkg: Fix and clarify handling regarding = non-utf8 dir entries") >>> Signed-off-by: Savva Mitrofanov >>> --- >>> Features/Ext4Pkg/Ext4Dxe/Directory.c | 9 ++++++++- >>> 1 file changed, 8 insertions(+), 1 deletion(-) >>>=20 >>> diff --git a/Features/Ext4Pkg/Ext4Dxe/Directory.c = b/Features/Ext4Pkg/Ext4Dxe/Directory.c >>> index 0753a20b5377..465749c9b51d 100644 >>> --- a/Features/Ext4Pkg/Ext4Dxe/Directory.c >>> +++ b/Features/Ext4Pkg/Ext4Dxe/Directory.c >>> @@ -28,9 +28,16 @@ Ext4GetUcs2DirentName ( >>> { >>> CHAR8 Utf8NameBuf[EXT4_NAME_MAX + 1]; >>> UINT16 *Str; >>> + UINTN Index; >>=20 >> I *really* do not like UINTN in code that does not deal with buffer = addresses and sizes. I'd change it to UINT8, but I'll leave it up to = Pedro. > Considering this is a size (length of name) and it gets explicitly Well, being a "size" is not sufficient, I explicitly referred to buffer = sizes. Say, you get an arbitrarily long buffer from a caller, its size = parameter should probably be UINTN, to be able to optimally leverage the = platform memory and be flexible. This is not a buffer size, this is a = bounded index of an architecture-agnostic data structure. I simply do = not want behaviour that depends on the host architecture in = architecture-agnostic code for no reason. We actually had multiple prior = examples of bugs due to this, though this is obviously fine (thus not = demanding it's changed). > bounded by name_len (a UINT8) I'm okay with it. But if for some reason > you need to submit a v4, please change. >=20 > Reviewed-by: Pedro Falcato >=20 > --=20 > Pedro