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

["Pedro Falcato" <pedro.falcato@gmail.com>]

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

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

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