[1/9] fs/fat/fat.c: Do not perform zero block reads if there are no blocks left

While using u-boot with qemu's virtio driver I stumbled across a
problem reading files less than sector size.  On the real hardware the
block reader seems ok with reading zero blocks, and while we could fix
the virtio host side of qemu to deal with a zero block read instead of
crashing, the u-boot fat driver should not be doing zero block reads
in the first place.  If you ask hardware to read zero blocks you are
just going to get zero data.  There may also be other hardware that
responds similarly to the virtio interface so this is worth fixing.

Without the patch I get the following and have to restart qemu because
it dies.
---------------------------------
=> fatls virtio 0:1
       30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
qemu-system-aarch64: virtio: zero sized buffers are not allowed
---------------------------------

With the patch I get the expected results.
---------------------------------
=> fatls virtio 0:1
       30   cmdline.txt
=> fatload virtio 0:1 ${loadaddr} cmdline.txt
30 bytes read in 11 ms (2 KiB/s)
=> md.b ${loadaddr} 0x1E
40080000: 64 77 63 5f 6f 74 67 2e 6c 70 6d 5f 65 6e 61 62    dwc_otg.lpm_enab
40080010: 6c 65 3d 30 20 72 6f 6f 74 77 61 69 74 0a          le=0 rootwait.

---------------------------------

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
 fs/fat/fat.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

On 7/24/20 11:50 PM, Jason Wessel wrote:
> While using u-boot with qemu's virtio driver I stumbled across a
> problem reading files less than sector size.  On the real hardware the
> block reader seems ok with reading zero blocks, and while we could fix
> the virtio host side of qemu to deal with a zero block read instead of
> crashing, the u-boot fat driver should not be doing zero block reads
> in the first place.  If you ask hardware to read zero blocks you are
> just going to get zero data.  There may also be other hardware that
> responds similarly to the virtio interface so this is worth fixing.
>
> Without the patch I get the following and have to restart qemu because
> it dies.

Our block drivers all have a different view on this:

mmc_read_blocks() is reading one block even if 0 blocks are requested.
scsi_setup_read16() happily passes the 0 to the controller.

I think blk_read_devnum(), blk_write_devnum(), blk_dread(), and
blk_dwrite() is where we need the block count check.

> ---------------------------------
> => fatls virtio 0:1
>        30   cmdline.txt
> => fatload virtio 0:1 ${loadaddr} cmdline.txt
> qemu-system-aarch64: virtio: zero sized buffers are not allowed
> ---------------------------------
>
> With the patch I get the expected results.
> ---------------------------------
> => fatls virtio 0:1
>        30   cmdline.txt
> => fatload virtio 0:1 ${loadaddr} cmdline.txt
> 30 bytes read in 11 ms (2 KiB/s)
> => md.b ${loadaddr} 0x1E
> 40080000: 64 77 63 5f 6f 74 67 2e 6c 70 6d 5f 65 6e 61 62    dwc_otg.lpm_enab
> 40080010: 6c 65 3d 30 20 72 6f 6f 74 77 61 69 74 0a          le=0 rootwait.
>
> ---------------------------------
>
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> ---
>  fs/fat/fat.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 9578b74bae..28aa5aaa9f 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -278,7 +278,10 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
>  		}
>  	} else {
>  		idx = size / mydata->sect_size;
> -		ret = disk_read(startsect, idx, buffer);
> +		if (idx == 0)
> +			ret = 0;
> +		else
> +			ret = disk_read(startsect, idx, buffer);

Using idx as variable name here for a block count is quite misleading.

Best regards

Heinrich

>  		if (ret != idx) {
>  			debug("Error reading data (got %d)\n", ret);
>  			return -1;
>