public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support
@ 2022-09-07 14:02 Savva Mitrofanov
  2022-09-07 14:02 ` [edk2-platforms][PATCH v5 1/2] Ext4Pkg: Add symbolic links support Savva Mitrofanov
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Savva Mitrofanov @ 2022-09-07 14:02 UTC (permalink / raw)
  To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3677

Hi all,

In the fifth version I corrected remarks from Pedro Falcato and Marvin Häuser.
I moved symlink procedures into independent C file, corrected code style like
debug messages macros and etc.

This patchset adds symbolic links support with simple recursion protection based
on symbolic link nest level limitation, also I included patch which adds BASE_CR
to extract EXT4_FILE private structure to prevent possible code corruption caused
by structure changes and rearrangements in future.

REF: https://github.com/savvamitrofanov/edk2-platforms/tree/ext4pkg_symlink_support

Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>

Savva Mitrofanov (2):
  Ext4Pkg: Add symbolic links support
  Ext4Pkg: Add base containing record macro for EXT4_FILE

 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf |   1 +
 Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h  |  13 +-
 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h   |  74 +++++-
 Features/Ext4Pkg/Ext4Dxe/File.c      | 179 +++++++++++---
 Features/Ext4Pkg/Ext4Dxe/Inode.c     |  15 ++
 Features/Ext4Pkg/Ext4Dxe/Symlink.c   | 261 ++++++++++++++++++++
 6 files changed, 499 insertions(+), 44 deletions(-)
 create mode 100644 Features/Ext4Pkg/Ext4Dxe/Symlink.c

-- 
2.37.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [edk2-platforms][PATCH v5 1/2] Ext4Pkg: Add symbolic links support
  2022-09-07 14:02 [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support Savva Mitrofanov
@ 2022-09-07 14:02 ` Savva Mitrofanov
  2022-09-07 14:02 ` [edk2-platforms][PATCH v5 2/2] Ext4Pkg: Add base containing record macro for EXT4_FILE Savva Mitrofanov
  2022-09-15 21:00 ` [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support Pedro Falcato
  2 siblings, 0 replies; 5+ messages in thread
From: Savva Mitrofanov @ 2022-09-07 14:02 UTC (permalink / raw)
  To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3677

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äuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
---
 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf |   1 +
 Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h  |  13 +-
 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h   |  72 +++++-
 Features/Ext4Pkg/Ext4Dxe/File.c      | 169 +++++++++++--
 Features/Ext4Pkg/Ext4Dxe/Inode.c     |  15 ++
 Features/Ext4Pkg/Ext4Dxe/Symlink.c   | 261 ++++++++++++++++++++
 6 files changed, 492 insertions(+), 39 deletions(-)

diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
index deaf89fb3743..a153fc41ccd6 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
@@ -108,6 +108,7 @@
   Directory.c
   Extents.c
   File.c
+  Symlink.c
   Collation.c
   Ext4Disk.h
   Ext4Dxe.h
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
index 7a19d2f79d53..4fd91a423324 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  128U
 
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
index 81e59a4babc9..6d352d3995f1 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -31,8 +31,14 @@
 
 #include "Ext4Disk.h"
 
+#define SYMLOOP_MAX    8
 #define EXT4_NAME_MAX  255
-
+//
+// 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,
+// but present in UNIX distros and shell environments, which may varies from 1024 to 4096.
+//
+#define EXT4_EFI_PATH_MAX    4096
 #define EXT4_DRIVER_VERSION  0x0000
 
 /**
@@ -324,11 +330,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 +374,7 @@ struct _Ext4File {
 
   UINT64                OpenMode;
   UINT64                Position;
+  UINT32                SymLoops;
 
   EXT4_PARTITION        *Partition;
 
@@ -497,6 +504,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 or 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,
+  IN  EXT4_FILE  *Source,
+  IN  CHAR16     *FileName,
+  IN  UINT64     OpenMode,
+  IN  UINT64     Attributes
+  );
+
 /**
    Closes a file.
 
@@ -774,6 +820,18 @@ 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
+  );
+
 /**
    Checks if a file is a regular file.
    @param[in]      File          Pointer to the opened file.
@@ -797,7 +855,7 @@ Ext4FileIsReg (
            it's a regular file or a directory, since most other file types
            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 >=                          \
diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
index ff1746d5640a..86ccfff8603a 100644
--- a/Features/Ext4Pkg/Ext4Dxe/File.c
+++ b/Features/Ext4Pkg/Ext4Dxe/File.c
@@ -9,6 +9,26 @@
 
 #include <Library/BaseUcs2Utf8Lib.h>
 
+/**
+  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 inode value
+**/
+EFI_STATUS
+Ext4ReadSymlink (
+  IN     EXT4_PARTITION  *Partition,
+  IN     EXT4_FILE       *File,
+  OUT    CHAR16          **Symlink
+  );
+
 /**
    Duplicates a file structure.
 
@@ -137,11 +157,11 @@ Ext4DirCanLookup (
 /**
   Opens a new file relative to the source file's location.
 
-  @param[in]  This       A pointer to the EFI_FILE_PROTOCOL instance that 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 +185,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 +199,14 @@ Ext4Open (
   CHAR16          PathSegment[EXT4_NAME_MAX + 1];
   UINTN           Length;
   EXT4_FILE       *File;
+  CHAR16          *Symlink;
   EFI_STATUS      Status;
 
-  Current   = (EXT4_FILE *)This;
+  Current   = Source;
   Partition = Current->Partition;
   Level     = 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] == L'\\') {
     FileName++;
@@ -194,6 +214,11 @@ Ext4Open (
   }
 
   while (FileName[0] != L'\0') {
+    if (Partition->Root->SymLoops > SYMLOOP_MAX) {
+      DEBUG ((DEBUG_FS, "[ext4] Symloop limit is hit !\n"));
+      return EFI_ACCESS_DENIED;
+    }
+
     // Discard leading path separators
     while (FileName[0] == L'\\') {
       FileName++;
@@ -238,18 +263,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, since 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 = 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 = 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 = *FoundFile;
+    }
+
     if (Level != 0) {
       // Careful not to close the base directory
       Ext4CloseInternal (Current);
@@ -273,12 +325,75 @@ Ext4Open (
     return EFI_ACCESS_DENIED;
   }
 
-  *NewHandle = &Current->Protocol;
+  *FoundFile = 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 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 "..".
+  @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 or 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 = (EXT4_FILE *)This;
+
+  //
+  // Reset SymLoops counter
+  //
+  Source->Partition->Root->SymLoops = 0;
+
+  Status = Ext4OpenInternal (
+             &FoundFile,
+             Source,
+             FileName,
+             OpenMode,
+             Attributes
+             );
+
+  if (!EFI_ERROR (Status)) {
+    *NewHandle = &FoundFile->Protocol;
+  }
+
+  return Status;
+}
+
 /**
   Closes a specified file handle.
 
@@ -588,7 +703,7 @@ Ext4GetVolumeName (
 
   // s_volume_name is only valid on dynamic revision; old filesystems don't support this
   if (Partition->SuperBlock.s_rev_level == EXT4_DYNAMIC_REV) {
-    CopyMem (TempVolName, (CONST CHAR8 *)Partition->SuperBlock.s_volume_name, 16);
+    CopyMem (TempVolName, Partition->SuperBlock.s_volume_name, 16);
     TempVolName[16] = '\0';
 
     Status = UTF8StrToUCS2 (TempVolName, &VolumeName);
@@ -754,12 +869,14 @@ Ext4GetInfo (
   OUT VOID              *Buffer
   )
 {
+  EXT4_FILE       *File;
   EXT4_PARTITION  *Partition;
 
-  Partition = ((EXT4_FILE *)This)->Partition;
+  File      = (EXT4_FILE *)This;
+  Partition = File->Partition;
 
   if (CompareGuid (InformationType, &gEfiFileInfoGuid)) {
-    return Ext4GetFileInfo ((EXT4_FILE *)This, Buffer, BufferSize);
+    return Ext4GetFileInfo (File, Buffer, BufferSize);
   }
 
   if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
@@ -870,12 +987,12 @@ Ext4SetInfo (
   )
 {
   EXT4_FILE       *File;
-  EXT4_PARTITION  *Part;
+  EXT4_PARTITION  *Partition;
 
-  File = (EXT4_FILE *)This;
-  Part = File->Partition;
+  File      = (EXT4_FILE *)This;
+  Partition = 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 7f8be2f02643..5ccb4d2bfc42 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Inode.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c
@@ -255,6 +255,21 @@ Ext4FileIsDir (
   return (File->Inode->i_mode & EXT4_INO_TYPE_DIR) == 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) == EXT4_INO_TYPE_SYMLINK;
+}
+
 /**
    Checks if a file is a regular file.
    @param[in]      File          Pointer to the opened file.
diff --git a/Features/Ext4Pkg/Ext4Dxe/Symlink.c b/Features/Ext4Pkg/Ext4Dxe/Symlink.c
new file mode 100644
index 000000000000..0905417ffb88
--- /dev/null
+++ b/Features/Ext4Pkg/Ext4Dxe/Symlink.c
@@ -0,0 +1,261 @@
+/** @file
+  Symbolic links routines
+
+  Copyright (c) 2022 Savva Mitrofanov All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "Ext4Dxe.h"
+
+/**
+   Detects if a symlink is a fast symlink.
+
+   @param[in]      File          Pointer to the opened file.
+
+   @return BOOLEAN         Whether symlink is a fast symlink
+**/
+STATIC
+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 blocks
+  // 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 symlink 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) == 0) {
+    FileAcl = File->Inode->i_file_acl;
+    if (EXT4_IS_64_BIT (File->Partition)) {
+      //
+      // We don't care about final value, we are just checking for any bit is set
+      // so, thats why we neglect LShiftU64(.., 32)
+      //
+      FileAcl |= File->Inode->i_osd2.data_linux.l_i_file_acl_high;
+    }
+
+    ExtAttrBlocks = FileAcl != 0 ? (File->Partition->BlockSize >> 9) : 0;
+
+    return File->Inode->i_blocks == ExtAttrBlocks;
+  }
+
+  return EXT4_INODE_SIZE (File->Inode) <= EXT4_FAST_SYMLINK_MAX_SIZE;
+}
+
+/**
+  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.
+  //
+  SymlinkSize = (UINT32)MIN (EXT4_INODE_SIZE (File->Inode), EXT4_FAST_SYMLINK_MAX_SIZE);
+
+  AsciiSymlinkTmp = AllocatePool (SymlinkSize + 1);
+  if (AsciiSymlinkTmp == NULL) {
+    DEBUG ((DEBUG_ERROR, "[ext4] Failed to allocate symlink ascii string buffer\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  CopyMem (AsciiSymlinkTmp, File->Inode->i_data, SymlinkSize);
+
+  //
+  // Add null-terminator
+  //
+  AsciiSymlinkTmp[SymlinkSize] = '\0';
+
+  *AsciiSymlink     = AsciiSymlinkTmp;
+  *AsciiSymlinkSize = 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 inode 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 = EXT4_INODE_SIZE (File->Inode);
+
+  //
+  // Allocate EXT4_INODE_SIZE + 1
+  //
+  if (SymlinkSizeTmp >= EXT4_EFI_PATH_MAX) {
+    DEBUG ((
+      DEBUG_WARN,
+      "[ext4] Warn: symlink path maximum length was hit!\n"
+      ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  SymlinkAllocateSize = (UINT32)SymlinkSizeTmp + 1;
+
+  SymlinkTmp = AllocatePool (SymlinkAllocateSize);
+  if (SymlinkTmp == NULL) {
+    DEBUG ((DEBUG_FS, "[ext4] Failed to allocate symlink ascii string buffer\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  ReadSize = (UINTN)SymlinkSizeTmp;
+  Status   = 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));
+    FreePool (SymlinkTmp);
+    return Status;
+  }
+
+  //
+  // Add null-terminator
+  //
+  SymlinkTmp[SymlinkSizeTmp] = '\0';
+
+  if (SymlinkSizeTmp != ReadSize) {
+    DEBUG ((
+      DEBUG_FS,
+      "[ext4] Error! The size of the read block doesn't match the value from the inode!\n"
+      ));
+    return EFI_VOLUME_CORRUPTED;
+  }
+
+  *AsciiSymlinkSize = SymlinkAllocateSize;
+  *AsciiSymlink     = 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 inode 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) != 0) {
+    DEBUG ((DEBUG_WARN, "[ext4] Warn: symlink is encrypted\n"));
+    return EFI_ACCESS_DENIED;
+  }
+
+  if (Ext4SymlinkIsFastSymlink (File)) {
+    Status = Ext4ReadFastSymlink (Partition, File, &SymlinkTmp, &SymlinkSize);
+  } else {
+    Status = Ext4ReadSlowSymlink (Partition, File, &SymlinkTmp, &SymlinkSize);
+  }
+
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_FS, "[ext4] Symlink read error with Status %r\n", Status));
+    return Status;
+  }
+
+  Symlink16Tmp = AllocatePool (SymlinkSize * sizeof (CHAR16));
+  if (Symlink16Tmp == NULL) {
+    DEBUG ((DEBUG_FS, "[ext4] Failed to allocate symlink unicode string buffer\n"));
+    FreePool (SymlinkTmp);
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  Status = AsciiStrToUnicodeStrS (
+             SymlinkTmp,
+             Symlink16Tmp,
+             SymlinkSize
+             );
+
+  FreePool (SymlinkTmp);
+
+  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 = Symlink16Tmp; *Needle != L'\0'; Needle++) {
+    if (*Needle == L'/') {
+      *Needle = L'\\';
+    }
+  }
+
+  *Symlink = Symlink16Tmp;
+
+  return Status;
+}
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [edk2-platforms][PATCH v5 2/2] Ext4Pkg: Add base containing record macro for EXT4_FILE
  2022-09-07 14:02 [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support Savva Mitrofanov
  2022-09-07 14:02 ` [edk2-platforms][PATCH v5 1/2] Ext4Pkg: Add symbolic links support Savva Mitrofanov
@ 2022-09-07 14:02 ` Savva Mitrofanov
  2022-09-15 21:00 ` [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support Pedro Falcato
  2 siblings, 0 replies; 5+ messages in thread
From: Savva Mitrofanov @ 2022-09-07 14:02 UTC (permalink / raw)
  To: devel; +Cc: Marvin Häuser, Pedro Falcato, Vitaly Cheptsov

We shouldn't use direct casts, because in the future it could break
the code, so using BASE_CR would be safe against possible structure
changes and rearrangements

Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>
---
 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h |  2 ++
 Features/Ext4Pkg/Ext4Dxe/File.c    | 16 ++++++++--------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
index 6d352d3995f1..adf3c13f6ea9 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -386,6 +386,8 @@ struct _Ext4File {
   EXT4_DENTRY           *Dentry;
 };
 
+#define EXT4_FILE_FROM_THIS(This)  BASE_CR ((This), EXT4_FILE, Protocol)
+
 #define EXT4_FILE_FROM_OPEN_FILES_NODE(Node)                                   \
   BASE_CR(Node, EXT4_FILE, OpenFilesListNode)
 
diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
index 86ccfff8603a..04198a53bfc0 100644
--- a/Features/Ext4Pkg/Ext4Dxe/File.c
+++ b/Features/Ext4Pkg/Ext4Dxe/File.c
@@ -372,7 +372,7 @@ Ext4Open (
   EXT4_FILE   *FoundFile;
   EXT4_FILE   *Source;
 
-  Source = (EXT4_FILE *)This;
+  Source = EXT4_FILE_FROM_THIS (This);
 
   //
   // Reset SymLoops counter
@@ -409,7 +409,7 @@ Ext4Close (
   IN EFI_FILE_PROTOCOL  *This
   )
 {
-  return Ext4CloseInternal ((EXT4_FILE *)This);
+  return Ext4CloseInternal (EXT4_FILE_FROM_THIS (This));
 }
 
 /**
@@ -490,7 +490,7 @@ Ext4ReadFile (
   EXT4_PARTITION  *Partition;
   EFI_STATUS      Status;
 
-  File      = (EXT4_FILE *)This;
+  File      = EXT4_FILE_FROM_THIS (This);
   Partition = File->Partition;
 
   ASSERT (Ext4FileIsOpenable (File));
@@ -541,7 +541,7 @@ Ext4WriteFile (
 {
   EXT4_FILE  *File;
 
-  File = (EXT4_FILE *)This;
+  File = EXT4_FILE_FROM_THIS (This);
 
   if (!(File->OpenMode & EFI_FILE_MODE_WRITE)) {
     return EFI_ACCESS_DENIED;
@@ -571,7 +571,7 @@ Ext4GetPosition (
 {
   EXT4_FILE  *File;
 
-  File = (EXT4_FILE *)This;
+  File = EXT4_FILE_FROM_THIS (This);
 
   if (Ext4FileIsDir (File)) {
     return EFI_UNSUPPORTED;
@@ -604,7 +604,7 @@ Ext4SetPosition (
 {
   EXT4_FILE  *File;
 
-  File = (EXT4_FILE *)This;
+  File = EXT4_FILE_FROM_THIS (This);
 
   // Only seeks to 0 (so it resets the ReadDir operation) are allowed
   if (Ext4FileIsDir (File) && (Position != 0)) {
@@ -872,7 +872,7 @@ Ext4GetInfo (
   EXT4_FILE       *File;
   EXT4_PARTITION  *Partition;
 
-  File      = (EXT4_FILE *)This;
+  File      = EXT4_FILE_FROM_THIS (This);
   Partition = File->Partition;
 
   if (CompareGuid (InformationType, &gEfiFileInfoGuid)) {
@@ -989,7 +989,7 @@ Ext4SetInfo (
   EXT4_FILE       *File;
   EXT4_PARTITION  *Partition;
 
-  File      = (EXT4_FILE *)This;
+  File      = EXT4_FILE_FROM_THIS (This);
   Partition = File->Partition;
 
   if (Partition->ReadOnly) {
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support
  2022-09-07 14:02 [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support Savva Mitrofanov
  2022-09-07 14:02 ` [edk2-platforms][PATCH v5 1/2] Ext4Pkg: Add symbolic links support Savva Mitrofanov
  2022-09-07 14:02 ` [edk2-platforms][PATCH v5 2/2] Ext4Pkg: Add base containing record macro for EXT4_FILE Savva Mitrofanov
@ 2022-09-15 21:00 ` Pedro Falcato
  2022-09-15 21:02   ` Pedro Falcato
  2 siblings, 1 reply; 5+ messages in thread
From: Pedro Falcato @ 2022-09-15 21:00 UTC (permalink / raw)
  To: Savva Mitrofanov; +Cc: devel, Marvin Häuser, Vitaly Cheptsov

[-- Attachment #1: Type: text/plain, Size: 1602 bytes --]

Series-Reviewed-by: Pedro Falcato <pedro.falcato@gmail.com>

On Wed, Sep 7, 2022 at 3:02 PM Savva Mitrofanov <savvamtr@gmail.com> wrote:

> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3677
>
> Hi all,
>
> In the fifth version I corrected remarks from Pedro Falcato and Marvin
> Häuser.
> I moved symlink procedures into independent C file, corrected code style
> like
> debug messages macros and etc.
>
> This patchset adds symbolic links support with simple recursion protection
> based
> on symbolic link nest level limitation, also I included patch which adds
> BASE_CR
> to extract EXT4_FILE private structure to prevent possible code corruption
> caused
> by structure changes and rearrangements in future.
>
> REF:
> https://github.com/savvamitrofanov/edk2-platforms/tree/ext4pkg_symlink_support
>
> Cc: Marvin Häuser <mhaeuser@posteo.de>
> Cc: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
>
> Savva Mitrofanov (2):
>   Ext4Pkg: Add symbolic links support
>   Ext4Pkg: Add base containing record macro for EXT4_FILE
>
>  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf |   1 +
>  Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h  |  13 +-
>  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h   |  74 +++++-
>  Features/Ext4Pkg/Ext4Dxe/File.c      | 179 +++++++++++---
>  Features/Ext4Pkg/Ext4Dxe/Inode.c     |  15 ++
>  Features/Ext4Pkg/Ext4Dxe/Symlink.c   | 261 ++++++++++++++++++++
>  6 files changed, 499 insertions(+), 44 deletions(-)
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Symlink.c
>
> --
> 2.37.3
>
>

-- 
Pedro Falcato

[-- Attachment #2: Type: text/html, Size: 2520 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support
  2022-09-15 21:00 ` [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support Pedro Falcato
@ 2022-09-15 21:02   ` Pedro Falcato
  0 siblings, 0 replies; 5+ messages in thread
From: Pedro Falcato @ 2022-09-15 21:02 UTC (permalink / raw)
  To: Savva Mitrofanov
  Cc: edk2-devel-groups-io, Marvin Häuser, Vitaly Cheptsov

[-- Attachment #1: Type: text/plain, Size: 1811 bytes --]

Pushed as e81432f and 76fc44f.
Thanks!

On Thu, Sep 15, 2022 at 10:00 PM Pedro Falcato <pedro.falcato@gmail.com>
wrote:

> Series-Reviewed-by: Pedro Falcato <pedro.falcato@gmail.com>
>
> On Wed, Sep 7, 2022 at 3:02 PM Savva Mitrofanov <savvamtr@gmail.com>
> wrote:
>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3677
>>
>> Hi all,
>>
>> In the fifth version I corrected remarks from Pedro Falcato and Marvin
>> Häuser.
>> I moved symlink procedures into independent C file, corrected code style
>> like
>> debug messages macros and etc.
>>
>> This patchset adds symbolic links support with simple recursion
>> protection based
>> on symbolic link nest level limitation, also I included patch which adds
>> BASE_CR
>> to extract EXT4_FILE private structure to prevent possible code
>> corruption caused
>> by structure changes and rearrangements in future.
>>
>> REF:
>> https://github.com/savvamitrofanov/edk2-platforms/tree/ext4pkg_symlink_support
>>
>> Cc: Marvin Häuser <mhaeuser@posteo.de>
>> Cc: Pedro Falcato <pedro.falcato@gmail.com>
>> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
>>
>> Savva Mitrofanov (2):
>>   Ext4Pkg: Add symbolic links support
>>   Ext4Pkg: Add base containing record macro for EXT4_FILE
>>
>>  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf |   1 +
>>  Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h  |  13 +-
>>  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h   |  74 +++++-
>>  Features/Ext4Pkg/Ext4Dxe/File.c      | 179 +++++++++++---
>>  Features/Ext4Pkg/Ext4Dxe/Inode.c     |  15 ++
>>  Features/Ext4Pkg/Ext4Dxe/Symlink.c   | 261 ++++++++++++++++++++
>>  6 files changed, 499 insertions(+), 44 deletions(-)
>>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Symlink.c
>>
>> --
>> 2.37.3
>>
>>
>
> --
> Pedro Falcato
>


-- 
Pedro Falcato

[-- Attachment #2: Type: text/html, Size: 3066 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-09-15 21:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-07 14:02 [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support Savva Mitrofanov
2022-09-07 14:02 ` [edk2-platforms][PATCH v5 1/2] Ext4Pkg: Add symbolic links support Savva Mitrofanov
2022-09-07 14:02 ` [edk2-platforms][PATCH v5 2/2] Ext4Pkg: Add base containing record macro for EXT4_FILE Savva Mitrofanov
2022-09-15 21:00 ` [edk2-platforms][PATCH v5 0/2] Ext4Pkg: Add Symbolic Links support Pedro Falcato
2022-09-15 21:02   ` Pedro Falcato

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox