Bug in ext2load?

Bug in ext2load?

[Gary Johnson]

I think I've found a bug in the ext2load command.  When the file
being read is a sparse file, the command fails as in this example.

    => ext2load usb 0:1 $load_addr firmware_ls2088ardb_norboot.img
    fs_devread read outside partition 60063712
    Failed to load 'firmware_ls2088ardb_norboot.img'

However, if the same image file is copied to the USB flash drive on
the build machine so that it is not sparse, that is, with "cp
--sparse=never ...", then the ext2load command on the target machine
succeeds.

I think the problem may be in the ext4fs_read_file() command in
fs/ext4/ext4fs.c.  That is the only function I see in the path from
the ext2load command to the error message that does anything
explicitly with blocks.  Also, this comment at the top of that
function suggests that someone tried to optimize the reading of the
file and may have missed some sparse-file condition.

    /*
     * Taken from openmoko-kernel mailing list: By Andy green
     * Optimized read file API : collects and defers contiguous sector
     * reads into one potentially more efficient larger sequential read action
     */

However, I did not try to debug the problem further, as using "cp
--sparse=never" is an adequate workaround and I have more pressing
work to do.

It appears that the ext2load command runs off the end of the list of
blocks in the inode and tries to read beyond the end of the
partition.

    => usb storage
      Device 0: Vendor:  USB Rev: 1.00 Prod:  SanDisk 3.2Gen1
                Type: Removable Hard Disk
                Capacity: 29328.0 MB = 28.6 GB (60063744 x 512)

Note that the drive capacity is 32 sectors less than the sector
reported in the error message.  According to the part list command,
the partition starts at sector 32, and the number of sectors
available matches the number reported in the error message, so this
adds up.

    => part list usb 0

    Partition Map for USB device 0  --   Partition Type: DOS

    Part    Start Sector    Num Sectors     UUID            Type
      1     32              60063712        eca4cd09-01     83

This error happens with two versions of U-Boot:

    U-Boot 2016.092.0+ga06b209 (Mar 30 2017 - 01:15:01 +0800)
    U-Boot 2021.04-ge2eba0cd58 (Aug 27 2021 - 22:23:21 +0800)

The last change to fs/ext4/ext4fs.c appears to have been made at
commit e6f6f9e648 (2020-05-10).

Let me know if you need any addtional information or if I missed
something and am wrong about all this.

Regards,
Gary

Re: Bug in ext2load?

[Simon Glass]

+Heinrich Schuchardt

On Thu, 29 Sept 2022 at 16:31, Gary Johnson <garyjohn@spocom.com> wrote:
>
> I think I've found a bug in the ext2load command.  When the file
> being read is a sparse file, the command fails as in this example.
>
>     => ext2load usb 0:1 $load_addr firmware_ls2088ardb_norboot.img
>     fs_devread read outside partition 60063712
>     Failed to load 'firmware_ls2088ardb_norboot.img'
>
> However, if the same image file is copied to the USB flash drive on
> the build machine so that it is not sparse, that is, with "cp
> --sparse=never ...", then the ext2load command on the target machine
> succeeds.
>
> I think the problem may be in the ext4fs_read_file() command in
> fs/ext4/ext4fs.c.  That is the only function I see in the path from
> the ext2load command to the error message that does anything
> explicitly with blocks.  Also, this comment at the top of that
> function suggests that someone tried to optimize the reading of the
> file and may have missed some sparse-file condition.
>
>     /*
>      * Taken from openmoko-kernel mailing list: By Andy green
>      * Optimized read file API : collects and defers contiguous sector
>      * reads into one potentially more efficient larger sequential read action
>      */
>
> However, I did not try to debug the problem further, as using "cp
> --sparse=never" is an adequate workaround and I have more pressing
> work to do.
>
> It appears that the ext2load command runs off the end of the list of
> blocks in the inode and tries to read beyond the end of the
> partition.
>
>     => usb storage
>       Device 0: Vendor:  USB Rev: 1.00 Prod:  SanDisk 3.2Gen1
>                 Type: Removable Hard Disk
>                 Capacity: 29328.0 MB = 28.6 GB (60063744 x 512)
>
> Note that the drive capacity is 32 sectors less than the sector
> reported in the error message.  According to the part list command,
> the partition starts at sector 32, and the number of sectors
> available matches the number reported in the error message, so this
> adds up.
>
>     => part list usb 0
>
>     Partition Map for USB device 0  --   Partition Type: DOS
>
>     Part    Start Sector    Num Sectors     UUID            Type
>       1     32              60063712        eca4cd09-01     83
>
> This error happens with two versions of U-Boot:
>
>     U-Boot 2016.092.0+ga06b209 (Mar 30 2017 - 01:15:01 +0800)
>     U-Boot 2021.04-ge2eba0cd58 (Aug 27 2021 - 22:23:21 +0800)
>
> The last change to fs/ext4/ext4fs.c appears to have been made at
> commit e6f6f9e648 (2020-05-10).
>
> Let me know if you need any addtional information or if I missed
> something and am wrong about all this.
>
> Regards,
> Gary
>

Re: Bug in ext2load?

[Heinrich Schuchardt]

On 9/30/22 15:28, Simon Glass wrote:
> +Heinrich Schuchardt
>
> On Thu, 29 Sept 2022 at 16:31, Gary Johnson <garyjohn@spocom.com> wrote:
>>
>> I think I've found a bug in the ext2load command.  When the file

According to the description below the bug is not in the command but in
the file-system driver.

Function read_allocated_block() tries to implement handling of sparse
files. According to comments it is a copy from GRUB. GRUB has tried to
fix an error for sparse files with commit 1e5ed5f5f5a ("fs/ext2: Fix
handling of missing sparse extent leafs.")

Maybe we could try to upgrade to GRUB's current code.

>> being read is a sparse file, the command fails as in this example.
>>
>>      => ext2load usb 0:1 $load_addr firmware_ls2088ardb_norboot.img
>>      fs_devread read outside partition 60063712
>>      Failed to load 'firmware_ls2088ardb_norboot.img'
>>
>> However, if the same image file is copied to the USB flash drive on
>> the build machine so that it is not sparse, that is, with "cp
>> --sparse=never ...", then the ext2load command on the target machine
>> succeeds.
>>
>> I think the problem may be in the ext4fs_read_file() command in
>> fs/ext4/ext4fs.c.  That is the only function I see in the path from
>> the ext2load command to the error message that does anything
>> explicitly with blocks.  Also, this comment at the top of that
>> function suggests that someone tried to optimize the reading of the
>> file and may have missed some sparse-file condition.
>>
>>      /*
>>       * Taken from openmoko-kernel mailing list: By Andy green
>>       * Optimized read file API : collects and defers contiguous sector
>>       * reads into one potentially more efficient larger sequential read action
>>       */
>>
>> However, I did not try to debug the problem further, as using "cp
>> --sparse=never" is an adequate workaround and I have more pressing
>> work to do.
>>
>> It appears that the ext2load command runs off the end of the list of
>> blocks in the inode and tries to read beyond the end of the
>> partition.
>>
>>      => usb storage
>>        Device 0: Vendor:  USB Rev: 1.00 Prod:  SanDisk 3.2Gen1
>>                  Type: Removable Hard Disk
>>                  Capacity: 29328.0 MB = 28.6 GB (60063744 x 512)
>>
>> Note that the drive capacity is 32 sectors less than the sector
>> reported in the error message.  According to the part list command,
>> the partition starts at sector 32, and the number of sectors
>> available matches the number reported in the error message, so this
>> adds up.
>>
>>      => part list usb 0
>>
>>      Partition Map for USB device 0  --   Partition Type: DOS
>>
>>      Part    Start Sector    Num Sectors     UUID            Type
>>        1     32              60063712        eca4cd09-01     83
>>
>> This error happens with two versions of U-Boot:
>>
>>      U-Boot 2016.092.0+ga06b209 (Mar 30 2017 - 01:15:01 +0800)
>>      U-Boot 2021.04-ge2eba0cd58 (Aug 27 2021 - 22:23:21 +0800)
>>
>> The last change to fs/ext4/ext4fs.c appears to have been made at
>> commit e6f6f9e648 (2020-05-10).
>>
>> Let me know if you need any addtional information or if I missed
>> something and am wrong about all this.
>>
>> Regards,
>> Gary
>>

Here is another error report for ext4:

[BUG] ext4: the load command on an EXT4 partition leads to errors due to
unaligned buffers
https://lists.denx.de/pipermail/u-boot/2021-January/437180.html

Unfortunately filesystems cbfs, fat, ext4, reiserfs, zfs don't have
maintainers.

Best regards

Heinrich