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