From: Ana Bobillo <[email protected]> A block driver sets blksz and log2blksz from what the device reports: the READ CAPACITY reply of a USB stick or a SCSI disk, the LBA format of an NVMe namespace, the sector size of a Xen backend. Apart from the virtio block driver and the host command, none of these drivers checks the value, yet the partition and filesystem code relies on it:
- part_get_info_extended() and print_partition_extended() read one block into a buffer of blksz bytes and parse the partition table at offset 0x1be, past the end of a block smaller than 512 bytes - fs_devread() converts bytes to blocks with shifts by log2blksz, masks by blksz - 1 and divisions by blksz, which only agree when blksz is a power of two; with any other value it reads more bytes into the caller's buffer than the caller asked for - buffers of blksz bytes go on the stack, so a block size in the gigabytes overruns it Add blk_valid_blksz(), which accepts a power of two from 512 bytes up to BLK_MAX_BLKSZ (64KiB, the largest logical block size Linux accepts), so that the block drivers can check what their devices report. Use it in the virtio block driver and the host command, which then also refuse a block size above 64KiB. The next patches check the block size in the USB storage, SCSI, IDE and NVMe drivers, and the last one in blk_post_probe() for every block device. Signed-off-by: Ana Bobillo <[email protected]> Reviewed-by: Simon Glass <[email protected]> --- (no changes since v2) Changes in v2: - Split out of v1 patch 1: the helper comes first, and blk_post_probe() checks the block size only at the end of the series (Simon) - Use is_power_of_2() and leave the linux/log2.h includes alone (Simon) cmd/host.c | 5 +++-- drivers/virtio/virtio_blk.c | 2 +- include/blk.h | 21 +++++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/cmd/host.c b/cmd/host.c index 51f2b1e3f6e..7a94ea1fa27 100644 --- a/cmd/host.c +++ b/cmd/host.c @@ -67,8 +67,9 @@ static int do_host_bind(struct cmd_tbl *cmdtp, int flag, int argc, file = argv[1]; if (argc > 2) { blksz = dectoul(argv[2], NULL); - if (blksz < DEFAULT_BLKSZ || !is_power_of_2(blksz)) { - printf("blksz must be >= 512 and power of 2\n"); + if (!blk_valid_blksz(blksz)) { + printf("blksz must be a power of 2 from %d to %d\n", + DEFAULT_BLKSZ, BLK_MAX_BLKSZ); return CMD_RET_FAILURE; } } diff --git a/drivers/virtio/virtio_blk.c b/drivers/virtio/virtio_blk.c index 3d5d29f00df..8587605d100 100644 --- a/drivers/virtio/virtio_blk.c +++ b/drivers/virtio/virtio_blk.c @@ -258,7 +258,7 @@ static int virtio_blk_probe(struct udevice *dev) if (virtio_has_feature(dev, VIRTIO_BLK_F_BLK_SIZE)) { virtio_cread(dev, struct virtio_blk_config, blk_size, &blk_size); desc->blksz = blk_size; - if (!is_power_of_2(blk_size) || desc->blksz < 512) + if (!blk_valid_blksz(blk_size)) return -EIO; } else { desc->blksz = 512; diff --git a/include/blk.h b/include/blk.h index 5ef9129b3e6..07e0448f65e 100644 --- a/include/blk.h +++ b/include/blk.h @@ -10,6 +10,8 @@ #include <bouncebuf.h> #include <dm/uclass-id.h> #include <efi.h> +#include <linux/log2.h> +#include <linux/sizes.h> #ifdef CONFIG_SYS_64BIT_LBA typedef uint64_t lbaint_t; @@ -22,9 +24,28 @@ typedef ulong lbaint_t; #define LBAFU "%" LBAFlength "u" #define DEFAULT_BLKSZ 512 +/* Largest block size supported, as in Linux */ +#define BLK_MAX_BLKSZ SZ_64K struct udevice; +/** + * blk_valid_blksz() - Check that a block size is supported + * + * The block, partition and filesystem code size buffers by the block size, + * expect a block to hold at least a 512-byte boot sector and convert between + * bytes and blocks with log2(block size), so a device is only usable with a + * block size that is a power of two from DEFAULT_BLKSZ to BLK_MAX_BLKSZ. + * + * @blksz: Block size in bytes + * Return: true if @blksz is supported, false if not + */ +static inline bool blk_valid_blksz(unsigned long blksz) +{ + return blksz >= DEFAULT_BLKSZ && blksz <= BLK_MAX_BLKSZ && + is_power_of_2(blksz); +} + static inline bool blk_enabled(void) { return CONFIG_IS_ENABLED(BLK) || IS_ENABLED(CONFIG_SPL_LEGACY_BLOCK);
