From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web11.120480.1673716218064298502 for ; Sat, 14 Jan 2023 09:10:18 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=G9reRQF6; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id AC6D42401AE for ; Sat, 14 Jan 2023 18:10:15 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1673716215; bh=ifmhpco0l3ZYE5Ld0TUhYobEYAZ+NqEM/m6tAABQlso=; h=From:Subject:Date:Cc:To:From; b=G9reRQF6QgQ8p6eYAcmQSOjnUO6bvKzxBa8RF6qh3huwWmzYTa7mPavBDS1U/bhRj 5XMxvn/mkIdaKW0299/NywhD6J+kn3j1SeUoa/oCr375kQYzvP6bYXp2N69MKMJOL5 qWPde89qHe8uMLacweuuo93lBaNNwnQ7vZ5m9hAtFN845CKb1tYoTzHfrWoM705Ogg uJU7nP0FB/YQgFBX84kO5HNUV7Cu5C3cxgwgtA0EhPpOhE6RYqCL9vUDRGiRcQrVto sJj7PUhCRPg0x+xhpo1HGaU4TldJwRRUjS5bxC4Xm/noE+RFu2VFzUQHCXeuX4/iqs S1YHX5kWc95hA== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4NvPtG6x8zz6tmG; Sat, 14 Jan 2023 18:10:14 +0100 (CET) From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-Id: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.300.101.1.3\)) Subject: Re: [PATCH 2/3] Ext4Pkg: Add documentation surrounding ext4 directory entries Date: Sat, 14 Jan 2023 17:10:04 +0000 In-Reply-To: <20230111235920.252317-4-pedro.falcato@gmail.com> Cc: edk2-devel-groups-io To: Pedro Falcato References: <20230111235920.252317-1-pedro.falcato@gmail.com> <20230111235920.252317-4-pedro.falcato@gmail.com> Content-Type: multipart/alternative; boundary="Apple-Mail=_80AFE934-7699-4B87-8465-FC21C9037E66" --Apple-Mail=_80AFE934-7699-4B87-8465-FC21C9037E66 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Well, this is still not Doxygen-friendly (requires three slashes) and = it's still awkward (as in done nowhere else) to explicitly list the = offset or name in the comment. In case neither is a concern: Reviewed-by: Marvin H=C3=A4user > > On 12. Jan 2023, at 00:59, Pedro Falcato = wrote: >=20 > Several questions have popped up regarding the ext4 directory entry > layout and contents off-list. Attempt to clarify these issues by = adding > some explanatory comments. >=20 > Signed-off-by: Pedro Falcato > Cc: Marvin H=C3=A4user > --- > Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 21 +++++++++++++++++++-- > Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 5 ++--- > 2 files changed, 21 insertions(+), 5 deletions(-) >=20 > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h = b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > index 4fd91a423324..d0a455d0e572 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > @@ -1,7 +1,7 @@ > /** @file > Raw filesystem data structures >=20 > - Copyright (c) 2021 Pedro Falcato All rights reserved. > + Copyright (c) 2021 - 2023 Pedro Falcato All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > Layout of an EXT2/3/4 filesystem: > @@ -397,12 +397,29 @@ typedef struct _Ext4Inode { > UINT32 i_projid; > } EXT4_INODE; >=20 > +#define EXT4_NAME_MAX 255 > + > typedef struct { > + // offset 0x0: inode number (if 0, unused entry, should skip.) > UINT32 inode; > + // offset 0x4: Directory entry's length. > + // Note: rec_len >=3D name_len + EXT4_MIN_DIR_ENTRY_LEN = and rec_len % 4 =3D=3D 0. > UINT16 rec_len; > + // offset 0x6: Directory entry's name's length > UINT8 name_len; > + // offset 0x7: Directory entry's file type indicator > UINT8 file_type; > - CHAR8 name[255]; > + // offset 0x8: name[name_len]: Variable length character array; not = null-terminated. > + CHAR8 name[EXT4_NAME_MAX]; > + // Further notes on names: > + // 1) We use EXT4_NAME_MAX here instead of flexible arrays for ease = of use around the driver. > + // > + // 2) ext4 directories are defined, as the documentation puts it, = as: > + // "a directory is more or less a flat file that maps an arbitrary = byte string > + // (usually ASCII) to an inode number on the filesystem". So, they = are not > + // necessarily encoded with ASCII, UTF-8, or any of the sort. We = must treat it > + // as a bag of bytes. When interacting with EFI interfaces = themselves (which expect UCS-2) > + // we skip any directory entry that is not valid UTF-8. > } EXT4_DIR_ENTRY; >=20 > #define EXT4_MIN_DIR_ENTRY_LEN 8 > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h = b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > index adf3c13f6ea9..466e49523030 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > @@ -1,7 +1,7 @@ > /** @file > Common header for the driver >=20 > - Copyright (c) 2021 - 2022 Pedro Falcato All rights reserved. > + Copyright (c) 2021 - 2023 Pedro Falcato All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent > **/ >=20 > @@ -31,8 +31,7 @@ >=20 > #include "Ext4Disk.h" >=20 > -#define SYMLOOP_MAX 8 > -#define EXT4_NAME_MAX 255 > +#define SYMLOOP_MAX 8 > // > // We need to specify path length limit for security purposes, to = prevent possible > // overflows and dead-loop conditions. Originally this limit is absent = in FS design, > --=20 > 2.39.0 >=20 --Apple-Mail=_80AFE934-7699-4B87-8465-FC21C9037E66 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Well, this is = still not Doxygen-friendly (requires three slashes) and it's still = awkward (as in done nowhere else) to explicitly list the offset or name = in the comment. In case neither is a = concern:

Reviewed-by: Marvin H=C3=A4user <mhaeuser@posteo.de>

=
On 12. Jan 2023, at 00:59, Pedro Falcato = <pedro.falcato@gmail.com> wrote:

Several questions have = popped up regarding the ext4 directory entry
layout and contents = off-list. Attempt to clarify these issues by adding
some explanatory = comments.

Signed-off-by: Pedro Falcato = <pedro.falcato@gmail.com>
Cc: Marvin H=C3=A4user = <mhaeuser@posteo.de>
---
= Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 21 +++++++++++++++++++--
= Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h  |  5 ++---
2 files = changed, 21 insertions(+), 5 deletions(-)

diff --git = a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h = b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
index = 4fd91a423324..d0a455d0e572 100644
--- = a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
+++ = b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
@@ -1,7 +1,7 @@
/** = @file
  Raw filesystem data structures

- =  Copyright (c) 2021 Pedro Falcato All rights reserved.
+ =  Copyright (c) 2021 - 2023 Pedro Falcato All rights reserved.
=   SPDX-License-Identifier: BSD-2-Clause-Patent

=   Layout of an EXT2/3/4 filesystem:
@@ -397,12 +397,29 @@ = typedef struct _Ext4Inode {
  UINT32 =       i_projid;
} = EXT4_INODE;

+#define EXT4_NAME_MAX  255
+
typedef = struct {
+  // offset 0x0: inode number (if 0, unused entry, = should skip.)
  UINT32    inode;
+ =  // offset 0x4: Directory entry's length.
+  // =             No= te: rec_len >=3D name_len + EXT4_MIN_DIR_ENTRY_LEN and rec_len % 4 =3D=3D= 0.
  UINT16    rec_len;
+  // = offset 0x6: Directory entry's name's length
  UINT8 =     name_len;
+  // offset 0x7: Directory = entry's file type indicator
  UINT8 =     file_type;
-  CHAR8 =     name[255];
+  // offset 0x8: = name[name_len]: Variable length character array; not = null-terminated.
+  CHAR8 =     name[EXT4_NAME_MAX];
+  // Further notes = on names:
+  // 1) We use EXT4_NAME_MAX here instead of flexible = arrays for ease of use around the driver.
+  //
+  // 2) = ext4 directories are defined, as the documentation puts it, as:
+ =  // "a directory is more or less a flat file that maps an arbitrary = byte string
+  // (usually ASCII) to an inode number on the = filesystem". So, they are not
+  // necessarily encoded with = ASCII, UTF-8, or any of the sort. We must treat it
+  // as a = bag of bytes. When interacting with EFI interfaces themselves (which = expect UCS-2)
+  // we skip any directory entry that is not = valid UTF-8.
} EXT4_DIR_ENTRY;

#define = EXT4_MIN_DIR_ENTRY_LEN  8
diff --git = a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h = b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
index adf3c13f6ea9..466e49523030 = 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ = b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -1,7 +1,7 @@
/** = @file
  Common header for the driver

- =  Copyright (c) 2021 - 2022 Pedro Falcato All rights reserved.
+ =  Copyright (c) 2021 - 2023 Pedro Falcato All rights reserved.
=   SPDX-License-Identifier: BSD-2-Clause-Patent
= **/

@@ -31,8 +31,7 @@

#include = "Ext4Disk.h"

-#define SYMLOOP_MAX    8
-#define = EXT4_NAME_MAX  255
+#define SYMLOOP_MAX  8
//
// We = need to specify path length limit for security purposes, to prevent = possible
// overflows and dead-loop conditions. Originally this = limit is absent in FS design,
-- =
2.39.0


= --Apple-Mail=_80AFE934-7699-4B87-8465-FC21C9037E66--