From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f181.google.com (mail-pf1-f181.google.com [209.85.210.181]) by mx.groups.io with SMTP id smtpd.web11.41306.1673480657653152589 for ; Wed, 11 Jan 2023 15:44:17 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=jkU832xv; spf=pass (domain: gmail.com, ip: 209.85.210.181, mailfrom: pedro.falcato@gmail.com) Received: by mail-pf1-f181.google.com with SMTP id x4so8458024pfj.1 for ; Wed, 11 Jan 2023 15:44:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=xWKlA6P0VrXXnfvYmmrvCDD31+JFCYUD6nfdBopp6x4=; b=jkU832xvcXNzYePelqeePgtnWj1AaIRBxMxnQDE+BgWES9tlDnypY9a5UtD4VMPQy2 iLRIHIg8ITgJSgDbONP8+Nxs8+iMSNkP3dqpR+hep5DukAMgMutRYEeIJkubdv2l5ORG I28zkEGnc9lBjPOZq+0WFOMABwV3UrbWSim/afyZFy4sfViNgoMbum7/FYTpuwm037ul Wdz8aT5K5LbN855AzE6gkRnHHXzT3QFSWLyE18oyB25ocg/vJcIFyekUUHZO8HEXV8aa YAcUf9ZbkZoON+M7T5YGH6RUZMEYGYUgReS+RG3lcfx07NPQUtZIYBwPwBI4JpZbuttf wI3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=xWKlA6P0VrXXnfvYmmrvCDD31+JFCYUD6nfdBopp6x4=; b=x8d7ZW4Fqv0MNm1RhUubVHnIH5yzbFgTgiQhq9Ptddr93HTxklcygE1QTC3EhMJXBr JtyVIG61NZA8RqXV8iC23ZmKcIhLq31pJFZQVcOYk1lDJbxi64sSihkqa47NdHB6pa5K tg87oT6lhErUIKPuqiXS6WjLPUQw2YaNeh9zL5xJwCuGavKdZPd1JNTmsJ4Pn0B2V+wq CX49qpEyPZkBFNPhEcb+3uRgIuqrY/3wvvnNMD6MJoHrUK2xAGzWoYWRAOCIA78MJA38 KGX/tILXs3qHQurU4XOW79eQcwrECjgNBdh/T1l+0TX1SY1zwgjFR7V4aeNgOzd1VgeW mA/Q== X-Gm-Message-State: AFqh2kpYad0owrASSnN7t/TVI9s1RNQz1yY+8LnOkJ96D3pqjVHmZqFt C41SQ2ap0VHTm/2B/68cOXnhW3cfEwazBeoJqsA= X-Google-Smtp-Source: AMrXdXvAj//yqeBScR0QiY9+LEgwwqWH0MZNQP6IgcCwotCCqSxZtigU4dAnxX4rpFVQ383lcruB3vFGW6xiPxhC8jo= X-Received: by 2002:a63:165c:0:b0:491:9772:f4a6 with SMTP id 28-20020a63165c000000b004919772f4a6mr4144841pgw.90.1673480657027; Wed, 11 Jan 2023 15:44:17 -0800 (PST) MIME-Version: 1.0 References: <20230111232131.244584-1-pedro.falcato@gmail.com> <6B939CBC-0E8F-4BBD-98EE-051C88DEF238@posteo.de> In-Reply-To: <6B939CBC-0E8F-4BBD-98EE-051C88DEF238@posteo.de> From: "Pedro Falcato" Date: Wed, 11 Jan 2023 23:44:05 +0000 Message-ID: Subject: Re: [PATCH 1/1] Ext4Pkg: Add documentation surrounding ext4 directory entries To: =?UTF-8?Q?Marvin_H=C3=A4user?= Cc: devel@edk2.groups.io Content-Type: multipart/alternative; boundary="00000000000019cbe405f20594db" --00000000000019cbe405f20594db Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Jan 11, 2023 at 11:34 PM Marvin H=C3=A4user wr= ote: > > > On 12. Jan 2023, at 00:21, Pedro Falcato > wrote: > > > > =EF=BB=BFSeveral 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 > > Cc: Marvin H=C3=A4user > > --- > > Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 18 +++++++++++++++++- > > Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 3 +-- > > 2 files changed, 18 insertions(+), 3 deletions(-) > > > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > > index 4fd91a423324..2dad967e575d 100644 > > --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > > +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > > @@ -397,12 +397,28 @@ typedef struct _Ext4Inode { > > UINT32 i_projid; > > } EXT4_INODE; > > > > +#define EXT4_NAME_MAX 255 > > + > > typedef struct { > > + // ext4 directory entries are layed out in the > > laid out > > > following fashion: > > + // offset 0x0: UINT32 inode number (if 0, unused entry, should skip.= ) > > + // offset 0x4: UINT16 rec_len: Dir entry's length. > > + // Note: rec_len >=3D name_len + EXT4_MIN_DIR_ENTRY_LEN = and > rec_len % 4 =3D=3D 0. > > + // offset 0x6: UINT8 name_len: Dir entry's name's length > > + // offset 0x7: UINT8 file_type Dir entry's file type indicator > > + // offset 0x8: CHAR8 name[name_len]: Variable length character array= ; > not null-terminated. > > Why not document all that right above the member names? Would remove > redundant info and be more Doxygen-friendly. > > > + // > > + // Further note: 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 mus= t > treat it > > + // as a bag of bytes. When interacting with EFI interfaces themselve= s > (which expect UCS-2) > > + // we skip any directory entry that is not valid UTF-8. > > UINT32 inode; > > UINT16 rec_len; > > UINT8 name_len; > > UINT8 file_type; > > - CHAR8 name[255]; > > + CHAR8 name[EXT4_NAME_MAX]; > > } 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..81ba568c5947 100644 > > --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > > +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > > @@ -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 > > > ACK, sending v2. --=20 Pedro --00000000000019cbe405f20594db Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Wed, Jan 11, 2023 at 11:34 PM Marvin H= =C3=A4user <mhaeuser@posteo.de= > wrote:

> On 12. Jan 2023, at 00:21, Pedro Falcato <pedro.falcato@gmail.com> wrote:<= br> >
> =EF=BB=BFSeveral questions have popped up regarding the ext4 directory= entry
> layout and contents off-list. Attempt to clarify these issues by addin= g
> 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 | 18 +++++++++++++++++-
> Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h=C2=A0 |=C2=A0 3 +--
> 2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h b/Features/Ext4Pkg/Ex= t4Dxe/Ext4Disk.h
> index 4fd91a423324..2dad967e575d 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
> @@ -397,12 +397,28 @@ typedef struct _Ext4Inode {
>=C2=A0 =C2=A0UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0i_projid;
> } EXT4_INODE;
>
> +#define EXT4_NAME_MAX=C2=A0 255
> +
> typedef struct {
> +=C2=A0 // ext4 directory entries are layed out in the

laid out

> following fashion:
> +=C2=A0 // offset 0x0: UINT32 inode number (if 0, unused entry, should= skip.)
> +=C2=A0 // offset 0x4: UINT16 rec_len: Dir entry's length.
> +=C2=A0 //=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Note: rec_le= n >=3D name_len + EXT4_MIN_DIR_ENTRY_LEN and rec_len % 4 =3D=3D 0.
> +=C2=A0 // offset 0x6: UINT8 name_len: Dir entry's name's leng= th
> +=C2=A0 // offset 0x7: UINT8 file_type Dir entry's file type indic= ator
> +=C2=A0 // offset 0x8: CHAR8 name[name_len]: Variable length character= array; not null-terminated.

Why not document all that right above the member names? Would remove redund= ant info and be more Doxygen-friendly.

> +=C2=A0 //
> +=C2=A0 // Further note: ext4 directories are defined, as the document= ation puts it, as:
> +=C2=A0 // "a directory is more or less a flat file that maps an = arbitrary byte string
> +=C2=A0 // (usually ASCII) to an inode number on the filesystem".= So, they are not
> +=C2=A0 // necessarily encoded with ASCII, UTF-8, or any of the sort. = We must treat it
> +=C2=A0 // as a bag of bytes. When interacting with EFI interfaces the= mselves (which expect UCS-2)
> +=C2=A0 // we skip any directory entry that is not valid UTF-8.
>=C2=A0 =C2=A0UINT32=C2=A0 =C2=A0 inode;
>=C2=A0 =C2=A0UINT16=C2=A0 =C2=A0 rec_len;
>=C2=A0 =C2=A0UINT8=C2=A0 =C2=A0 =C2=A0name_len;
>=C2=A0 =C2=A0UINT8=C2=A0 =C2=A0 =C2=A0file_type;
> -=C2=A0 CHAR8=C2=A0 =C2=A0 =C2=A0name[255];
> +=C2=A0 CHAR8=C2=A0 =C2=A0 =C2=A0name[EXT4_NAME_MAX];
> } EXT4_DIR_ENTRY;
>
> #define EXT4_MIN_DIR_ENTRY_LEN=C2=A0 8
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext= 4Dxe/Ext4Dxe.h
> index adf3c13f6ea9..81ba568c5947 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> @@ -31,8 +31,7 @@
>
> #include "Ext4Disk.h"
>
> -#define SYMLOOP_MAX=C2=A0 =C2=A0 8
> -#define EXT4_NAME_MAX=C2=A0 255
> +#define SYMLOOP_MAX=C2=A0 8
> //
> // We need to specify path length limit for security purposes, to prev= ent possible
> // overflows and dead-loop conditions. Originally this limit is absent= in FS design,
> --
> 2.39.0
>

ACK, sending v2.
=

--
Pedro
--00000000000019cbe405f20594db--