[edk2-devel] [edk2-platforms][PATCH v1 06/12] Ext4Pkg: Add comparison between Position and FileSize in Ext4SetPosition

[Savva Mitrofanov]

Missing such comparison leads to infinite loop states, for example code
which trying to read entire file can easily get out of bound of
file size by passing position value which exceeds file size without this
check. So we need to add there missing comparison between the desired
position to be set and file size

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

diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
index dde4f4cb0e06..1dcb644e3b35 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -31,7 +31,7 @@
 

 #include "Ext4Disk.h"

 

-#define SYMLOOP_MAX    8

+#define SYMLOOP_MAX  8

 //

 // We need to specify path length limit for security purposes, to prevent possible

 // overflows and dead-loop conditions. Originally this limit is absent in FS design,

@@ -715,16 +715,15 @@ Ext4GetPosition (
 /**

   Sets a file's current position.

 

-  @param[in]  This            A pointer to the EFI_FILE_PROTOCOL instance that

-is the file handle to set the requested position on.

-  @param[in] Position        The byte position from the start of the file to

-set.

+  @param[in]  This            A pointer to the EFI_FILE_PROTOCOL instance that is the

+                              file handle to set the requested position on.

+  @param[in]  Position        The byte position from the start of the file to set.

 

-  @retval EFI_SUCCESS      The position was set.

-  @retval EFI_UNSUPPORTED  The seek request for nonzero is not valid on open

-                           directories.

-  @retval EFI_DEVICE_ERROR An attempt was made to set the position of a deleted

-file.

+  @retval EFI_SUCCESS            The position was set.

+  @retval EFI_INVALID_PARAMETER  The seek request for non-zero position is not valid on open

+                                 directories.

+  @retval EFI_UNSUPPORTED        The seek request for position is exceeds FileSize.

+  @retval EFI_DEVICE_ERROR       An attempt was made to set the position of a deleted file.

 

 **/

 EFI_STATUS

diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
index 04198a53bfc0..b4ed78847258 100644
--- a/Features/Ext4Pkg/Ext4Dxe/File.c
+++ b/Features/Ext4Pkg/Ext4Dxe/File.c
@@ -587,12 +587,13 @@ Ext4GetPosition (
 

   @param[in]  This            A pointer to the EFI_FILE_PROTOCOL instance that is the

                               file handle to set the requested position on.

-  @param[in] Position        The byte position from the start of the file to set.

+  @param[in]  Position        The byte position from the start of the file to set.

 

-  @retval EFI_SUCCESS      The position was set.

-  @retval EFI_UNSUPPORTED  The seek request for nonzero is not valid on open

-                           directories.

-  @retval EFI_DEVICE_ERROR An attempt was made to set the position of a deleted file.

+  @retval EFI_SUCCESS            The position was set.

+  @retval EFI_INVALID_PARAMETER  The seek request for non-zero position is not valid on open

+                                 directories.

+  @retval EFI_UNSUPPORTED        The seek request for position is exceeds FileSize.

+  @retval EFI_DEVICE_ERROR       An attempt was made to set the position of a deleted file.

 

 **/

 EFI_STATUS

@@ -603,17 +604,23 @@ Ext4SetPosition (
   )

 {

   EXT4_FILE  *File;

+  UINT64     FileSize;

 

   File = EXT4_FILE_FROM_THIS (This);

 

   // Only seeks to 0 (so it resets the ReadDir operation) are allowed

   if (Ext4FileIsDir (File) && (Position != 0)) {

-    return EFI_UNSUPPORTED;

+    return EFI_INVALID_PARAMETER;

   }

 

+  FileSize = EXT4_INODE_SIZE (File->Inode);

+

   // -1 (0xffffff.......) seeks to the end of the file

   if (Position == (UINT64)-1) {

-    Position = EXT4_INODE_SIZE (File->Inode);

+    Position = FileSize;

+  } else if (Position > FileSize) {

+    DEBUG ((DEBUG_FS, "[ext4] Ext4SetPosition Cannot seek to #%Lx of %Lx\n", Position, FileSize));

+    return EFI_UNSUPPORTED;

   }

 

   File->Position = Position;

-- 
2.38.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97196): https://edk2.groups.io/g/devel/message/97196
Mute This Topic: https://groups.io/mt/95563280/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-