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

["Savva Mitrofanov" <savvamtr@gmail.com>]

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

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 <mailto: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 <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: 3220 bytes --]