Seems I misunderstood the usage of SetPosition, thanks for pointing out.  So we can just drop this commit
and keep everything as is, because this check is already present in Ext4Read.

Savva Mitrofanov

On 10 Dec 2022, at 04:12, Pedro Falcato <pedro.falcato@gmail.com> wrote:

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.


> 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