From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) by mx.groups.io with SMTP id smtpd.web09.4499.1661811168012870732 for ; Mon, 29 Aug 2022 15:12:48 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=mxsh6O/H; spf=pass (domain: gmail.com, ip: 209.85.214.178, mailfrom: pedro.falcato@gmail.com) Received: by mail-pl1-f178.google.com with SMTP id p18so9294875plr.8 for ; Mon, 29 Aug 2022 15:12:47 -0700 (PDT) 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; bh=wDAsnrGD2w5M7iL7UT2Pz5iscEZpjT3qnhctqoTdOag=; b=mxsh6O/HzDU3wkXqeQMtK4mx6R6xFFcScJdSpK3qTvsysYgiNLYkgJ/bE9la5Qwvnj Kw7ue5A3Pr/oR7xYZFwe9K01yaSl+Z8KMb5XyXtfS48urV1wcAOz4uC3WBphn3ddHcJi Qoge+eyg9ij1tgBfJQ1kYZ7Z96vAoF0j555/avCMlmVB+LeopzN8/3jiiWpGRBzXZYTF urvjwdJJm3TRoiIQx2IGURUiufweXquKP1/PYZOhzlowZMvVt9yHBmJgleHAS3c5/gYG AwrDu8/11yXW26KnJYZxa5dcwsjXHQ12FrSVislKQikLVb9nXAFLQ8+nG/oE8KRy8rvF HE2g== 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; bh=wDAsnrGD2w5M7iL7UT2Pz5iscEZpjT3qnhctqoTdOag=; b=f2ClYNU1sYSFCLEP8HtSBk6wcEPnsySXxsggS5zQ2p73Yutxy8iGI2qJEI0EqpHJNr eHNNxY3RqfZtW9xeUQSQAJbdxMmTXBICbsZOT/X6X0AjLVDzqOVctMT/WaH79ZSjAKM9 7rMKfOx66Nm7g3WjRAQsIrwmxLDfXT9HAkDJPjIykKTvdNnB51US2MrHX1APwKZGGHrl jhIaMcR8mR+D9Q0M0wWLVxcsh3iAUfiIcnNJL3F9E5QTFGsHf6RgBJipMLBVkcidG1b1 mJKbAeIxMurP7cwY7QwqZbUQ6Obc8D6OtHYpXu7xiEWlTC0AKL4DLJUcj6plhmN8FT9s axMg== X-Gm-Message-State: ACgBeo3XdKwaG2fw8DeER//WX1opzp/Z05jXInfXGo84N0GB8oQ7O6Xj L+JSrCcEXl+uBTVxsdnPKbbhkC4syEbCydtNNYM= X-Google-Smtp-Source: AA6agR7rin487FUv3Z6vcZl6HmeD+S365+jLY20KBee4dysEExJmGrmS4epmnrUAOa4V0WYKKIIkH3YvC0QKRT5Zv14= X-Received: by 2002:a17:902:c944:b0:174:f62a:14f0 with SMTP id i4-20020a170902c94400b00174f62a14f0mr3574700pla.168.1661811167262; Mon, 29 Aug 2022 15:12:47 -0700 (PDT) MIME-Version: 1.0 References: <20220728152644.11435-1-savvamtr@gmail.com> <20220728152644.11435-2-savvamtr@gmail.com> In-Reply-To: <20220728152644.11435-2-savvamtr@gmail.com> From: "Pedro Falcato" Date: Mon, 29 Aug 2022 23:12:35 +0100 Message-ID: Subject: Re: [edk2-platforms][PATCH v4 1/2] Ext4Pkg: Add symbolic links support To: Savva Mitrofanov Cc: edk2-devel-groups-io , =?UTF-8?Q?Marvin_H=C3=A4user?= , Vitaly Cheptsov Content-Type: multipart/alternative; boundary="0000000000004efec705e768907c" --0000000000004efec705e768907c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Savva, Sorry for the huge delay. Comments inline. On Thu, Jul 28, 2022 at 4:26 PM Savva Mitrofanov wrote= : > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3677 > > Provided support for symlink file type. Added routine which allows > reading and following them through recursive open() call. As a security > meausure implemented simple symlink loop check with nest level limit > equal 8. Also this patch moves Ext4Open functionality to internal > routine. > > Cc: Marvin H=C3=A4user > Cc: Pedro Falcato > Cc: Vitaly Cheptsov > Signed-off-by: Savva Mitrofanov > --- > Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 13 +- > Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 98 +++++- > Features/Ext4Pkg/Ext4Dxe/File.c | 359 ++++++++++++++++++-- > Features/Ext4Pkg/Ext4Dxe/Inode.c | 53 +++ > 4 files changed, 485 insertions(+), 38 deletions(-) > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > index a55cd2fa68ad..a73e3f8622f1 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h > @@ -171,7 +171,7 @@ > #define EXT4_DIRTY_FL 0x00000100 > #define EXT4_COMPRBLK_FL 0x00000200 > #define EXT4_NOCOMPR_FL 0x00000400 > -#define EXT4_ECOMPR_FL 0x00000800 > +#define EXT4_ENCRYPT_FL 0x00000800 > #define EXT4_BTREE_FL 0x00001000 > #define EXT4_INDEX_FL 0x00002000 > #define EXT4_JOURNAL_DATA_FL 0x00004000 > @@ -332,11 +332,12 @@ STATIC_ASSERT ( > "ext4 block group descriptor struct has incorrect size" > ); > > -#define EXT4_DBLOCKS 12 > -#define EXT4_IND_BLOCK 12 > -#define EXT4_DIND_BLOCK 13 > -#define EXT4_TIND_BLOCK 14 > -#define EXT4_NR_BLOCKS 15 > +#define EXT4_DBLOCKS 12 > +#define EXT4_IND_BLOCK 12 > +#define EXT4_DIND_BLOCK 13 > +#define EXT4_TIND_BLOCK 14 > +#define EXT4_NR_BLOCKS 15 > +#define EXT4_FAST_SYMLINK_MAX_SIZE EXT4_NR_BLOCKS * sizeof(UINT32) > > #define EXT4_GOOD_OLD_INODE_SIZE 128 > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > index b1508482b0a7..c1df9d1149e4 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > @@ -31,7 +31,9 @@ > > #include "Ext4Disk.h" > > +#define SYMLOOP_MAX 8 > I'm scared that this may not be enough. Could we use 40 here as Linux does? > #define EXT4_NAME_MAX 255 > +#define EFI_PATH_MAX 4096 > Don't use EFI_* in Ext4Dxe code, so prefix this with EXT4_ instead maybe? > > #define EXT4_DRIVER_VERSION 0x0000 > > @@ -324,11 +326,11 @@ number of read bytes. > **/ > EFI_STATUS > Ext4Read ( > - IN EXT4_PARTITION *Partition, > - IN EXT4_FILE *File, > - OUT VOID *Buffer, > - IN UINT64 Offset, > - IN OUT UINTN *Length > + IN EXT4_PARTITION *Partition, > + IN EXT4_FILE *File, > + OUT VOID *Buffer, > + IN UINT64 Offset, > + IN OUT UINTN *Length > ); > > /** > @@ -368,6 +370,7 @@ struct _Ext4File { > > UINT64 OpenMode; > UINT64 Position; > + UINT32 SymLoops; > > EXT4_PARTITION *Partition; > > @@ -497,6 +500,45 @@ Ext4SetupFile ( > IN EXT4_PARTITION *Partition > ); > > +/** > + Opens a new file relative to the source file's location. > + > + @param[out] FoundFile A pointer to the location to return the opened > handle for the new > + file. > + @param[in] Source A pointer to the EXT4_FILE instance that is the > file > + handle to the source location. This would > typically be an open > + handle to a directory. > + @param[in] FileName The Null-terminated string of the name of the > file to be opened. > + The file name may contain the following path > modifiers: "\", ".", > + and "..". > + @param[in] OpenMode The mode to open the file. The only valid > combinations that the > + file may be opened with are: Read, Read/Write, > or Create/Read/Write. > + @param[in] Attributes Only valid for EFI_FILE_MODE_CREATE, in which > case these are the > + attribute bits for the newly created file. > + > + @retval EFI_SUCCESS The file was opened. > + @retval EFI_NOT_FOUND The specified file could not be found on > the device. > + @retval EFI_NO_MEDIA The device has no medium. > + @retval EFI_MEDIA_CHANGED The device has a different medium in it o= r > the medium is no > + longer supported. > + @retval EFI_DEVICE_ERROR The device reported an error. > + @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted. > + @retval EFI_WRITE_PROTECTED An attempt was made to create a file, or > open a file for write > + when the media is write-protected. > + @retval EFI_ACCESS_DENIED The service denied access to the file. > + @retval EFI_OUT_OF_RESOURCES Not enough resources were available to > open the file. > + @retval EFI_VOLUME_FULL The volume is full. > + > +**/ > +EFI_STATUS > +Ext4OpenInternal ( > + OUT EXT4_FILE **FoundFile, > First IN parameters, then OUT parameters. > + IN EXT4_FILE *Source, > + IN CHAR16 *FileName, > + IN UINT64 OpenMode, > + IN UINT64 Attributes > + ); > + > /** > Closes a file. > > @@ -774,6 +816,30 @@ Ext4FileIsDir ( > IN CONST EXT4_FILE *File > ); > > +/** > + Checks if a file is a symlink. > + > + @param[in] File Pointer to the opened file. > + > + @return BOOLEAN Whether file is a symlink > +**/ > +BOOLEAN > +Ext4FileIsSymlink ( > + IN CONST EXT4_FILE *File > + ); > + > +/** > + Detects if a symlink is a fast symlink. > + > + @param[in] File Pointer to the opened file. > + > + @return BOOLEAN Whether symlink is a fast symlink > +**/ > +BOOLEAN > +Ext4SymlinkIsFastSymlink ( > + IN CONST EXT4_FILE *File > + ); > Does this need to be in Ext4Dxe.h? This is pretty symlink specific. > + > /** > Checks if a file is a regular file. > @param[in] File Pointer to the opened file. > @@ -797,7 +863,7 @@ Ext4FileIsReg ( > it's a regular file or a directory, since most other file typ= es > don't make sense under UEFI. > **/ > -#define Ext4FileIsOpenable(File) (Ext4FileIsReg(File) || > Ext4FileIsDir(File)) > +#define Ext4FileIsOpenable(File) (Ext4FileIsReg (File) || Ext4FileIsDir > (File) || Ext4FileIsSymlink (File)) > > #define EXT4_INODE_HAS_FIELD(Inode, Field) > \ > (Inode->i_extra_isize + EXT4_GOOD_OLD_INODE_SIZE >=3D > \ > @@ -935,6 +1001,26 @@ Ext4ReadDir ( > IN OUT UINTN *OutLength > ); > > +/** > + Reads a symlink file. > + > + @param[in] Partition Pointer to the ext4 partition. > + @param[in] File Pointer to the open symlink file. > + @param[out] Symlink Pointer to the output unicode symlink > string. > + > + @retval EFI_SUCCESS Symlink was read. > + @retval EFI_ACCESS_DENIED Symlink is encrypted. > + @retval EFI_OUT_OF_RESOURCES Memory allocation error. > + @retval EFI_INVALID_PARAMETER Symlink path has incorrect length > + @retval EFI_VOLUME_CORRUPTED Symlink read block size differ from inod= e > value > +**/ > +EFI_STATUS > +Ext4ReadSymlink ( > + IN EXT4_PARTITION *Partition, > + IN EXT4_FILE *File, > + OUT CHAR16 **Symlink > + ); > + > /** > Initialises the (empty) extents map, that will work as a cache of > extents. > > diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c > b/Features/Ext4Pkg/Ext4Dxe/File.c > index ff1746d5640a..ae9230d6422b 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/File.c > +++ b/Features/Ext4Pkg/Ext4Dxe/File.c > @@ -134,14 +134,224 @@ Ext4DirCanLookup ( > return (File->Inode->i_mode & EXT4_INO_PERM_EXEC_OWNER) =3D=3D > EXT4_INO_PERM_EXEC_OWNER; > } > > +/** > + Reads a fast symlink file. > + > + @param[in] Partition Pointer to the ext4 partition. > + @param[in] File Pointer to the open symlink file. > + @param[out] AsciiSymlink Pointer to the output ascii symlink > string. > + @param[out] AsciiSymlinkSize Pointer to the output ascii symlink > string length. > + > + @retval EFI_SUCCESS Fast symlink was read. > + @retval EFI_OUT_OF_RESOURCES Memory allocation error. > +**/ > +STATIC > +EFI_STATUS > +Ext4ReadFastSymlink ( > + IN EXT4_PARTITION *Partition, > + IN EXT4_FILE *File, > + OUT CHAR8 **AsciiSymlink, > + OUT UINT32 *AsciiSymlinkSize > + ) > +{ > + UINT32 SymlinkSize; > + CHAR8 *AsciiSymlinkTmp; > + // > + // Fast-symlink's EXT4_INODE_SIZE is not necessarily validated when we > checked it in > + // Ext4SymlinkIsFastSymlink(), so truncate if necessary. > + // > I think the correct solution here is to check if the inode size fits in EXT4_FAST_SYMLINK_MAX_SIZE in Ext4SymlinkIsFastSymlink. Or possibly check it here and error-out (blatant corruption). Truncating doesn't seem correct. > + SymlinkSize =3D (UINT32)MIN (EXT4_INODE_SIZE (File->Inode), > EXT4_FAST_SYMLINK_MAX_SIZE); > + > + AsciiSymlinkTmp =3D AllocatePool (SymlinkSize + 1); > + if (AsciiSymlinkTmp =3D=3D NULL) { > + DEBUG ((DEBUG_FS, "[ext4] Failed to allocate symlink ascii string > buffer\n")); > Nit: DEBUG_ERROR? > + return EFI_OUT_OF_RESOURCES; > + } > + > + CopyMem (AsciiSymlinkTmp, File->Inode->i_data, SymlinkSize); > + > + // > + // Add null-terminator > + // > + AsciiSymlinkTmp[SymlinkSize] =3D '\0'; > + > + *AsciiSymlink =3D AsciiSymlinkTmp; > + *AsciiSymlinkSize =3D SymlinkSize + 1; > + > + return EFI_SUCCESS; > +} > + > +/** > + Reads a slow symlink file. > + > + @param[in] Partition Pointer to the ext4 partition. > + @param[in] File Pointer to the open symlink file. > + @param[out] AsciiSymlink Pointer to the output ascii symlink > string. > + @param[out] AsciiSymlinkSize Pointer to the output ascii symlink > string length. > + > + @retval EFI_SUCCESS Slow symlink was read. > + @retval EFI_OUT_OF_RESOURCES Memory allocation error. > + @retval EFI_INVALID_PARAMETER Slow symlink path has incorrect length > + @retval EFI_VOLUME_CORRUPTED Symlink read block size differ from inod= e > value > +**/ > +STATIC > +EFI_STATUS > +Ext4ReadSlowSymlink ( > + IN EXT4_PARTITION *Partition, > + IN EXT4_FILE *File, > + OUT CHAR8 **AsciiSymlink, > + OUT UINT32 *AsciiSymlinkSize > + ) > +{ > + EFI_STATUS Status; > + CHAR8 *SymlinkTmp; > + UINT64 SymlinkSizeTmp; > + UINT32 SymlinkAllocateSize; > + UINTN ReadSize; > + > + SymlinkSizeTmp =3D EXT4_INODE_SIZE (File->Inode); > + > + // > + // Allocate EXT4_INODE_SIZE + 1 > + // > + if (SymlinkSizeTmp > EFI_PATH_MAX - 1) { > Why is this > EFI_PATH_MAX - 1 and not >=3D EFI_PATH_MAX? Also, why does th= e inode size need to be strictly smaller than EFI_PATH_MAX? > + DEBUG (( > + DEBUG_FS, > Nit: DEBUG_ERROR? > + "[ext4] Error! Symlink path maximum length was hit!\n" > + )); > + return EFI_INVALID_PARAMETER; > + } > + > + SymlinkAllocateSize =3D (UINT32)SymlinkSizeTmp + 1; > + > + SymlinkTmp =3D AllocatePool (SymlinkAllocateSize); > + if (SymlinkTmp =3D=3D NULL) { > + DEBUG ((DEBUG_FS, "[ext4] Failed to allocate symlink ascii string > buffer\n")); > + return EFI_OUT_OF_RESOURCES; > + } > + + ReadSize =3D (UINTN)SymlinkSizeTmp; > + Status =3D Ext4Read (Partition, File, SymlinkTmp, File->Position, > &ReadSize); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_FS, "[ext4] Failed to read symlink from blocks with > Status %r\n", Status)); > Nit: lowercase "status" in the DEBUG. > + FreePool (SymlinkTmp); > + return Status; > + } > + > + File->Position +=3D ReadSize; > Taking Position into account (here and in the Ext4Read call) seems wrong. Why would the file seek affect the readlink() operation? > + > + // > + // Add null-terminator > + // > + SymlinkTmp[SymlinkSizeTmp] =3D '\0'; > + > + if (SymlinkSizeTmp !=3D ReadSize) { > + DEBUG (( > + DEBUG_FS, > + "[ext4] Error! The sz of the read block doesn't match the value > from the inode!\n" > Nit: size > + )); > + return EFI_VOLUME_CORRUPTED; > + } > + > + *AsciiSymlinkSize =3D SymlinkAllocateSize; > + *AsciiSymlink =3D SymlinkTmp; > + > + return EFI_SUCCESS; > +} > + > +/** > + Reads a symlink file. > + > + @param[in] Partition Pointer to the ext4 partition. > + @param[in] File Pointer to the open symlink file. > + @param[out] Symlink Pointer to the output unicode symlink > string. > + > + @retval EFI_SUCCESS Symlink was read. > + @retval EFI_ACCESS_DENIED Symlink is encrypted. > + @retval EFI_OUT_OF_RESOURCES Memory allocation error. > + @retval EFI_INVALID_PARAMETER Symlink path has incorrect length > + @retval EFI_VOLUME_CORRUPTED Symlink read block size differ from inod= e > value > +**/ > +EFI_STATUS > +Ext4ReadSymlink ( > + IN EXT4_PARTITION *Partition, > + IN EXT4_FILE *File, > + OUT CHAR16 **Symlink > + ) > +{ > + EFI_STATUS Status; > + CHAR8 *SymlinkTmp; > + UINT32 SymlinkSize; > + CHAR16 *Symlink16Tmp; > + CHAR16 *Needle; > + > + // > + // Assume that we alread read Inode via Ext4ReadInode > + // Skip reading, just check encryption flag > + // > + if ((File->Inode->i_flags & EXT4_ENCRYPT_FL) !=3D 0) { > + DEBUG ((DEBUG_FS, "[ext4] Error, symlink is encrypted\n")); > Nit: DEBUG_ERROR and "Error: ". > + return EFI_ACCESS_DENIED; > + } > + > + if (Ext4SymlinkIsFastSymlink (File)) { > + Status =3D Ext4ReadFastSymlink (Partition, File, &SymlinkTmp, > &SymlinkSize); > + } else { > + Status =3D Ext4ReadSlowSymlink (Partition, File, &SymlinkTmp, > &SymlinkSize); > + } > + > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_FS, "[ext4] Symlink read error with Status %r\n", > Status)); > + return Status; > + } > + > + Symlink16Tmp =3D AllocateZeroPool (SymlinkSize * sizeof (CHAR16)); > + if (Symlink16Tmp =3D=3D NULL) { > + DEBUG ((DEBUG_FS, "[ext4] Failed to allocate symlink unicode string > buffer\n")); > + FreePool (SymlinkTmp); > + return EFI_OUT_OF_RESOURCES; > + } > + > + Status =3D AsciiStrToUnicodeStrS ( > + SymlinkTmp, > + Symlink16Tmp, > + SymlinkSize > + ); > + > + if (EFI_ERROR (Status)) { > + DEBUG (( > + DEBUG_FS, > + "[ext4] Failed to convert ascii symlink to unicode with Status > %r\n", > + Status > + )); > + FreePool (Symlink16Tmp); > + FreePool (SymlinkTmp); > + return Status; > + } > + > + // > + // Convert to UEFI slashes > + // > + for (Needle =3D Symlink16Tmp; *Needle !=3D L'\0'; Needle++) { > + if (*Needle =3D=3D L'/') { > + *Needle =3D L'\\'; > + } > + } > + > + *Symlink =3D Symlink16Tmp; > + > + FreePool (SymlinkTmp); > + return Status; > +} > + > /** > Opens a new file relative to the source file's location. > > - @param[in] This A pointer to the EFI_FILE_PROTOCOL instance tha= t > is the file > + @param[out] FoundFile A pointer to the location to return the opened > handle for the new > + file. > + @param[in] Source A pointer to the EXT4_FILE instance that is the > file > handle to the source location. This would > typically be an open > handle to a directory. > - @param[out] NewHandle A pointer to the location to return the opened > handle for the new > - file. > @param[in] FileName The Null-terminated string of the name of the > file to be opened. > The file name may contain the following path > modifiers: "\", ".", > and "..". > @@ -165,13 +375,12 @@ Ext4DirCanLookup ( > > **/ > EFI_STATUS > -EFIAPI > -Ext4Open ( > - IN EFI_FILE_PROTOCOL *This, > - OUT EFI_FILE_PROTOCOL **NewHandle, > - IN CHAR16 *FileName, > - IN UINT64 OpenMode, > - IN UINT64 Attributes > +Ext4OpenInternal ( > + OUT EXT4_FILE **FoundFile, > + IN EXT4_FILE *Source, > + IN CHAR16 *FileName, > + IN UINT64 OpenMode, > + IN UINT64 Attributes > ) > { > EXT4_FILE *Current; > @@ -180,13 +389,14 @@ Ext4Open ( > CHAR16 PathSegment[EXT4_NAME_MAX + 1]; > UINTN Length; > EXT4_FILE *File; > + CHAR16 *Symlink; > EFI_STATUS Status; > > - Current =3D (EXT4_FILE *)This; > + Current =3D Source; > Partition =3D Current->Partition; > Level =3D 0; > > - DEBUG ((DEBUG_FS, "[ext4] Ext4Open %s\n", FileName)); > + DEBUG ((DEBUG_FS, "[ext4] Ext4OpenInternal %s\n", FileName)); > // If the path starts with a backslash, we treat the root directory as > the base directory > if (FileName[0] =3D=3D L'\\') { > FileName++; > @@ -194,6 +404,11 @@ Ext4Open ( > } > > while (FileName[0] !=3D L'\0') { > + if (Partition->Root-> > > SYMLOOP_MAX) { > + DEBUG ((DEBUG_FS, "[ext4] Symloop limit is hit !\n")); > + return EFI_ACCESS_DENIED; > + } > + > // Discard leading path separators > while (FileName[0] =3D=3D L'\\') { > FileName++; > @@ -238,18 +453,45 @@ Ext4Open ( > } > > // Check if this is a valid file to open in EFI > - > - // What to do with symlinks? They're nonsense when absolute but may > - // be useful when they're relative. Right now, they're ignored, sinc= e > they > - // bring a lot of trouble for something that's not as useful in our > case. > - // If you want to link, use hard links. > - > if (!Ext4FileIsOpenable (File)) { > Ext4CloseInternal (File); > // This looks like an /okay/ status to return. > return EFI_ACCESS_DENIED; > } > > + // > + // Reading symlink and then trying to follow it > + // > + if (Ext4FileIsSymlink (File)) { > + Partition->Root->SymLoops++; > + DEBUG ((DEBUG_FS, "[ext4] File %s is symlink, trying to read it\n"= , > PathSegment)); > + Status =3D Ext4ReadSymlink (Partition, File, &Symlink); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_FS, "[ext4] Error reading %s symlink!\n", > PathSegment)); > + return Status; > + } > + > + DEBUG ((DEBUG_FS, "[ext4] File %s is linked to %s\n", PathSegment, > Symlink)); > + // > + // Close symlink file > + // > + Ext4CloseInternal (File); > + // > + // Open linked file by recursive call of Ext4OpenFile > + // > + Status =3D Ext4OpenInternal (FoundFile, Current, Symlink, OpenMode= , > Attributes); > + FreePool (Symlink); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_FS, "[ext4] Error opening linked file %s\n", > Symlink)); > + return Status; > + } > + > + // > + // Set File to newly opened > + // > + File =3D *FoundFile; > + } > + > if (Level !=3D 0) { > // Careful not to close the base directory > Ext4CloseInternal (Current); > @@ -273,12 +515,75 @@ Ext4Open ( > return EFI_ACCESS_DENIED; > } > > - *NewHandle =3D &Current->Protocol; > + *FoundFile =3D Current; > > DEBUG ((DEBUG_FS, "[ext4] Opened filename %s\n", > Current->Dentry->Name)); > return EFI_SUCCESS; > } > > +/** > + Opens a new file relative to the source file's location. > + @param[in] This A pointer to the EFI_FILE_PROTOCOL instance tha= t > is the file > + handle to the source location. This would > typically be an open > + handle to a directory. > + @param[out] NewHandle A pointer to the location to return the opened > handle for the new > + file. > + @param[in] FileName The Null-terminated string of the name of the > file to be opened. > + The file name may contain the following path > modifiers: "\", ".", > + and "..". > + @param[in] OpenMode The mode to open the file. The only valid > combinations that the > + file may be opened with are: Read, Read/Write, > or Create/Read/Write. > + @param[in] Attributes Only valid for EFI_FILE_MODE_CREATE, in which > case these are the > + attribute bits for the newly created file. > + @retval EFI_SUCCESS The file was opened. > + @retval EFI_NOT_FOUND The specified file could not be found on > the device. > + @retval EFI_NO_MEDIA The device has no medium. > + @retval EFI_MEDIA_CHANGED The device has a different medium in it o= r > the medium is no > + longer supported. > + @retval EFI_DEVICE_ERROR The device reported an error. > + @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted. > + @retval EFI_WRITE_PROTECTED An attempt was made to create a file, or > open a file for write > + when the media is write-protected. > + @retval EFI_ACCESS_DENIED The service denied access to the file. > + @retval EFI_OUT_OF_RESOURCES Not enough resources were available to > open the file. > + @retval EFI_VOLUME_FULL The volume is full. > +**/ > +EFI_STATUS > +EFIAPI > +Ext4Open ( > + IN EFI_FILE_PROTOCOL *This, > + OUT EFI_FILE_PROTOCOL **NewHandle, > + IN CHAR16 *FileName, > + IN UINT64 OpenMode, > + IN UINT64 Attributes > + ) > +{ > + EFI_STATUS Status; > + EXT4_FILE *FoundFile; > + EXT4_FILE *Source; > + > + Source =3D (EXT4_FILE *)This; > + > + // > + // Reset SymLoops counter > + // > + Source->Partition->Root->SymLoops =3D 0; > Keeping SymLoops in the root FILE seems kind of out of place. My idea here (and this was going to be chained with a bunch of improvements to the path walking logic, which is why I never got around to adding symlink support) was to add a nameidata(see Onyx code, or NetBSD's namei(9) manpage)-like structure, as you would in typical UNIX fashion. You would then do your whole path traversal on that structure, which would keep your path walking state, walked_symlinks, etc. > + > + Status =3D Ext4OpenInternal ( > + &FoundFile, > + Source, > + FileName, > + OpenMode, > + Attributes > + ); > + > + if (!EFI_ERROR (Status)) { > + *NewHandle =3D &FoundFile->Protocol; > + } > + > + return Status; > +} > + > /** > Closes a specified file handle. > > @@ -588,7 +893,7 @@ Ext4GetVolumeName ( > > // s_volume_name is only valid on dynamic revision; old filesystems > don't support this > if (Partition->SuperBlock.s_rev_level =3D=3D EXT4_DYNAMIC_REV) { > - CopyMem (TempVolName, (CONST CHAR8 > *)Partition->SuperBlock.s_volume_name, 16); > + CopyMem (TempVolName, Partition->SuperBlock.s_volume_name, 16); > TempVolName[16] =3D '\0'; > > Status =3D UTF8StrToUCS2 (TempVolName, &VolumeName); > @@ -754,12 +1059,14 @@ Ext4GetInfo ( > OUT VOID *Buffer > ) > { > + EXT4_FILE *File; > EXT4_PARTITION *Partition; > > - Partition =3D ((EXT4_FILE *)This)->Partition; > + File =3D (EXT4_FILE *)This; > + Partition =3D File->Partition; > > if (CompareGuid (InformationType, &gEfiFileInfoGuid)) { > - return Ext4GetFileInfo ((EXT4_FILE *)This, Buffer, BufferSize); > + return Ext4GetFileInfo (File, Buffer, BufferSize); > } > > if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) { > @@ -870,12 +1177,12 @@ Ext4SetInfo ( > ) > { > EXT4_FILE *File; > - EXT4_PARTITION *Part; > + EXT4_PARTITION *Partition; > > - File =3D (EXT4_FILE *)This; > - Part =3D File->Partition; > + File =3D (EXT4_FILE *)This; > + Partition =3D File->Partition; > > - if (Part->ReadOnly) { > + if (Partition->ReadOnly) { > return EFI_WRITE_PROTECTED; > } > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c > b/Features/Ext4Pkg/Ext4Dxe/Inode.c > index 831f5946e870..e7a6b3225709 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Inode.c > +++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c > @@ -255,6 +255,59 @@ Ext4FileIsDir ( > return (File->Inode->i_mode & EXT4_INO_TYPE_DIR) =3D=3D EXT4_INO_TYPE_= DIR; > } > > +/** > + Checks if a file is a symlink. > + > + @param[in] File Pointer to the opened file. > + > + @return BOOLEAN Whether file is a symlink > +**/ > +BOOLEAN > +Ext4FileIsSymlink ( > + IN CONST EXT4_FILE *File > + ) > +{ > + return (File->Inode->i_mode & EXT4_INO_TYPE_SYMLINK) =3D=3D > EXT4_INO_TYPE_SYMLINK; > +} > + > +/** > + Detects if a symlink is a fast symlink. > + > + @param[in] File Pointer to the opened file. > + > + @return BOOLEAN Whether symlink is a fast symlink > +**/ > +BOOLEAN > +Ext4SymlinkIsFastSymlink ( > + IN CONST EXT4_FILE *File > + ) > +{ > + // > + // Detection logic of the fast-symlink splits into two behaviors - old > and new. > + // The old behavior is based on comparing the extended attribute block= s > + // with the inode's i_blocks, and if it's zero we know the inode isn't > storing > + // the link in filesystem blocks, so we look to the inode->i_data. > + // The new behavior is apparently needed only with the large EA inode > feature. > + // In this case we check that inode size less than maximum fast symlin= k > size. > + // So, we revert to the old behavior if the large EA inode feature is > not set. > + // > + UINT32 FileAcl; > + UINT32 ExtAttrBlocks; > + > + if ((File->Inode->i_flags & EXT4_EA_INODE_FL) =3D=3D 0) { > + FileAcl =3D File->Inode->i_file_acl; > + if (EXT4_IS_64_BIT (File->Partition)) { > + FileAcl |=3D LShiftU64 > (File->Inode->i_osd2.data_linux.l_i_file_acl_high, 32); > + } > + > + ExtAttrBlocks =3D FileAcl !=3D 0 ? (File->Partition->BlockSize >> 9)= : 0; > + > + return File->Inode->i_blocks =3D=3D ExtAttrBlocks; > + } > + > + return EXT4_INODE_SIZE (File->Inode) <=3D EXT4_FAST_SYMLINK_MAX_SIZE; > +} > + > /** > Checks if a file is a regular file. > @param[in] File Pointer to the opened file. > -- > 2.37.1 > > --=20 Pedro Falcato --0000000000004efec705e768907c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Savva,

Sorry for the huge= delay. Comments inline.

On Thu, Jul 28, 2022 at 4:26 PM Savva Mitrofa= nov <savvamtr@gmail.com> wr= ote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3677<= /a>

Provided support for symlink file type. Added routine which allows
reading and following them through recursive open() call. As a security
meausure implemented simple symlink loop check with nest level limit
equal 8. Also this patch moves Ext4Open functionality to internal
routine.

Cc: Marvin H=C3=A4user <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
=C2=A0Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h |=C2=A0 13 +-
=C2=A0Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h=C2=A0 |=C2=A0 98 +++++-
=C2=A0Features/Ext4Pkg/Ext4Dxe/File.c=C2=A0 =C2=A0 =C2=A0| 359 ++++++++++++= ++++++--
=C2=A0Features/Ext4Pkg/Ext4Dxe/Inode.c=C2=A0 =C2=A0 |=C2=A0 53 +++
=C2=A04 files changed, 485 insertions(+), 38 deletions(-)

diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h b/Features/Ext4Pkg/Ext4Dxe= /Ext4Disk.h
index a55cd2fa68ad..a73e3f8622f1 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
@@ -171,7 +171,7 @@
=C2=A0#define EXT4_DIRTY_FL=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00x00000100
=C2=A0#define EXT4_COMPRBLK_FL=C2=A0 =C2=A0 =C2=A0 0x00000200
=C2=A0#define EXT4_NOCOMPR_FL=C2=A0 =C2=A0 =C2=A0 =C2=A00x00000400
-#define EXT4_ECOMPR_FL=C2=A0 =C2=A0 =C2=A0 =C2=A0 0x00000800
+#define EXT4_ENCRYPT_FL=C2=A0 =C2=A0 =C2=A0 =C2=A00x00000800
=C2=A0#define EXT4_BTREE_FL=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00x00001000
=C2=A0#define EXT4_INDEX_FL=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00x00002000
=C2=A0#define EXT4_JOURNAL_DATA_FL=C2=A0 0x00004000
@@ -332,11 +332,12 @@ STATIC_ASSERT (
=C2=A0 =C2=A0"ext4 block group descriptor struct has incorrect size&qu= ot;
=C2=A0 =C2=A0);

-#define EXT4_DBLOCKS=C2=A0 =C2=A0 =C2=A012
-#define EXT4_IND_BLOCK=C2=A0 =C2=A012
-#define EXT4_DIND_BLOCK=C2=A0 13
-#define EXT4_TIND_BLOCK=C2=A0 14
-#define EXT4_NR_BLOCKS=C2=A0 =C2=A015
+#define EXT4_DBLOCKS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 12
+#define EXT4_IND_BLOCK=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 12<= br> +#define EXT4_DIND_BLOCK=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A013<= br> +#define EXT4_TIND_BLOCK=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A014<= br> +#define EXT4_NR_BLOCKS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 15<= br> +#define EXT4_FAST_SYMLINK_MAX_SIZE=C2=A0 EXT4_NR_BLOCKS * sizeof(UINT32)
=C2=A0#define EXT4_GOOD_OLD_INODE_SIZE=C2=A0 128

diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dxe/= Ext4Dxe.h
index b1508482b0a7..c1df9d1149e4 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -31,7 +31,9 @@

=C2=A0#include "Ext4Disk.h"

+#define SYMLOOP_MAX=C2=A0 =C2=A0 8
I'm scared tha= t this may not be enough. Could we use 40 here as Linux does?
=C2=A0#define EXT4_NAME_MAX=C2=A0 255
+#define EFI_PATH_MAX=C2=A0 =C2=A04096
Don't use E= FI_* in Ext4Dxe code, so prefix this with EXT4_ instead maybe?

=C2=A0#define EXT4_DRIVER_VERSION=C2=A0 0x0000

@@ -324,11 +326,11 @@ number of read bytes.
=C2=A0**/
=C2=A0EFI_STATUS
=C2=A0Ext4Read (
-=C2=A0 IN EXT4_PARTITION=C2=A0 *Partition,
-=C2=A0 IN EXT4_FILE=C2=A0 =C2=A0 =C2=A0 =C2=A0*File,
-=C2=A0 OUT VOID=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*Buffer,
-=C2=A0 IN UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Offset,
-=C2=A0 IN OUT UINTN=C2=A0 =C2=A0 =C2=A0 =C2=A0*Length
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0EXT4_PARTITION=C2=A0 *Partition,
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0EXT4_FILE=C2=A0 =C2=A0 =C2=A0 =C2=A0*File, +=C2=A0 OUT=C2=A0 =C2=A0 VOID=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *Buf= fer,
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Offs= et,
+=C2=A0 IN OUT UINTN=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*Length
=C2=A0 =C2=A0);

=C2=A0/**
@@ -368,6 +370,7 @@ struct _Ext4File {

=C2=A0 =C2=A0UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = OpenMode;
=C2=A0 =C2=A0UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = Position;
+=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 SymLo= ops;

=C2=A0 =C2=A0EXT4_PARTITION=C2=A0 =C2=A0 =C2=A0 =C2=A0 *Partition;

@@ -497,6 +500,45 @@ Ext4SetupFile (
=C2=A0 =C2=A0IN EXT4_PARTITION=C2=A0 *Partition
=C2=A0 =C2=A0);

+/**
+=C2=A0 Opens a new file relative to the source file's location.
+
+=C2=A0 @param[out] FoundFile=C2=A0 A pointer to the location to return the= opened handle for the new
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0file.
+=C2=A0 @param[in]=C2=A0 Source=C2=A0 =C2=A0 =C2=A0A pointer to the EXT4_FI= LE instance that is the file
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0handle to the source location. This would typically be an = open
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0handle to a directory.
+=C2=A0 @param[in]=C2=A0 FileName=C2=A0 =C2=A0The Null-terminated string of= the name of the file to be opened.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0The file name may contain the following path modifiers: &q= uot;\", ".",
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0and "..".
+=C2=A0 @param[in]=C2=A0 OpenMode=C2=A0 =C2=A0The mode to open the file. Th= e only valid combinations that the
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0file may be opened with are: Read, Read/Write, or Create/R= ead/Write.
+=C2=A0 @param[in]=C2=A0 Attributes Only valid for EFI_FILE_MODE_CREATE, in= which case these are the
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0attribute bits for the newly created file.
+
+=C2=A0 @retval EFI_SUCCESS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 The file was = opened.
+=C2=A0 @retval EFI_NOT_FOUND=C2=A0 =C2=A0 =C2=A0 =C2=A0 The specified file= could not be found on the device.
+=C2=A0 @retval EFI_NO_MEDIA=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0The device ha= s no medium.
+=C2=A0 @retval EFI_MEDIA_CHANGED=C2=A0 =C2=A0 The device has a different m= edium in it or the medium is no
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0longer supported.
+=C2=A0 @retval EFI_DEVICE_ERROR=C2=A0 =C2=A0 =C2=A0The device reported an = error.
+=C2=A0 @retval EFI_VOLUME_CORRUPTED The file system structures are corrupt= ed.
+=C2=A0 @retval EFI_WRITE_PROTECTED=C2=A0 An attempt was made to create a f= ile, or open a file for write
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0when the media is write-protected. +=C2=A0 @retval EFI_ACCESS_DENIED=C2=A0 =C2=A0 The service denied access to= the file.
+=C2=A0 @retval EFI_OUT_OF_RESOURCES Not enough resources were available to= open the file.
+=C2=A0 @retval EFI_VOLUME_FULL=C2=A0 =C2=A0 =C2=A0 The volume is full.
+
+**/
+EFI_STATUS
+Ext4OpenInternal (
+=C2=A0 OUT EXT4_FILE=C2=A0 **FoundFile,
First IN para= meters, then OUT parameters.
+=C2=A0 IN=C2=A0 EXT4_FILE=C2=A0 *Source,
+=C2=A0 IN=C2=A0 CHAR16=C2=A0 =C2=A0 =C2=A0*FileName,
+=C2=A0 IN=C2=A0 UINT64=C2=A0 =C2=A0 =C2=A0OpenMode,
+=C2=A0 IN=C2=A0 UINT64=C2=A0 =C2=A0 =C2=A0Attributes
+=C2=A0 );
+
=C2=A0/**
=C2=A0 =C2=A0 Closes a file.

@@ -774,6 +816,30 @@ Ext4FileIsDir (
=C2=A0 =C2=A0IN CONST EXT4_FILE=C2=A0 *File
=C2=A0 =C2=A0);

+/**
+=C2=A0 =C2=A0Checks if a file is a symlink.
+
+=C2=A0 =C2=A0@param[in]=C2=A0 =C2=A0 =C2=A0 File=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 Pointer to the opened file.
+
+=C2=A0 =C2=A0@return BOOLEAN=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Whether file= is a symlink
+**/
+BOOLEAN
+Ext4FileIsSymlink (
+=C2=A0 IN CONST EXT4_FILE=C2=A0 *File
+=C2=A0 );
+
+/**
+=C2=A0 =C2=A0Detects if a symlink is a fast symlink.
+
+=C2=A0 =C2=A0@param[in]=C2=A0 =C2=A0 =C2=A0 File=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 Pointer to the opened file.
+
+=C2=A0 =C2=A0@return BOOLEAN=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Whether syml= ink is a fast symlink
+**/
+BOOLEAN
+Ext4SymlinkIsFastSymlink (
+=C2=A0 IN CONST EXT4_FILE=C2=A0 *File
+=C2=A0 );
Does this need to be in Ext4Dxe.h? This is = pretty symlink specific.
+
=C2=A0/**
=C2=A0 =C2=A0 Checks if a file is a regular file.
=C2=A0 =C2=A0 @param[in]=C2=A0 =C2=A0 =C2=A0 File=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 Pointer to the opened file.
@@ -797,7 +863,7 @@ Ext4FileIsReg (
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 it's a regular file or a dire= ctory, since most other file types
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 don't make sense under UEFI.<= br> =C2=A0**/
-#define Ext4FileIsOpenable(File)=C2=A0 (Ext4FileIsReg(File) || Ext4FileIsD= ir(File))
+#define Ext4FileIsOpenable(File)=C2=A0 (Ext4FileIsReg (File) || Ext4FileIs= Dir (File) || Ext4FileIsSymlink (File))

=C2=A0#define EXT4_INODE_HAS_FIELD(Inode, Field)=C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0\
=C2=A0 =C2=A0(Inode->i_extra_isize + EXT4_GOOD_OLD_INODE_SIZE >=3D=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 \
@@ -935,6 +1001,26 @@ Ext4ReadDir (
=C2=A0 =C2=A0IN OUT UINTN=C2=A0 =C2=A0 =C2=A0 =C2=A0*OutLength
=C2=A0 =C2=A0);

+/**
+=C2=A0 Reads a symlink file.
+
+=C2=A0 @param[in]=C2=A0 =C2=A0 =C2=A0 Partition=C2=A0 =C2=A0Pointer to the= ext4 partition.
+=C2=A0 @param[in]=C2=A0 =C2=A0 =C2=A0 File=C2=A0 =C2=A0 =C2=A0 =C2=A0 Poin= ter to the open symlink file.
+=C2=A0 @param[out]=C2=A0 =C2=A0 =C2=A0Symlink=C2=A0 =C2=A0 =C2=A0Pointer t= o the output unicode symlink string.
+
+=C2=A0 @retval EFI_SUCCESS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Symlink= was read.
+=C2=A0 @retval EFI_ACCESS_DENIED=C2=A0 =C2=A0 =C2=A0Symlink is encrypted.<= br> +=C2=A0 @retval EFI_OUT_OF_RESOURCES=C2=A0 Memory allocation error.
+=C2=A0 @retval EFI_INVALID_PARAMETER Symlink path has incorrect length
+=C2=A0 @retval EFI_VOLUME_CORRUPTED=C2=A0 Symlink read block size differ f= rom inode value
+**/
+EFI_STATUS
+Ext4ReadSymlink (
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0EXT4_PARTITION=C2=A0 *Partition,
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0EXT4_FILE=C2=A0 =C2=A0 =C2=A0 =C2=A0*File, +=C2=A0 OUT=C2=A0 =C2=A0 CHAR16=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 **Symlink=
+=C2=A0 );
+
=C2=A0/**
=C2=A0 =C2=A0 Initialises the (empty) extents map, that will work as a cach= e of extents.

diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/Fil= e.c
index ff1746d5640a..ae9230d6422b 100644
--- a/Features/Ext4Pkg/Ext4Dxe/File.c
+++ b/Features/Ext4Pkg/Ext4Dxe/File.c
@@ -134,14 +134,224 @@ Ext4DirCanLookup (
=C2=A0 =C2=A0return (File->Inode->i_mode & EXT4_INO_PERM_EXEC_OWN= ER) =3D=3D EXT4_INO_PERM_EXEC_OWNER;
=C2=A0}

+/**
+=C2=A0 Reads a fast symlink file.
+
+=C2=A0 @param[in]=C2=A0 =C2=A0 =C2=A0 Partition=C2=A0 =C2=A0Pointer to the= ext4 partition.
+=C2=A0 @param[in]=C2=A0 =C2=A0 =C2=A0 File=C2=A0 =C2=A0 =C2=A0 =C2=A0 Poin= ter to the open symlink file.
+=C2=A0 @param[out]=C2=A0 =C2=A0 =C2=A0AsciiSymlink=C2=A0 =C2=A0 =C2=A0Poin= ter to the output ascii symlink string.
+=C2=A0 @param[out]=C2=A0 =C2=A0 =C2=A0AsciiSymlinkSize Pointer to the outp= ut ascii symlink string length.
+
+=C2=A0 @retval EFI_SUCCESS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Fast s= ymlink was read.
+=C2=A0 @retval EFI_OUT_OF_RESOURCES=C2=A0 =C2=A0Memory allocation error. +**/
+STATIC
+EFI_STATUS
+Ext4ReadFastSymlink (
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0EXT4_PARTITION=C2=A0 *Partition,
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0EXT4_FILE=C2=A0 =C2=A0 =C2=A0 =C2=A0*File, +=C2=A0 OUT=C2=A0 =C2=A0 CHAR8=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0**As= ciiSymlink,
+=C2=A0 OUT=C2=A0 =C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *AsciiSym= linkSize
+=C2=A0 )
+{
+=C2=A0 UINT32=C2=A0 SymlinkSize;
+=C2=A0 CHAR8=C2=A0 =C2=A0*AsciiSymlinkTmp;
+=C2=A0 //
+=C2=A0 // Fast-symlink's EXT4_INODE_SIZE is not necessarily validated = when we checked it in
+=C2=A0 // Ext4SymlinkIsFastSymlink(), so truncate if necessary.
+=C2=A0 //
I think the correct solution here is to che= ck if the inode size fits in EXT4_FAST_SYMLINK_MAX_SIZE in Ext4SymlinkIsFas= tSymlink. Or possibly check it here and error-out (blatant corruption).
Truncating doesn't seem correct.
+=C2=A0 SymlinkSize =3D (UINT32)MIN (EXT4_INODE_SIZE (File->Inode), EXT4= _FAST_SYMLINK_MAX_SIZE);
+
+=C2=A0 AsciiSymlinkTmp =3D AllocatePool (SymlinkSize + 1);
+=C2=A0 if (AsciiSymlinkTmp =3D=3D NULL) {
+=C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] Failed to allocate symlink as= cii string buffer\n"));
Nit: DEBUG_ERROR?
+=C2=A0 =C2=A0 return EFI_OUT_OF_RESOURCES;
+=C2=A0 }
+
+=C2=A0 CopyMem (AsciiSymlinkTmp, File->Inode->i_data, SymlinkSize);<= br> +
+=C2=A0 //
+=C2=A0 // Add null-terminator
+=C2=A0 //
+=C2=A0 AsciiSymlinkTmp[SymlinkSize] =3D '\0';
+
+=C2=A0 *AsciiSymlink=C2=A0 =C2=A0 =C2=A0=3D AsciiSymlinkTmp;
+=C2=A0 *AsciiSymlinkSize =3D SymlinkSize + 1;
+
+=C2=A0 return EFI_SUCCESS;
+}
+
+/**
+=C2=A0 Reads a slow symlink file.
+
+=C2=A0 @param[in]=C2=A0 =C2=A0 =C2=A0 Partition=C2=A0 =C2=A0 =C2=A0 =C2=A0= Pointer to the ext4 partition.
+=C2=A0 @param[in]=C2=A0 =C2=A0 =C2=A0 File=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0Pointer to the open symlink file.
+=C2=A0 @param[out]=C2=A0 =C2=A0 =C2=A0AsciiSymlink=C2=A0 =C2=A0 =C2=A0Poin= ter to the output ascii symlink string.
+=C2=A0 @param[out]=C2=A0 =C2=A0 =C2=A0AsciiSymlinkSize Pointer to the outp= ut ascii symlink string length.
+
+=C2=A0 @retval EFI_SUCCESS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Slow sy= mlink was read.
+=C2=A0 @retval EFI_OUT_OF_RESOURCES=C2=A0 Memory allocation error.
+=C2=A0 @retval EFI_INVALID_PARAMETER Slow symlink path has incorrect lengt= h
+=C2=A0 @retval EFI_VOLUME_CORRUPTED=C2=A0 Symlink read block size differ f= rom inode value
+**/
+STATIC
+EFI_STATUS
+Ext4ReadSlowSymlink (
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0EXT4_PARTITION=C2=A0 *Partition,
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0EXT4_FILE=C2=A0 =C2=A0 =C2=A0 =C2=A0*File, +=C2=A0 OUT=C2=A0 =C2=A0 CHAR8=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0**As= ciiSymlink,
+=C2=A0 OUT=C2=A0 =C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *AsciiSym= linkSize
+=C2=A0 )
+{
+=C2=A0 EFI_STATUS=C2=A0 Status;
+=C2=A0 CHAR8=C2=A0 =C2=A0 =C2=A0 =C2=A0*SymlinkTmp;
+=C2=A0 UINT64=C2=A0 =C2=A0 =C2=A0 SymlinkSizeTmp;
+=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 SymlinkAllocateSize;
+=C2=A0 UINTN=C2=A0 =C2=A0 =C2=A0 =C2=A0ReadSize;
+
+=C2=A0 SymlinkSizeTmp =3D EXT4_INODE_SIZE (File->Inode);
+
+=C2=A0 //
+=C2=A0 // Allocate EXT4_INODE_SIZE + 1
+=C2=A0 //
+=C2=A0 if (SymlinkSizeTmp > EFI_PATH_MAX - 1) {
Wh= y is this > EFI_PATH_MAX - 1 and not >=3D EFI_PATH_MAX? Also, why doe= s the inode size need to be strictly smaller than EFI_PATH_MAX?
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex"> +=C2=A0 =C2=A0 DEBUG ((
+=C2=A0 =C2=A0 =C2=A0 DEBUG_FS,
Nit: DEBUG_ERROR?
=
+=C2=A0 =C2=A0 =C2=A0 "[ext4] Error! Symlink path maximum length was h= it!\n"
+=C2=A0 =C2=A0 =C2=A0 ));
+=C2=A0 =C2=A0 return EFI_INVALID_PARAMETER;
+=C2=A0 }
+
+=C2=A0 SymlinkAllocateSize =3D (UINT32)SymlinkSizeTmp + 1;
+
+=C2=A0 SymlinkTmp =3D AllocatePool (SymlinkAllocateSize);
+=C2=A0 if (SymlinkTmp =3D=3D NULL) {
+=C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] Failed to allocate symlink as= cii string buffer\n"));
+=C2=A0 =C2=A0 return EFI_OUT_OF_RESOURCES;
+=C2=A0 }
+=C2=A0
+=C2=A0 ReadSize =3D (UINTN)SymlinkSizeTmp;
+=C2=A0 Status=C2=A0 =C2=A0=3D Ext4Read (Partition, File, SymlinkTmp, File-= >Position, &ReadSize);
+=C2=A0 if (EFI_ERROR (Status)) {
+=C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] Failed to read symlink from b= locks with Status %r\n", Status));
Nit: lowercase= "status" in the DEBUG.
+=C2=A0 =C2=A0 FreePool (SymlinkTmp);
+=C2=A0 =C2=A0 return Status;
+=C2=A0 }
+
+=C2=A0 File->Position +=3D ReadSize;
Taking Positi= on into account (here and in the Ext4Read call) seems wrong. Why would the = file seek affect the readlink() operation?
+
+=C2=A0 //
+=C2=A0 // Add null-terminator
+=C2=A0 //
+=C2=A0 SymlinkTmp[SymlinkSizeTmp] =3D '\0';
+
+=C2=A0 if (SymlinkSizeTmp !=3D ReadSize) {
+=C2=A0 =C2=A0 DEBUG ((
+=C2=A0 =C2=A0 =C2=A0 DEBUG_FS,
+=C2=A0 =C2=A0 =C2=A0 "[ext4] Error! The sz of the read block doesn= 9;t match the value from the inode!\n"
Nit: size =
+=C2=A0 =C2=A0 =C2=A0 ));
+=C2=A0 =C2=A0 return EFI_VOLUME_CORRUPTED;
+=C2=A0 }
+
+=C2=A0 *AsciiSymlinkSize =3D SymlinkAllocateSize;
+=C2=A0 *AsciiSymlink=C2=A0 =C2=A0 =C2=A0=3D SymlinkTmp;
+
+=C2=A0 return EFI_SUCCESS;
+}
+
+/**
+=C2=A0 Reads a symlink file.
+
+=C2=A0 @param[in]=C2=A0 =C2=A0 =C2=A0 Partition=C2=A0 =C2=A0Pointer to the= ext4 partition.
+=C2=A0 @param[in]=C2=A0 =C2=A0 =C2=A0 File=C2=A0 =C2=A0 =C2=A0 =C2=A0 Poin= ter to the open symlink file.
+=C2=A0 @param[out]=C2=A0 =C2=A0 =C2=A0Symlink=C2=A0 =C2=A0 =C2=A0Pointer t= o the output unicode symlink string.
+
+=C2=A0 @retval EFI_SUCCESS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Symlink= was read.
+=C2=A0 @retval EFI_ACCESS_DENIED=C2=A0 =C2=A0 =C2=A0Symlink is encrypted.<= br> +=C2=A0 @retval EFI_OUT_OF_RESOURCES=C2=A0 Memory allocation error.
+=C2=A0 @retval EFI_INVALID_PARAMETER Symlink path has incorrect length
+=C2=A0 @retval EFI_VOLUME_CORRUPTED=C2=A0 Symlink read block size differ f= rom inode value
+**/
+EFI_STATUS
+Ext4ReadSymlink (
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0EXT4_PARTITION=C2=A0 *Partition,
+=C2=A0 IN=C2=A0 =C2=A0 =C2=A0EXT4_FILE=C2=A0 =C2=A0 =C2=A0 =C2=A0*File, +=C2=A0 OUT=C2=A0 =C2=A0 CHAR16=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 **Symlink=
+=C2=A0 )
+{
+=C2=A0 EFI_STATUS=C2=A0 Status;
+=C2=A0 CHAR8=C2=A0 =C2=A0 =C2=A0 =C2=A0*SymlinkTmp;
+=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 SymlinkSize;
+=C2=A0 CHAR16=C2=A0 =C2=A0 =C2=A0 *Symlink16Tmp;
+=C2=A0 CHAR16=C2=A0 =C2=A0 =C2=A0 *Needle;
+
+=C2=A0 //
+=C2=A0 // Assume that we alread read Inode via Ext4ReadInode
+=C2=A0 // Skip reading, just check encryption flag
+=C2=A0 //
+=C2=A0 if ((File->Inode->i_flags & EXT4_ENCRYPT_FL) !=3D 0) { +=C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] Error, symlink is encrypted\n= "));
Nit: DEBUG_ERROR and "Error: ".
+=C2=A0 =C2=A0 return EFI_ACCESS_DENIED;
+=C2=A0 }
+
+=C2=A0 if (Ext4SymlinkIsFastSymlink (File)) {
+=C2=A0 =C2=A0 Status =3D Ext4ReadFastSymlink (Partition, File, &Symlin= kTmp, &SymlinkSize);
+=C2=A0 } else {
+=C2=A0 =C2=A0 Status =3D Ext4ReadSlowSymlink (Partition, File, &Symlin= kTmp, &SymlinkSize);
+=C2=A0 }
+
+=C2=A0 if (EFI_ERROR (Status)) {
+=C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] Symlink read error with Statu= s %r\n", Status));
+=C2=A0 =C2=A0 return Status;
+=C2=A0 }
+
+=C2=A0 Symlink16Tmp =3D AllocateZeroPool (SymlinkSize * sizeof (CHAR16));<= br> +=C2=A0 if (Symlink16Tmp =3D=3D NULL) {
+=C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] Failed to allocate symlink un= icode string buffer\n"));
+=C2=A0 =C2=A0 FreePool (SymlinkTmp);
+=C2=A0 =C2=A0 return EFI_OUT_OF_RESOURCES;
+=C2=A0 }
+
+=C2=A0 Status =3D AsciiStrToUnicodeStrS (
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0SymlinkTmp,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Symlink16Tmp,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0SymlinkSize
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0);
+
+=C2=A0 if (EFI_ERROR (Status)) {
+=C2=A0 =C2=A0 DEBUG ((
+=C2=A0 =C2=A0 =C2=A0 DEBUG_FS,
+=C2=A0 =C2=A0 =C2=A0 "[ext4] Failed to convert ascii symlink to unico= de with Status %r\n",
+=C2=A0 =C2=A0 =C2=A0 Status
+=C2=A0 =C2=A0 =C2=A0 ));
+=C2=A0 =C2=A0 FreePool (Symlink16Tmp);
+=C2=A0 =C2=A0 FreePool (SymlinkTmp);
+=C2=A0 =C2=A0 return Status;
+=C2=A0 }
+
+=C2=A0 //
+=C2=A0 // Convert to UEFI slashes
+=C2=A0 //
+=C2=A0 for (Needle =3D Symlink16Tmp; *Needle !=3D L'\0'; Needle++)= {
+=C2=A0 =C2=A0 if (*Needle =3D=3D L'/') {
+=C2=A0 =C2=A0 =C2=A0 *Needle =3D L'\\';
+=C2=A0 =C2=A0 }
+=C2=A0 }
+
+=C2=A0 *Symlink =3D Symlink16Tmp;
+
+=C2=A0 FreePool (SymlinkTmp);
+=C2=A0 return Status;
+}
+
=C2=A0/**
=C2=A0 =C2=A0Opens a new file relative to the source file's location.
-=C2=A0 @param[in]=C2=A0 This=C2=A0 =C2=A0 =C2=A0 =C2=A0A pointer to the EF= I_FILE_PROTOCOL instance that is the file
+=C2=A0 @param[out] FoundFile=C2=A0 A pointer to the location to return the= opened handle for the new
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0file.
+=C2=A0 @param[in]=C2=A0 Source=C2=A0 =C2=A0 =C2=A0A pointer to the EXT4_FI= LE instance that is the file
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 handle to the source location. This would typically be an= open
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 handle to a directory.
-=C2=A0 @param[out] NewHandle=C2=A0 A pointer to the location to return the= opened handle for the new
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0file.
=C2=A0 =C2=A0@param[in]=C2=A0 FileName=C2=A0 =C2=A0The Null-terminated stri= ng of the name of the file to be opened.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 The file name may contain the following path modifiers: &= quot;\", ".",
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 and "..".
@@ -165,13 +375,12 @@ Ext4DirCanLookup (

=C2=A0**/
=C2=A0EFI_STATUS
-EFIAPI
-Ext4Open (
-=C2=A0 IN EFI_FILE_PROTOCOL=C2=A0 =C2=A0*This,
-=C2=A0 OUT EFI_FILE_PROTOCOL=C2=A0 **NewHandle,
-=C2=A0 IN CHAR16=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *FileName= ,
-=C2=A0 IN UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 OpenMode,=
-=C2=A0 IN UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Attribute= s
+Ext4OpenInternal (
+=C2=A0 OUT EXT4_FILE=C2=A0 **FoundFile,
+=C2=A0 IN=C2=A0 EXT4_FILE=C2=A0 *Source,
+=C2=A0 IN=C2=A0 CHAR16=C2=A0 =C2=A0 =C2=A0*FileName,
+=C2=A0 IN=C2=A0 UINT64=C2=A0 =C2=A0 =C2=A0OpenMode,
+=C2=A0 IN=C2=A0 UINT64=C2=A0 =C2=A0 =C2=A0Attributes
=C2=A0 =C2=A0)
=C2=A0{
=C2=A0 =C2=A0EXT4_FILE=C2=A0 =C2=A0 =C2=A0 =C2=A0*Current;
@@ -180,13 +389,14 @@ Ext4Open (
=C2=A0 =C2=A0CHAR16=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 PathSegment[EXT4_NAME= _MAX + 1];
=C2=A0 =C2=A0UINTN=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Length;
=C2=A0 =C2=A0EXT4_FILE=C2=A0 =C2=A0 =C2=A0 =C2=A0*File;
+=C2=A0 CHAR16=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *Symlink;
=C2=A0 =C2=A0EFI_STATUS=C2=A0 =C2=A0 =C2=A0 Status;

-=C2=A0 Current=C2=A0 =C2=A0=3D (EXT4_FILE *)This;
+=C2=A0 Current=C2=A0 =C2=A0=3D Source;
=C2=A0 =C2=A0Partition =3D Current->Partition;
=C2=A0 =C2=A0Level=C2=A0 =C2=A0 =C2=A0=3D 0;

-=C2=A0 DEBUG ((DEBUG_FS, "[ext4] Ext4Open %s\n", FileName));
+=C2=A0 DEBUG ((DEBUG_FS, "[ext4] Ext4OpenInternal %s\n", FileNam= e));
=C2=A0 =C2=A0// If the path starts with a backslash, we treat the root dire= ctory as the base directory
=C2=A0 =C2=A0if (FileName[0] =3D=3D L'\\') {
=C2=A0 =C2=A0 =C2=A0FileName++;
@@ -194,6 +404,11 @@ Ext4Open (
=C2=A0 =C2=A0}

=C2=A0 =C2=A0while (FileName[0] !=3D L'\0') {
+=C2=A0 =C2=A0 if (Partition->Root->
=C2=A0
> SYMLOOP_MAX) {
+=C2=A0 =C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] Symloop limit is hit != \n"));
+=C2=A0 =C2=A0 =C2=A0 return EFI_ACCESS_DENIED;
+=C2=A0 =C2=A0 }
+
=C2=A0 =C2=A0 =C2=A0// Discard leading path separators
=C2=A0 =C2=A0 =C2=A0while (FileName[0] =3D=3D L'\\') {
=C2=A0 =C2=A0 =C2=A0 =C2=A0FileName++;
@@ -238,18 +453,45 @@ Ext4Open (
=C2=A0 =C2=A0 =C2=A0}

=C2=A0 =C2=A0 =C2=A0// Check if this is a valid file to open in EFI
-
-=C2=A0 =C2=A0 // What to do with symlinks? They're nonsense when absol= ute but may
-=C2=A0 =C2=A0 // be useful when they're relative. Right now, they'= re ignored, since they
-=C2=A0 =C2=A0 // bring a lot of trouble for something that's not as us= eful in our case.
-=C2=A0 =C2=A0 // If you want to link, use hard links.
-
=C2=A0 =C2=A0 =C2=A0if (!Ext4FileIsOpenable (File)) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0Ext4CloseInternal (File);
=C2=A0 =C2=A0 =C2=A0 =C2=A0// This looks like an /okay/ status to return. =C2=A0 =C2=A0 =C2=A0 =C2=A0return EFI_ACCESS_DENIED;
=C2=A0 =C2=A0 =C2=A0}

+=C2=A0 =C2=A0 //
+=C2=A0 =C2=A0 // Reading symlink and then trying to follow it
+=C2=A0 =C2=A0 //
+=C2=A0 =C2=A0 if (Ext4FileIsSymlink (File)) {
+=C2=A0 =C2=A0 =C2=A0 Partition->Root->SymLoops++;
+=C2=A0 =C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] File %s is symlink, tr= ying to read it\n", PathSegment));
+=C2=A0 =C2=A0 =C2=A0 Status =3D Ext4ReadSymlink (Partition, File, &Sym= link);
+=C2=A0 =C2=A0 =C2=A0 if (EFI_ERROR (Status)) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] Error reading %= s symlink!\n", PathSegment));
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return Status;
+=C2=A0 =C2=A0 =C2=A0 }
+
+=C2=A0 =C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] File %s is linked to %= s\n", PathSegment, Symlink));
+=C2=A0 =C2=A0 =C2=A0 //
+=C2=A0 =C2=A0 =C2=A0 // Close symlink file
+=C2=A0 =C2=A0 =C2=A0 //
+=C2=A0 =C2=A0 =C2=A0 Ext4CloseInternal (File);
+=C2=A0 =C2=A0 =C2=A0 //
+=C2=A0 =C2=A0 =C2=A0 // Open linked file by recursive call of Ext4OpenFile=
+=C2=A0 =C2=A0 =C2=A0 //
+=C2=A0 =C2=A0 =C2=A0 Status =3D Ext4OpenInternal (FoundFile, Current, Syml= ink, OpenMode, Attributes);
+=C2=A0 =C2=A0 =C2=A0 FreePool (Symlink);
+=C2=A0 =C2=A0 =C2=A0 if (EFI_ERROR (Status)) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 DEBUG ((DEBUG_FS, "[ext4] Error opening l= inked file %s\n", Symlink));
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return Status;
+=C2=A0 =C2=A0 =C2=A0 }
+
+=C2=A0 =C2=A0 =C2=A0 //
+=C2=A0 =C2=A0 =C2=A0 // Set File to newly opened
+=C2=A0 =C2=A0 =C2=A0 //
+=C2=A0 =C2=A0 =C2=A0 File =3D *FoundFile;
+=C2=A0 =C2=A0 }
+
=C2=A0 =C2=A0 =C2=A0if (Level !=3D 0) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0// Careful not to close the base directory
=C2=A0 =C2=A0 =C2=A0 =C2=A0Ext4CloseInternal (Current);
@@ -273,12 +515,75 @@ Ext4Open (
=C2=A0 =C2=A0 =C2=A0return EFI_ACCESS_DENIED;
=C2=A0 =C2=A0}

-=C2=A0 *NewHandle =3D &Current->Protocol;
+=C2=A0 *FoundFile =3D Current;

=C2=A0 =C2=A0DEBUG ((DEBUG_FS, "[ext4] Opened filename %s\n", Cur= rent->Dentry->Name));
=C2=A0 =C2=A0return EFI_SUCCESS;
=C2=A0}

+/**
+=C2=A0 Opens a new file relative to the source file's location.
+=C2=A0 @param[in]=C2=A0 This=C2=A0 =C2=A0 =C2=A0 =C2=A0A pointer to the EF= I_FILE_PROTOCOL instance that is the file
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0handle to the source location. This would typically be an = open
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0handle to a directory.
+=C2=A0 @param[out] NewHandle=C2=A0 A pointer to the location to return the= opened handle for the new
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0file.
+=C2=A0 @param[in]=C2=A0 FileName=C2=A0 =C2=A0The Null-terminated string of= the name of the file to be opened.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0The file name may contain the following path modifiers: &q= uot;\", ".",
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0and "..".
+=C2=A0 @param[in]=C2=A0 OpenMode=C2=A0 =C2=A0The mode to open the file. Th= e only valid combinations that the
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0file may be opened with are: Read, Read/Write, or Create/R= ead/Write.
+=C2=A0 @param[in]=C2=A0 Attributes Only valid for EFI_FILE_MODE_CREATE, in= which case these are the
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0attribute bits for the newly created file.
+=C2=A0 @retval EFI_SUCCESS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 The file was = opened.
+=C2=A0 @retval EFI_NOT_FOUND=C2=A0 =C2=A0 =C2=A0 =C2=A0 The specified file= could not be found on the device.
+=C2=A0 @retval EFI_NO_MEDIA=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0The device ha= s no medium.
+=C2=A0 @retval EFI_MEDIA_CHANGED=C2=A0 =C2=A0 The device has a different m= edium in it or the medium is no
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0longer supported.
+=C2=A0 @retval EFI_DEVICE_ERROR=C2=A0 =C2=A0 =C2=A0The device reported an = error.
+=C2=A0 @retval EFI_VOLUME_CORRUPTED The file system structures are corrupt= ed.
+=C2=A0 @retval EFI_WRITE_PROTECTED=C2=A0 An attempt was made to create a f= ile, or open a file for write
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0when the media is write-protected. +=C2=A0 @retval EFI_ACCESS_DENIED=C2=A0 =C2=A0 The service denied access to= the file.
+=C2=A0 @retval EFI_OUT_OF_RESOURCES Not enough resources were available to= open the file.
+=C2=A0 @retval EFI_VOLUME_FULL=C2=A0 =C2=A0 =C2=A0 The volume is full.
+**/
+EFI_STATUS
+EFIAPI
+Ext4Open (
+=C2=A0 IN EFI_FILE_PROTOCOL=C2=A0 =C2=A0*This,
+=C2=A0 OUT EFI_FILE_PROTOCOL=C2=A0 **NewHandle,
+=C2=A0 IN CHAR16=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *FileName= ,
+=C2=A0 IN UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 OpenMode,=
+=C2=A0 IN UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Attribute= s
+=C2=A0 )
+{
+=C2=A0 EFI_STATUS=C2=A0 Status;
+=C2=A0 EXT4_FILE=C2=A0 =C2=A0*FoundFile;
+=C2=A0 EXT4_FILE=C2=A0 =C2=A0*Source;
+
+=C2=A0 Source =3D (EXT4_FILE *)This;
+
+=C2=A0 //
+=C2=A0 // Reset SymLoops counter
+=C2=A0 //
+=C2=A0 Source->Partition->Root->SymLoops =3D 0;
<= div>Keeping SymLoops in the root FILE seems kind of out of place. My idea h= ere (and this was going to be chained with a bunch of improvements to the p= ath walking logic, which is why I never got around to adding symlink suppor= t) was to add a nameidata(see Onyx code, or NetBSD's namei(9) manpage)-= like structure, as you would in typical UNIX fashion. You would then do you= r whole path traversal on that structure, which would keep your path walkin= g state, walked_symlinks, etc.
+
+=C2=A0 Status =3D Ext4OpenInternal (
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&FoundFile,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Source,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0FileName,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0OpenMode,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Attributes
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0);
+
+=C2=A0 if (!EFI_ERROR (Status)) {
+=C2=A0 =C2=A0 *NewHandle =3D &FoundFile->Protocol;
+=C2=A0 }
+
+=C2=A0 return Status;
+}
+
=C2=A0/**
=C2=A0 =C2=A0Closes a specified file handle.

@@ -588,7 +893,7 @@ Ext4GetVolumeName (

=C2=A0 =C2=A0// s_volume_name is only valid on dynamic revision; old filesy= stems don't support this
=C2=A0 =C2=A0if (Partition->SuperBlock.s_rev_level =3D=3D EXT4_DYNAMIC_R= EV) {
-=C2=A0 =C2=A0 CopyMem (TempVolName, (CONST CHAR8 *)Partition->SuperBloc= k.s_volume_name, 16);
+=C2=A0 =C2=A0 CopyMem (TempVolName, Partition->SuperBlock.s_volume_name= , 16);
=C2=A0 =C2=A0 =C2=A0TempVolName[16] =3D '\0';

=C2=A0 =C2=A0 =C2=A0Status =3D UTF8StrToUCS2 (TempVolName, &VolumeName)= ;
@@ -754,12 +1059,14 @@ Ext4GetInfo (
=C2=A0 =C2=A0OUT VOID=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *Buff= er
=C2=A0 =C2=A0)
=C2=A0{
+=C2=A0 EXT4_FILE=C2=A0 =C2=A0 =C2=A0 =C2=A0*File;
=C2=A0 =C2=A0EXT4_PARTITION=C2=A0 *Partition;

-=C2=A0 Partition =3D ((EXT4_FILE *)This)->Partition;
+=C2=A0 File=C2=A0 =C2=A0 =C2=A0 =3D (EXT4_FILE *)This;
+=C2=A0 Partition =3D File->Partition;

=C2=A0 =C2=A0if (CompareGuid (InformationType, &gEfiFileInfoGuid)) { -=C2=A0 =C2=A0 return Ext4GetFileInfo ((EXT4_FILE *)This, Buffer, BufferSiz= e);
+=C2=A0 =C2=A0 return Ext4GetFileInfo (File, Buffer, BufferSize);
=C2=A0 =C2=A0}

=C2=A0 =C2=A0if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)= ) {
@@ -870,12 +1177,12 @@ Ext4SetInfo (
=C2=A0 =C2=A0)
=C2=A0{
=C2=A0 =C2=A0EXT4_FILE=C2=A0 =C2=A0 =C2=A0 =C2=A0*File;
-=C2=A0 EXT4_PARTITION=C2=A0 *Part;
+=C2=A0 EXT4_PARTITION=C2=A0 *Partition;

-=C2=A0 File =3D (EXT4_FILE *)This;
-=C2=A0 Part =3D File->Partition;
+=C2=A0 File=C2=A0 =C2=A0 =C2=A0 =3D (EXT4_FILE *)This;
+=C2=A0 Partition =3D File->Partition;

-=C2=A0 if (Part->ReadOnly) {
+=C2=A0 if (Partition->ReadOnly) {
=C2=A0 =C2=A0 =C2=A0return EFI_WRITE_PROTECTED;
=C2=A0 =C2=A0}

diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c b/Features/Ext4Pkg/Ext4Dxe/In= ode.c
index 831f5946e870..e7a6b3225709 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Inode.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c
@@ -255,6 +255,59 @@ Ext4FileIsDir (
=C2=A0 =C2=A0return (File->Inode->i_mode & EXT4_INO_TYPE_DIR) =3D= =3D EXT4_INO_TYPE_DIR;
=C2=A0}

+/**
+=C2=A0 =C2=A0Checks if a file is a symlink.
+
+=C2=A0 =C2=A0@param[in]=C2=A0 =C2=A0 =C2=A0 File=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 Pointer to the opened file.
+
+=C2=A0 =C2=A0@return BOOLEAN=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Whether file= is a symlink
+**/
+BOOLEAN
+Ext4FileIsSymlink (
+=C2=A0 IN CONST EXT4_FILE=C2=A0 *File
+=C2=A0 )
+{
+=C2=A0 return (File->Inode->i_mode & EXT4_INO_TYPE_SYMLINK) =3D= =3D EXT4_INO_TYPE_SYMLINK;
+}
+
+/**
+=C2=A0 =C2=A0Detects if a symlink is a fast symlink.
+
+=C2=A0 =C2=A0@param[in]=C2=A0 =C2=A0 =C2=A0 File=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 Pointer to the opened file.
+
+=C2=A0 =C2=A0@return BOOLEAN=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Whether syml= ink is a fast symlink
+**/
+BOOLEAN
+Ext4SymlinkIsFastSymlink (
+=C2=A0 IN CONST EXT4_FILE=C2=A0 *File
+=C2=A0 )
+{
+=C2=A0 //
+=C2=A0 // Detection logic of the fast-symlink splits into two behaviors - = old and new.
+=C2=A0 // The old behavior is based on comparing the extended attribute bl= ocks
+=C2=A0 // with the inode's i_blocks, and if it's zero we know the = inode isn't storing
+=C2=A0 // the link in filesystem blocks, so we look to the inode->i_dat= a.
+=C2=A0 // The new behavior is apparently needed only with the large EA ino= de feature.
+=C2=A0 // In this case we check that inode size less than maximum fast sym= link size.
+=C2=A0 // So, we revert to the old behavior if the large EA inode feature = is not set.
+=C2=A0 //
+=C2=A0 UINT32=C2=A0 FileAcl;
+=C2=A0 UINT32=C2=A0 ExtAttrBlocks;
+
+=C2=A0 if ((File->Inode->i_flags & EXT4_EA_INODE_FL) =3D=3D 0) {=
+=C2=A0 =C2=A0 FileAcl =3D File->Inode->i_file_acl;
+=C2=A0 =C2=A0 if (EXT4_IS_64_BIT (File->Partition)) {
+=C2=A0 =C2=A0 =C2=A0 FileAcl |=3D LShiftU64 (File->Inode->i_osd2.dat= a_linux.l_i_file_acl_high, 32);
+=C2=A0 =C2=A0 }
+
+=C2=A0 =C2=A0 ExtAttrBlocks =3D FileAcl !=3D 0 ? (File->Partition->B= lockSize >> 9) : 0;
+
+=C2=A0 =C2=A0 return File->Inode->i_blocks =3D=3D ExtAttrBlocks;
+=C2=A0 }
+
+=C2=A0 return EXT4_INODE_SIZE (File->Inode) <=3D EXT4_FAST_SYMLINK_M= AX_SIZE;
+}
+
=C2=A0/**
=C2=A0 =C2=A0 Checks if a file is a regular file.
=C2=A0 =C2=A0 @param[in]=C2=A0 =C2=A0 =C2=A0 File=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 Pointer to the opened file.
--
2.37.1



--
Pedro Falcato
--0000000000004efec705e768907c--