On Fri, Dec 9, 2022 at 4:11 PM Savva Mitrofanov <savvamtr@gmail.com> wrote:
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

+  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;

On further inspection, this case is covered in the UEFI spec.

https://uefi.org/specs/UEFI/2.10/13_Protocols_Media_Access.html#efi-file-protocol-read says:

> EFI_DEVICE_ERROR     On entry, the current file position is beyond the end of the file.

while the standard does not say SetPosition() can error out for bad seeks.

So I think we should allow this in SetPosition() and error out in Read(). Does this look good to you?
 
Pedro