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

[Heinrich Schuchardt]

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 at windriver.com>
> Reviewed-by: Tom Rini <trini at 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;
>