From: Ana Bobillo <[email protected]>

blk_post_probe() scans a block device for partitions with the block size
its driver set from what the device reports, and nothing checks it in
between. The USB storage, SCSI, IDE, NVMe and virtio block drivers and the
host command now check it where they read it, but other drivers do not,
such as the Xen PV block driver, and the partition and filesystem code
relies on it, as the patch adding blk_valid_blksz() describes.

Refuse to probe a block device whose block size blk_valid_blksz()
rejects, or whose log2blksz does not match its blksz, before its
partitions are scanned. This covers every block driver in one place
when its device is probed.

The check does not depend on PARTITIONS, since the filesystem code and
raw reads rely on the block size too. It is left out of SPL and TPL,
though: there it adds some 180 bytes, which takes the SPL of imx28_btt3
and popmetal-rk3288 over their size limits, and its message needs a
putc() that the SPL of evb-rk3036 does not have. In SPL the USB storage,
SCSI, IDE, NVMe and virtio block drivers still check the block size
where they read it.

A device with neither a size nor a block size, which is how the SCSI
code leaves a removable drive without a medium, is still probed, but
outside SPL and TPL its partitions are no longer scanned: part_test_dos()
could otherwise parse 512 bytes of a zero-sized buffer.

Signed-off-by: Ana Bobillo <[email protected]>
---
Changes in v2:
- Split out of v1 patch 1 and moved after the driver patches, so that
  each step is bisectable (Simon)
- Leave the check out of SPL and TPL, and say why in the commit message,
  along with why it is outside the PARTITIONS condition (Simon). On the
  code size: in SPL the v1 check and its message added 184 bytes to
  imx28_btt3, taking it over its SPL size limit, and 176 bytes to
  popmetal-rk3288, over its limit too, and the SPL of evb-rk3036 no
  longer linked (its tiny printf has no putc()). With the check left
  out, the SPL of these boards is the same size as without the series

 drivers/block/blk-uclass.c | 15 ++++++++++++++
 test/dm/blk.c              | 41 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 37d87be3a57..0c45a57f75f 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -828,6 +828,21 @@ static int part_create_block_devices(struct udevice 
*blk_dev)
 
 static int blk_post_probe(struct udevice *dev)
 {
+       if (!IS_ENABLED(CONFIG_XPL_BUILD)) {
+               struct blk_desc *desc = dev_get_uclass_plat(dev);
+
+               /* A removable device without a medium has no size yet */
+               if (!desc->blksz && !desc->lba)
+                       return 0;
+
+               if (!blk_valid_blksz(desc->blksz) ||
+                   desc->log2blksz != LOG2(desc->blksz)) {
+                       log_err("%s: unsupported block size %lu (log2 %d)\n",
+                               dev->name, desc->blksz, desc->log2blksz);
+                       return -EINVAL;
+               }
+       }
+
        if (CONFIG_IS_ENABLED(PARTITIONS) && blk_enabled()) {
                struct blk_desc *desc = dev_get_uclass_plat(dev);
 
diff --git a/test/dm/blk.c b/test/dm/blk.c
index f67869107da..80441f723c7 100644
--- a/test/dm/blk.c
+++ b/test/dm/blk.c
@@ -9,6 +9,7 @@
 #include <sandbox_host.h>
 #include <usb.h>
 #include <asm/state.h>
+#include <dm/device-internal.h>
 #include <dm/test.h>
 #include <test/test.h>
 #include <test/ut.h>
@@ -113,6 +114,46 @@ static int dm_test_blk_find(struct unit_test_state *uts)
 }
 DM_TEST(dm_test_blk_find, UTF_SCAN_PDATA | UTF_SCAN_FDT);
 
+/* Test that a block device with an unsupported block size is refused */
+static int dm_test_blk_blksz(struct unit_test_state *uts)
+{
+       static const ulong good[] = { DEFAULT_BLKSZ, SZ_4K, BLK_MAX_BLKSZ };
+       static const ulong bad[] = { 256, 520, 3 * DEFAULT_BLKSZ,
+                                    2 * BLK_MAX_BLKSZ };
+       struct udevice *blk, *dev;
+       struct blk_desc *desc;
+       char label[10];
+       int i;
+
+       for (i = 0; i < ARRAY_SIZE(good); i++) {
+               snprintf(label, sizeof(label), "good%d", i);
+               ut_assertok(host_create_device(label, false, good[i], &dev));
+               ut_assertok(blk_get_from_parent(dev, &blk));
+               desc = dev_get_uclass_plat(blk);
+               ut_asserteq(good[i], desc->blksz);
+       }
+
+       for (i = 0; i < ARRAY_SIZE(bad); i++) {
+               snprintf(label, sizeof(label), "bad%d", i);
+               ut_assertok(host_create_device(label, false, bad[i], &dev));
+               ut_asserteq(-EINVAL, blk_get_from_parent(dev, &blk));
+       }
+
+       /* A device without a medium has no block size yet */
+       ut_assertok(host_create_device("none", false, 0, &dev));
+       ut_assertok(blk_get_from_parent(dev, &blk));
+
+       /* The block size and its log2 must agree */
+       ut_assertok(host_create_device("log2", false, DEFAULT_BLKSZ, &dev));
+       ut_assertok(blk_find_from_parent(dev, &blk));
+       desc = dev_get_uclass_plat(blk);
+       desc->log2blksz = 10;
+       ut_asserteq(-EINVAL, device_probe(blk));
+
+       return 0;
+}
+DM_TEST(dm_test_blk_blksz, UTF_SCAN_PDATA | UTF_SCAN_FDT);
+
 /* Test that block device numbering works as expected */
 static int dm_test_blk_devnum(struct unit_test_state *uts)
 {

Reply via email to