[PATCH v3 00/14] block, disk: Check block sizes and partition tables of devices

[PATCH v3 00/14] block, disk: Check block sizes and partition tables of devices

[Ana Bobillo]

U-Boot scans the partitions of a block device as soon as it probes it,
and takes several sizes from the device and from its partition tables
as they are. A USB stick or SD card that U-Boot merely probes can
therefore make it read or write outside its buffers. This series checks
those sizes where they are read.

Block size. A block driver sets the block size of a device from what
the device reports, and only the virtio block driver and the host
command check it. The partition and filesystem code sizes buffers by the
block size, expects a block to hold a 512-byte boot sector and converts
between bytes and blocks with log2(blksz). So a USB stick, SCSI disk,
ATAPI drive or NVMe namespace reporting a block size below 512 bytes,
one that is not a power of two or a huge one makes U-Boot read or write
outside its buffers, in the partition scan run when the device is probed
and in the filesystem code after it.

Patch 1 drops two local copies of is_power_of_2(), so that blk.h can
use the one in linux/log2.h, and patch 2 adds blk_valid_blksz() there.
Patch 3 fixes the reply the USB storage driver makes up when READ
CAPACITY fails, which gave a 128KiB block size on a little-endian host.
Patches 4-7 check the block size where the USB storage, SCSI, IDE
(ATAPI) and NVMe drivers read it, so that each refuses such a device
cleanly: the USB LUN is skipped instead of failing the whole stick, no
SCSI block device is created, the ATAPI drive is treated as one without
a medium and the NVMe namespace is skipped like an inactive one. Patch 8
then has blk_post_probe() refuse a block device whose block size
blk_valid_blksz() rejects, which covers every block driver when its
device is probed, outside SPL and TPL (their size limits leave no room
for it). The MMC code sets the block size of a card again when it
initialises the card anew, as "mmc rescan" does, which that check does
not see: patch 14 has it always use 512-byte blocks, whatever block
length the card reports, as it already did in DDR mode.

The upper bound is 64KiB, the largest logical block size Linux accepts;
the GPT code is already tested with block sizes up to 32KiB.

Partition tables. Patch 9 checks the header size and the size of the
partition entry array of a GPT, as Linux does, before they size the CRC
of the header and the entry buffer. It leaves alloc_read_gpt_entries()
alone, so it applies with or without Benoît Monin's "disk: efi: Reuse
the GPT entries buffer with simple malloc". Patch 10 walks the chain of
DOS extended partition tables in a loop: a table linking back to itself
adds no partition, so the limit added for CVE-2019-13103 did not stop
the recursion. As Linux does, the walk now gives up after a few tables
in a row that hold no partition. Patch 11 makes room for partition
number 256 in the UUID made up for a DOS partition. Patch 12 has the
TCG2 measurement of the GPT measure the GPT the partition driver uses,
read with the driver's own code. It read the primary GPT itself,
unchecked, while the driver falls back to the backup GPT when the
primary one is not valid, so PCR 5 could hold a GPT the partitions were
not found in, and the sizes in an unchecked header could make the
measurement read past its buffer. Patch 13 then indexes the entries
directly there, as they now always have the size of a gpt_entry.

Patches 4, 5, 8-12 and 14 add tests: DM tests on sandbox, for the GPT a
test in test/lib on a blkmap device, and for the TCG2 measurement a test
in test/py that starts an EFI application from a GPT disk on sandbox and
checks PCR 5. With the series they pass, as do the rest of "ut dm" and
"ut lib" on sandbox_defconfig with CONFIG_ASAN (but for lib_crypt, which
ASan flags without the series too), and each patch builds and passes the
tests present so far on its own. The test/py test is skipped with
CONFIG_ASAN, under which sandbox cannot run an EFI application; on
sandbox_defconfig it passes, as do the other EFI, TPM and GPT tests in
test/py. The IDE and NVMe changes are build tested only, as sandbox has
no emulator for them.

CI: the series passes all 91 jobs of U-Boot's Azure pipeline, in build
13719 of my CI-only pull request, on main 3401e006432.
PR:    https://github.com/u-boot/u-boot/pull/1064
Build: https://dev.azure.com/u-boot/u-boot/_build/results?buildId=13719

Changes in v3:
- Add Simon's Reviewed-by to patches 1-11
- Patch 12: rename part_get_gpt_primary() to part_get_gpt(), measure
  the backup header as it is when the primary header is marked IGNOREME
  (ChromeOS), name the device in the warning when no valid GPT is read,
  and shorten the commit message (Simon)
- New patch 13, which indexes the GPT entries directly in
  tcg2_measure_gpt_data() and reads their count with le32_to_cpu()
  (Simon)
- New patch 14, which has the MMC code always use 512-byte blocks, as
  blk_post_probe() does not see the block size it sets when a card is
  initialised again
- Rebase on current main, which needed no changes to the patches

Changes in v2:
- Reorder the block size patches, so that each step is bisectable: the
  helper first, then the driver patches, and the blk_post_probe() check
  last (Simon)
- Use is_power_of_2() in blk_valid_blksz() (Simon), after a new patch 1
  that drops the local copies in the btrfs code and bcm2835_sdhost, which
  clash with it
- Split the USB READ CAPACITY fallback fix into a patch of its own, with
  a Fixes: tag, and fake 2880 blocks rather than 2881 (Simon)
- Name the LUN that the USB storage driver skips, and check the message
  in the test (Simon)
- Take the ATAPI block sizes 0, 2340 and 2352 as 2048, as Linux does,
  and say what is skipped otherwise (Simon)
- Leave the blk_post_probe() check out of SPL and TPL, where it took the
  SPL of imx28_btt3 and popmetal-rk3288 over their size limits and broke
  the link of evb-rk3036's, and say why it is outside the PARTITIONS
  condition (Simon)
- Include linux/sizes.h in part_efi.c, and word the GPT commit message
  so that the fastboot path is not taken as fully covered (Simon)
- Write the DOS walk as a for loop with a helper that follows the links,
  and stop it after 16 tables in a row without a partition (Simon)
- Measure the GPT the partition driver uses, which checks the CRC of
  the entries too (Simon), and when the primary GPT is not valid the
  backup one, with the primary header made from it, as EDK2 does
  (Ilias); say what the bug does to PCR 5 (Ilias)
- Avoid ut_assertok() of a known -ENOMEM in the GPT test (Coverity)

Ana Bobillo (14):
  treewide: Use is_power_of_2() from linux/log2.h
  block: Add blk_valid_blksz() to check a block size
  usb: storage: Fix the capacity faked when READ CAPACITY fails
  usb: storage: Refuse a block size the block layer cannot use
  scsi: Refuse a block size the block layer cannot use
  ide: Refuse an ATAPI block size atapi_read() cannot read
  nvme: Skip a namespace with an unsupported LBA data size
  block: Refuse a device with an unsupported block size
  disk: part_efi: Check the header and entry sizes of a GPT
  disk: part_dos: Walk the extended partition tables in a bounded loop
  disk: part_dos: Make room for a three-digit partition number in the
    UUID
  efi_loader: Measure the GPT the partition driver uses
  efi_loader: Index the GPT entries directly in tcg2_measure_gpt_data()
  mmc: Always use a 512-byte block length

 arch/sandbox/include/asm/test.h  |  30 ++++
 cmd/host.c                       |   5 +-
 common/usb_storage.c             |  10 +-
 disk/part_dos.c                  | 291 ++++++++++++++++++-------------
 disk/part_efi.c                  |  74 +++++++-
 drivers/block/blk-uclass.c       |  15 ++
 drivers/block/ide.c              |  23 ++-
 drivers/mmc/bcm2835_sdhost.c     |   6 +-
 drivers/mmc/mmc.c                |  34 ++--
 drivers/mmc/sandbox_mmc.c        |  11 +-
 drivers/nvme/nvme.c              |  33 +++-
 drivers/scsi/sandbox_scsi.c      |   8 +
 drivers/scsi/scsi.c              |   4 +
 drivers/usb/emul/sandbox_flash.c |   8 +
 drivers/virtio/virtio_blk.c      |   2 +-
 fs/btrfs/compat.h                |   6 +-
 include/blk.h                    |  21 +++
 include/part.h                   |  27 +++
 lib/efi_loader/efi_tcg2.c        |  76 ++++----
 test/dm/blk.c                    |  41 +++++
 test/dm/mmc.c                    |  40 +++++
 test/dm/part.c                   | 221 +++++++++++++++++++++++
 test/dm/scsi.c                   |  37 ++++
 test/dm/usb.c                    |  51 ++++++
 test/lib/part_efi.c              | 222 +++++++++++++++++++++++
 test/py/tests/test_efi_tcg2.py   | 246 ++++++++++++++++++++++++++
 26 files changed, 1329 insertions(+), 213 deletions(-)
 create mode 100644 test/py/tests/test_efi_tcg2.py


base-commit: 3401e006432abcbf7d73d7210f10df157f990ed3

[PATCH v3 01/14] treewide: Use is_power_of_2() from linux/log2.h

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

The btrfs code and the bcm2835_sdhost driver each define their own
is_power_of_2(), which clashes with the one in linux/log2.h once a
header they include brings that in, as include/blk.h does after the next
patch. Drop both and use the common one.

The btrfs copy is the same as the common one. The bcm2835_sdhost copy
takes a u64 and also takes 0 as a power of two; its only caller checks
the block size of a data transfer, which the MMC core never sets to 0,
and the Linux driver it comes from makes the same check with the common
is_power_of_2().

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(no changes since v2)

Changes in v2:
- New patch, so that blk.h can use is_power_of_2() from linux/log2.h

 drivers/mmc/bcm2835_sdhost.c | 6 +-----
 fs/btrfs/compat.h            | 6 +-----
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/bcm2835_sdhost.c b/drivers/mmc/bcm2835_sdhost.c
index 720127468d3..eee7e2d43a2 100644
--- a/drivers/mmc/bcm2835_sdhost.c
+++ b/drivers/mmc/bcm2835_sdhost.c
@@ -42,6 +42,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
+#include <linux/log2.h>
 #include <linux/sizes.h>
 #include <mach/gpio.h>
 #include <power/regulator.h>
@@ -660,11 +661,6 @@ static void bcm2835_set_clock(struct bcm2835_host *host, unsigned int clock)
 	writel(host->mmc->clock / 2, host->ioaddr + SDTOUT);
 }
 
-static inline int is_power_of_2(u64 x)
-{
-	return !(x & (x - 1));
-}
-
 static int bcm2835_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
 			    struct mmc_data *data)
 {
diff --git a/fs/btrfs/compat.h b/fs/btrfs/compat.h
index 4596b9d1dd3..ffeba740eff 100644
--- a/fs/btrfs/compat.h
+++ b/fs/btrfs/compat.h
@@ -4,6 +4,7 @@
 #define __BTRFS_COMPAT_H__
 
 #include <linux/errno.h>
+#include <linux/log2.h>
 #include <fs_internal.h>
 #include <u-boot/uuid.h>
 
@@ -74,9 +75,4 @@ static inline void uuid_unparse(const u8 *uuid, char *out)
 	return uuid_bin_to_str((unsigned char *)uuid, out, 0);
 }
 
-static inline int is_power_of_2(unsigned long n)
-{
-	return (n != 0 && ((n & (n - 1)) == 0));
-}
-
 #endif

[PATCH v3 02/14] block: Add blk_valid_blksz() to check a block size

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

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 <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(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);

[PATCH v3 03/14] usb: storage: Fix the capacity faked when READ CAPACITY fails

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

When READ CAPACITY fails, usb_stor_get_info() makes up a reply of its own
for a 1.44MB floppy, 2880 blocks of 512 bytes. But it sets the values in
host byte order, while the reply is read back as big endian, so on a
little-endian host the device gets 0x400b0001 blocks (about 1G) of 128KiB
rather than 2880 of 512 bytes. It also puts 2880 where READ CAPACITY
returns the last LBA, so even on a big-endian host the device gets 2881
blocks.

Set the reply in big endian like the one it stands in for, with 2879 as
the last LBA, so that such a device has 2880 blocks of 512 bytes on any
host.

Fixes: 149dded2b178 ("* Add support for USB Mass Storage Devices (BBB)   (tested with USB memory sticks only)")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(no changes since v2)

Changes in v2:
- New patch, split out of v1 patch 2, with a Fixes: tag (Simon)
- Give 2879 as the last LBA, so that the device has 2880 blocks (Simon)

 common/usb_storage.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/common/usb_storage.c b/common/usb_storage.c
index ac331f1c1b0..f7c96195b59 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -1496,8 +1496,9 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 	if (usb_read_capacity(pccb, ss) != 0) {
 		printf("READ_CAP ERROR\n");
 		ss->flags &= ~USB_READY;
-		cap[0] = 2880;
-		cap[1] = 0x200;
+		/* a 1.44MB floppy, as READ CAPACITY gives it: last LBA, size */
+		cap[0] = cpu_to_be32(2880 - 1);
+		cap[1] = cpu_to_be32(0x200);
 	}
 	debug("Read Capacity returns: 0x%08x, 0x%08x\n", cap[0], cap[1]);
 #if 0

[PATCH v3 04/14] usb: storage: Refuse a block size the block layer cannot use

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

usb_stor_get_info() takes the block size from the READ CAPACITY reply of
the device as it is. Once blk_post_probe() refuses a block size that
blk_valid_blksz() rejects, at the end of this series, such a device would
fail to probe there, and usb_stor_probe_device() would stop, skipping the
other LUNs of the device.

Check the block size where it is read, and treat a LUN reporting one
that blk_valid_blksz() rejects as unusable, as for a LUN that is not
ready, saying which LUN is skipped. This also covers the legacy (non-BLK)
path, which calls part_init() itself.

Add a test where the sandbox flash stick reports an unsupported block
size.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(no changes since v2)

Changes in v2:
- Move the READ CAPACITY fallback fix to a patch of its own (Simon)
- Name the LUN in the message (Simon)
- Check for that message in the test, with UTF_CONSOLE (Simon)

 arch/sandbox/include/asm/test.h  | 10 +++++++
 common/usb_storage.c             |  5 ++++
 drivers/usb/emul/sandbox_flash.c |  8 +++++
 test/dm/usb.c                    | 51 ++++++++++++++++++++++++++++++++
 4 files changed, 74 insertions(+)

diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h
index 0438790028b..e5590d01a92 100644
--- a/arch/sandbox/include/asm/test.h
+++ b/arch/sandbox/include/asm/test.h
@@ -123,6 +123,16 @@ long sandbox_i2c_rtc_get_set_base_time(struct udevice *dev, long base_time);
 
 int sandbox_usb_keyb_add_string(struct udevice *dev, const char *str);
 
+/**
+ * sandbox_flash_set_block_size() - Set the block size a flash stick reports
+ *
+ * @dev:	USB flash-stick emulator
+ * @block_size:	Block size in bytes
+ *
+ * The host reads it when it next probes the stick's mass-storage device.
+ */
+void sandbox_flash_set_block_size(struct udevice *dev, int block_size);
+
 /**
  * sandbox_osd_get_mem() - get the internal memory of a sandbox OSD
  *
diff --git a/common/usb_storage.c b/common/usb_storage.c
index f7c96195b59..6bada2b309b 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -1513,6 +1513,11 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 	blksz = be32_to_cpu(cap[1]);
 
 	debug("Capacity = 0x%08x, blocksz = 0x%08x\n", capacity, blksz);
+	if (!blk_valid_blksz(blksz)) {
+		printf("LUN %d: unsupported block size %u, skipping\n",
+		       dev_desc->lun, blksz);
+		return 0;
+	}
 	dev_desc->lba = capacity;
 	dev_desc->blksz = blksz;
 	dev_desc->log2blksz = LOG2(dev_desc->blksz);
diff --git a/drivers/usb/emul/sandbox_flash.c b/drivers/usb/emul/sandbox_flash.c
index b5176bb30ce..02e0901050f 100644
--- a/drivers/usb/emul/sandbox_flash.c
+++ b/drivers/usb/emul/sandbox_flash.c
@@ -13,6 +13,7 @@
 #include <scsi.h>
 #include <scsi_emul.h>
 #include <usb.h>
+#include <asm/test.h>
 
 /*
  * This driver emulates a flash stick using the UFI command specification and
@@ -357,6 +358,13 @@ static int sandbox_flash_probe(struct udevice *dev)
 	return 0;
 }
 
+void sandbox_flash_set_block_size(struct udevice *dev, int block_size)
+{
+	struct sandbox_flash_priv *priv = dev_get_priv(dev);
+
+	priv->eminfo.block_size = block_size;
+}
+
 static int sandbox_flash_remove(struct udevice *dev)
 {
 	struct sandbox_flash_priv *priv = dev_get_priv(dev);
diff --git a/test/dm/usb.c b/test/dm/usb.c
index fa894c1096e..79a85b11910 100644
--- a/test/dm/usb.c
+++ b/test/dm/usb.c
@@ -84,6 +84,57 @@ static int dm_test_usb_flash(struct unit_test_state *uts)
 }
 DM_TEST(dm_test_usb_flash, UTF_SCAN_PDATA | UTF_SCAN_FDT);
 
+/* Test that a flash stick reporting an unsupported block size is refused */
+static int dm_test_usb_flash_blksz(struct unit_test_state *uts)
+{
+	static const struct {
+		int blksz;
+		bool ok;
+	} cases[] = {
+		{ 256, false },
+		{ 520, false },
+		{ 2 * BLK_MAX_BLKSZ, false },
+		{ SZ_4K, true },
+	};
+	struct udevice *dev, *emul, *blk;
+	struct blk_desc *desc;
+	int i, ret;
+
+	state_set_skip_delays(true);
+	ut_assertok(usb_init());
+	ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 0, &dev));
+	ut_assertok(uclass_get_device_by_name(UCLASS_USB_EMUL, "flash-stick@0",
+					      &emul));
+
+	for (i = 0; i < ARRAY_SIZE(cases); i++) {
+		/* Probe the stick again, so that it reads the block size */
+		console_record_reset();
+		sandbox_flash_set_block_size(emul, cases[i].blksz);
+		ut_assertok(device_remove(dev, DM_REMOVE_NORMAL));
+		ut_assertok(device_chld_unbind(dev, NULL));
+		ut_assertok(device_probe(dev));
+
+		ret = device_find_first_child_by_uclass(dev, UCLASS_BLK, &blk);
+		if (!cases[i].ok) {
+			/* usb_stor_get_info() refuses the LUN */
+			ut_assert_nextline("LUN 0: unsupported block size %d, skipping",
+					   cases[i].blksz);
+			ut_assert_console_end();
+			ut_asserteq(-ENODEV, ret);
+			continue;
+		}
+		ut_assert_console_end();
+		ut_assertok(ret);
+		desc = dev_get_uclass_plat(blk);
+		ut_asserteq(cases[i].blksz, desc->blksz);
+	}
+
+	ut_assertok(usb_stop());
+
+	return 0;
+}
+DM_TEST(dm_test_usb_flash_blksz, UTF_SCAN_PDATA | UTF_SCAN_FDT | UTF_CONSOLE);
+
 /* test that we can handle multiple storage devices */
 static int dm_test_usb_multi(struct unit_test_state *uts)
 {

[PATCH v3 05/14] scsi: Refuse a block size the block layer cannot use

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

scsi_detect_dev() takes the block size from the READ CAPACITY reply of
the device as it is. do_scsi_scan_one() then hands it to
blk_create_devicef() as an int, which drops the upper bits of the
unsigned long, so the block layer may not even see the value the device
reported.

Check the block size where it is read, so that no block device is
created for a disk reporting one that blk_valid_blksz() rejects.

Add a test where the sandbox SCSI disk reports an unsupported block
size.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)

 arch/sandbox/include/asm/test.h | 10 +++++++++
 drivers/scsi/sandbox_scsi.c     |  8 +++++++
 drivers/scsi/scsi.c             |  4 ++++
 test/dm/scsi.c                  | 37 +++++++++++++++++++++++++++++++++
 4 files changed, 59 insertions(+)

diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h
index e5590d01a92..76e2f11b059 100644
--- a/arch/sandbox/include/asm/test.h
+++ b/arch/sandbox/include/asm/test.h
@@ -133,6 +133,16 @@ int sandbox_usb_keyb_add_string(struct udevice *dev, const char *str);
  */
 void sandbox_flash_set_block_size(struct udevice *dev, int block_size);
 
+/**
+ * sandbox_scsi_set_block_size() - Set the block size a SCSI disk reports
+ *
+ * @dev:	Sandbox SCSI controller
+ * @block_size:	Block size in bytes
+ *
+ * The host reads it when it next scans the bus.
+ */
+void sandbox_scsi_set_block_size(struct udevice *dev, int block_size);
+
 /**
  * sandbox_osd_get_mem() - get the internal memory of a sandbox OSD
  *
diff --git a/drivers/scsi/sandbox_scsi.c b/drivers/scsi/sandbox_scsi.c
index 5f0b01d86d5..34fc7cda915 100644
--- a/drivers/scsi/sandbox_scsi.c
+++ b/drivers/scsi/sandbox_scsi.c
@@ -14,6 +14,7 @@
 #include <malloc.h>
 #include <scsi.h>
 #include <scsi_emul.h>
+#include <asm/test.h>
 
 enum {
 	SANDBOX_SCSI_BLOCK_LEN		= 512,
@@ -118,6 +119,13 @@ static int sandbox_scsi_probe(struct udevice *dev)
 	return 0;
 }
 
+void sandbox_scsi_set_block_size(struct udevice *dev, int block_size)
+{
+	struct sandbox_scsi_priv *priv = dev_get_priv(dev);
+
+	priv->eminfo.block_size = block_size;
+}
+
 static int sandbox_scsi_remove(struct udevice *dev)
 {
 	struct sandbox_scsi_priv *priv = dev_get_priv(dev);
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 736dccfcbff..55d68cc701a 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -543,6 +543,10 @@ static int scsi_detect_dev(struct udevice *dev, int target, int lun,
 		scsi_print_error(pccb);
 		return -EINVAL;
 	}
+	if (!blk_valid_blksz(blksz)) {
+		printf("Unsupported block size %lu\n", blksz);
+		return -EINVAL;
+	}
 	dev_desc->lba = capacity;
 	dev_desc->blksz = blksz;
 	dev_desc->log2blksz = LOG2(dev_desc->blksz);
diff --git a/test/dm/scsi.c b/test/dm/scsi.c
index 66859c991df..7617ee54893 100644
--- a/test/dm/scsi.c
+++ b/test/dm/scsi.c
@@ -6,6 +6,7 @@
 #include <dm.h>
 #include <part.h>
 #include <scsi.h>
+#include <asm/test.h>
 #include <dm/test.h>
 #include <test/test.h>
 #include <test/ut.h>
@@ -36,3 +37,39 @@ static int dm_test_scsi_base(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_scsi_base, UTF_SCAN_PDATA | UTF_SCAN_FDT);
+
+/* Test that a disk reporting an unsupported block size is refused */
+static int dm_test_scsi_blksz(struct unit_test_state *uts)
+{
+	static const struct {
+		int blksz;
+		bool ok;
+	} cases[] = {
+		{ 256, false },
+		{ 520, false },
+		{ 2 * BLK_MAX_BLKSZ, false },
+		{ SZ_4K, true },
+	};
+	struct udevice *dev, *blk;
+	struct blk_desc *desc;
+	int i, ret;
+
+	ut_assertok(uclass_first_device_err(UCLASS_SCSI, &dev));
+
+	for (i = 0; i < ARRAY_SIZE(cases); i++) {
+		sandbox_scsi_set_block_size(dev, cases[i].blksz);
+		ut_assertok(scsi_scan(false));
+
+		ret = device_find_first_child_by_uclass(dev, UCLASS_BLK, &blk);
+		if (!cases[i].ok) {
+			ut_asserteq(-ENODEV, ret);
+			continue;
+		}
+		ut_assertok(ret);
+		desc = dev_get_uclass_plat(blk);
+		ut_asserteq(cases[i].blksz, desc->blksz);
+	}
+
+	return 0;
+}
+DM_TEST(dm_test_scsi_blksz, UTF_SCAN_PDATA | UTF_SCAN_FDT);

[PATCH v3 06/14] ide: Refuse an ATAPI block size atapi_read() cannot read

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

atapi_inquiry() takes the block size from the READ CAPACITY reply of an
ATAPI device as it is, while atapi_read() asks the device for, and moves
the buffer on by, ATAPI_READ_BLOCK_SIZE (2048) bytes for every block.
With any other block size the callers' buffers, which are sized by
blksz, do not match what atapi_read() writes into them: a 512-byte block
size makes a read of several blocks write past the end of the buffer.

Some CD drives report another block size although they read 2048-byte
blocks: 2340 (HP 4020i CD-Recorder), 2352 (Philips CD-Writers) or 0.
Take these as ATAPI_READ_BLOCK_SIZE, as Linux does in get_sectorsize(),
so that a drive reporting 0, which ide_probe() skipped until now, is
read with 2048-byte blocks too. Refuse any other value: a device
reporting one is left with a zero block size, which ide_probe() skips,
as for a drive without a medium. This refuses drives with 512-byte
blocks, such as ZIP drives, whose reads of more than one block already
overran the buffer; supporting them needs atapi_read() to use the block
size of the device.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(no changes since v2)

Changes in v2:
- Take 0, 2340 and 2352 as 2048, as Linux get_sectorsize() does (Simon)
- Say what is skipped in the message (Simon)

 drivers/block/ide.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 44ce054bdf5..56e55bc8bc1 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -450,6 +450,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 {
 	u8 ccb[12];	/* Command descriptor block */
 	u8 iobuf[64];	/* temp buf */
+	ulong blksz;
 	u8 c;
 	int device;
 
@@ -514,10 +515,28 @@ static void atapi_inquiry(struct blk_desc *desc)
 		  iobuf[0], iobuf[1], iobuf[2], iobuf[3],
 		  iobuf[4], iobuf[5], iobuf[6], iobuf[7]);
 
+	blksz = (ulong)iobuf[4] << 24 | (ulong)iobuf[5] << 16 |
+		(ulong)iobuf[6] << 8 | (ulong)iobuf[7];
+	/*
+	 * atapi_read() transfers ATAPI_READ_BLOCK_SIZE bytes per block. Some
+	 * CD drives report another size although they read 2048-byte blocks,
+	 * which Linux takes as 2048 in get_sectorsize(): do the same.
+	 */
+	switch (blksz) {
+	case 0:
+	case 2340:	/* HP 4020i CD-Recorder */
+	case 2352:	/* Philips CD-Writers */
+		blksz = ATAPI_READ_BLOCK_SIZE;
+		break;
+	case ATAPI_READ_BLOCK_SIZE:
+		break;
+	default:
+		printf("ATAPI block size %lu not supported, skipping\n", blksz);
+		return;
+	}
 	desc->lba = (ulong)iobuf[0] << 24 | (ulong)iobuf[1] << 16 |
 		(ulong)iobuf[2] << 8 | (ulong)iobuf[3];
-	desc->blksz = (ulong)iobuf[4] << 24 | (ulong)iobuf[5] << 16 |
-		(ulong)iobuf[6] << 8 | (ulong)iobuf[7];
+	desc->blksz = blksz;
 	desc->log2blksz = LOG2(desc->blksz);
 
 	/* ATAPI devices cannot use 48bit addressing (ATA/ATAPI v7) */

[PATCH v3 07/14] nvme: Skip a namespace with an unsupported LBA data size

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

nvme_blk_probe() takes the LBA data size (log2 of the block size) of the
namespace's current LBA format as the device reports it, and shifts by
it: 1 << ns->lba_shift is undefined for a value of 31 or more, and
nvme_blk_rw() sizes each transfer with
1 << (dev->max_transfer_shift - ns->lba_shift), a negative shift for a
block larger than the maximum transfer size of the controller.

Check it, through a new nvme_lba_shift(), against the block sizes
blk_valid_blksz() accepts and the maximum transfer size. nvme_init()
skips a namespace with an unsupported LBA data size, as it skips an
inactive one, rather than create a block device for it that then fails
to probe, failing nvme_init() and leaving the namespaces it has already
probed on a controller that failed to probe. nvme_blk_probe() checks the
value it uses again.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)

 drivers/nvme/nvme.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index c815a967137..3df9b531bfd 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -15,6 +15,7 @@
 #include <time.h>
 #include <dm/device-internal.h>
 #include <linux/compat.h>
+#include <linux/log2.h>
 #include "nvme.h"
 
 #define NVME_Q_DEPTH		2
@@ -701,6 +702,26 @@ int nvme_scan_namespace(void)
 	return 0;
 }
 
+/**
+ * nvme_lba_shift() - Get the LBA data size of a namespace
+ *
+ * @dev: NVMe controller
+ * @id: Identify Namespace data of the namespace
+ * Return: log2 of its block size, or -EINVAL if that size is not supported
+ */
+static int nvme_lba_shift(struct nvme_dev *dev, struct nvme_id_ns *id)
+{
+	u8 ds = id->lbaf[id->flbas & NVME_NS_FLBAS_LBA_MASK].ds;
+
+	if (ds < ilog2(DEFAULT_BLKSZ) || ds > ilog2(BLK_MAX_BLKSZ) ||
+	    ds > dev->max_transfer_shift) {
+		log_err("Unsupported LBA data size 2^%d\n", ds);
+		return -EINVAL;
+	}
+
+	return ds;
+}
+
 static int nvme_blk_probe(struct udevice *udev)
 {
 	struct nvme_dev *ndev = dev_get_priv(udev->parent);
@@ -708,6 +729,7 @@ static int nvme_blk_probe(struct udevice *udev)
 	struct nvme_ns *ns = dev_get_priv(udev);
 	u8 flbas;
 	struct nvme_id_ns *id;
+	int ret;
 
 	id = memalign(ndev->page_size, sizeof(struct nvme_id_ns));
 	if (!id)
@@ -724,7 +746,12 @@ static int nvme_blk_probe(struct udevice *udev)
 	memcpy(&ns->eui64, &id->eui64, sizeof(id->eui64));
 	flbas = id->flbas & NVME_NS_FLBAS_LBA_MASK;
 	ns->flbas = flbas;
-	ns->lba_shift = id->lbaf[flbas].ds;
+	ret = nvme_lba_shift(ndev, id);
+	if (ret < 0) {
+		free(id);
+		return ret;
+	}
+	ns->lba_shift = ret;
 	list_add(&ns->list, &ndev->namespaces);
 
 	desc->lba = le64_to_cpu(id->nsze);
@@ -899,6 +926,10 @@ int nvme_init(struct udevice *udev)
 		if (!id->nsze)
 			continue;
 
+		/* skip a namespace whose block size is not supported */
+		if (nvme_lba_shift(ndev, id) < 0)
+			continue;
+
 		/*
 		 * Encode the namespace id to the device name so that
 		 * we can extract it when doing the probe.

[PATCH v3 08/14] block: Refuse a device with an unsupported block size

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

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 <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(no changes since v2)

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)
 {

[PATCH v3 09/14] disk: part_efi: Check the header and entry sizes of a GPT

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

validate_gpt_header() computes the CRC of a GPT header over header_size
bytes, as the header gives it, and nothing checks the size of the
partition entries before they are used:

- the header is read into a buffer of one block, on the stack behind
  part_get_info(), part_print_efi(), gpt_repair_headers() and the gpt
  command, so a header_size larger than the block size makes the CRC
  read past the end of that buffer
- alloc_read_gpt_entries() allocates and reads
  num_partition_entries * sizeof_partition_entry bytes, computed in 32
  bits, but the entries are then used as an array of
  num_partition_entries gpt_entry structures: with a smaller entry size,
  or a product that wraps, part_get_gpt_pte() and part_print_efi() read
  past the end of the allocation. On a 32-bit board, rounding a product
  close to 4GiB up to whole blocks also wraps, to a tiny allocation that
  the entries are then read into.

As Linux does in is_gpt_valid(), refuse a header whose header_size is
smaller than the header or larger than the block it is read from, one
whose sizeof_partition_entry is not sizeof(gpt_entry), and one whose
entry array is larger than 4MiB (KMALLOC_MAX_SIZE in Linux with 4KiB
pages, 32768 entries). Larger entries are allowed by the UEFI
specification, but U-Boot has never read them correctly, and Linux does
not accept them either.

The header size is checked before the CRC is computed over it. All the
checks are in validate_gpt_header(), so they also apply to the header
that is_valid_gpt_buf() checks when fastboot is sent a GPT. That path
still takes partition_entry_lba from the header without checking it
against the size of the download, which is left for a follow-up.

Add a test that gives the primary header of a GPT on a blkmap device
each of these sizes, with CRCs to match, and checks that it is refused
and that the backup header is used instead.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(no changes since v2)

Changes in v2:
- Include linux/sizes.h, rather than rely on blk.h for SZ_4M (Simon)
- Don't word the commit message as if the fastboot path were fully
  covered: its partition_entry_lba is left for a follow-up (Simon)
- Check the header allocation with ut_assertnonnull() in the test, so
  that ut_assertok() never sees a known -ENOMEM (as Coverity CID 661179
  reports for lib_test_gpt_blkmap())

 disk/part_efi.c     |  47 +++++++++++--
 test/lib/part_efi.c | 163 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 205 insertions(+), 5 deletions(-)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index d8751297fbe..85945634915 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -28,8 +28,15 @@
 #include <linux/compiler.h>
 #include <linux/ctype.h>
 #include <linux/printk.h>
+#include <linux/sizes.h>
 #include <u-boot/crc.h>
 
+/*
+ * Largest partition entry array accepted, as in Linux (KMALLOC_MAX_SIZE with
+ * 4KiB pages): 32768 entries, far more than any tool writes
+ */
+#define GPT_ENTRIES_MAX_SIZE	SZ_4M
+
 /* GUID for basic data partitons */
 #if CONFIG_IS_ENABLED(EFI_PARTITION)
 static const efi_guid_t partition_basic_data_guid = PARTITION_BASIC_DATA_GUID;
@@ -89,10 +96,11 @@ static int get_bootable(gpt_entry *p)
 }
 
 static int validate_gpt_header(gpt_header *gpt_h, lbaint_t lba,
-		lbaint_t lastlba)
+		lbaint_t lastlba, unsigned long blksz)
 {
 	uint32_t crc32_backup = 0;
 	uint32_t calc_crc32;
+	u32 header_size, entry_size, num_entries;
 
 	/* Check the GPT header signature */
 	if (le64_to_cpu(gpt_h->signature) != GPT_HEADER_SIGNATURE_UBOOT) {
@@ -103,12 +111,22 @@ static int validate_gpt_header(gpt_header *gpt_h, lbaint_t lba,
 		return -1;
 	}
 
+	/*
+	 * The CRC covers header_size bytes, so the header has to fit in the
+	 * one block that was read into gpt_h
+	 */
+	header_size = le32_to_cpu(gpt_h->header_size);
+	if (header_size < sizeof(gpt_header) || header_size > blksz) {
+		log_debug("GPT: header_size incorrect: %u not in %zu..%lu\n",
+			  header_size, sizeof(gpt_header), blksz);
+		return -1;
+	}
+
 	/* Check the GUID Partition Table CRC */
 	memcpy(&crc32_backup, &gpt_h->header_crc32, sizeof(crc32_backup));
 	memset(&gpt_h->header_crc32, 0, sizeof(gpt_h->header_crc32));
 
-	calc_crc32 = efi_crc32((const unsigned char *)gpt_h,
-		le32_to_cpu(gpt_h->header_size));
+	calc_crc32 = efi_crc32((const unsigned char *)gpt_h, header_size);
 
 	memcpy(&gpt_h->header_crc32, &crc32_backup, sizeof(crc32_backup));
 
@@ -143,6 +161,24 @@ static int validate_gpt_header(gpt_header *gpt_h, lbaint_t lba,
 		return -1;
 	}
 
+	/*
+	 * The partition entries are used as an array of gpt_entry, read into
+	 * a buffer whose size is the product of these two fields, worked out
+	 * in 32 bits and then rounded up to whole blocks
+	 */
+	entry_size = le32_to_cpu(gpt_h->sizeof_partition_entry);
+	if (entry_size != sizeof(gpt_entry)) {
+		log_debug("GPT: sizeof_partition_entry incorrect: %u != %zu\n",
+			  entry_size, sizeof(gpt_entry));
+		return -1;
+	}
+	num_entries = le32_to_cpu(gpt_h->num_partition_entries);
+	if (num_entries > GPT_ENTRIES_MAX_SIZE / entry_size) {
+		log_debug("GPT: num_partition_entries too large: %u\n",
+			  num_entries);
+		return -1;
+	}
+
 	debug("GPT: first_usable_lba: %llX last_usable_lba: %llX last lba: "
 	      LBAF "\n", le64_to_cpu(gpt_h->first_usable_lba),
 	      le64_to_cpu(gpt_h->last_usable_lba), lastlba);
@@ -925,7 +961,7 @@ int is_valid_gpt_buf(struct blk_desc *desc, void *buf)
 	/* determine start of GPT Header in the buffer */
 	gpt_h = buf + (GPT_PRIMARY_PARTITION_TABLE_LBA * desc->blksz);
 	if (validate_gpt_header(gpt_h, GPT_PRIMARY_PARTITION_TABLE_LBA,
-				desc->lba))
+				desc->lba, desc->blksz))
 		return -1;
 
 	/* determine start of GPT Entries in the buffer */
@@ -1095,7 +1131,8 @@ static int is_gpt_valid(struct blk_desc *desc, u64 lba, gpt_header *pgpt_head,
 		return 2;
 	}
 
-	if (validate_gpt_header(pgpt_head, (lbaint_t)lba, desc->lba))
+	if (validate_gpt_header(pgpt_head, (lbaint_t)lba, desc->lba,
+				desc->blksz))
 		return 0;
 
 	if (desc->sig_type == SIG_TYPE_NONE) {
diff --git a/test/lib/part_efi.c b/test/lib/part_efi.c
index 82153fdabfd..42de9109360 100644
--- a/test/lib/part_efi.c
+++ b/test/lib/part_efi.c
@@ -12,6 +12,7 @@
 #include <part.h>
 #include <part_efi.h>
 #include <asm/byteorder.h>
+#include <u-boot/crc.h>
 #include <test/lib.h>
 #include <test/test.h>
 #include <test/ut.h>
@@ -280,4 +281,166 @@ static int lib_test_gpt_blkmap(struct unit_test_state *uts)
 }
 LIB_TEST(lib_test_gpt_blkmap, 0);
 
+/**
+ * gpt_sizes_check() - check the size fields of a GPT header one by one
+ *
+ * Give the primary header of a good table each set of sizes in turn, with
+ * both CRCs recomputed over what those sizes cover, as a crafted disk would
+ * have them. Only the checks on the sizes themselves can then refuse it.
+ *
+ * @uts:	test state
+ * @dev:	blkmap device with nothing mapped yet
+ * @lba:	size of the device in blocks
+ * @buf:	memory to back the device with, @lba blocks of DEFAULT_BLKSZ
+ * @hdr:	two blocks of memory, to rewrite the header in
+ * Return: 0 if OK, other value on error
+ */
+static int gpt_sizes_check(struct unit_test_state *uts, struct udevice *dev,
+			   lbaint_t lba, void *buf, gpt_header *hdr)
+{
+	static const struct {
+		u32 header_size;
+		u32 num_entries;
+		u32 entry_size;
+		int ret;
+	} cases[] = {
+		{ sizeof(gpt_header), GPT_ENTRY_NUMBERS, sizeof(gpt_entry), 0 },
+		/* the reserved rest of the block may be part of the header */
+		{ DEFAULT_BLKSZ, GPT_ENTRY_NUMBERS, sizeof(gpt_entry), 0 },
+		/* a header longer than the block it is read from */
+		{ DEFAULT_BLKSZ + 1, GPT_ENTRY_NUMBERS, sizeof(gpt_entry), -1 },
+		{ sizeof(gpt_header) - 1, GPT_ENTRY_NUMBERS, sizeof(gpt_entry),
+		  -1 },
+		/* entries are only ever read as a gpt_entry array */
+		{ sizeof(gpt_header), GPT_ENTRY_NUMBERS, sizeof(gpt_entry) / 2,
+		  -1 },
+		{ sizeof(gpt_header), GPT_ENTRY_NUMBERS, sizeof(gpt_entry) * 2,
+		  -1 },
+		/* an array whose size comes to a single entry in 32 bits */
+		{ sizeof(gpt_header), U32_MAX / sizeof(gpt_entry) + 2,
+		  sizeof(gpt_entry), -1 },
+	};
+
+	char str_disk_guid[UUID_STR_LEN + 1] =
+		"8d60b397-1bb6-4d33-80ee-b1587d24c2f8";
+	const unsigned long blksz = DEFAULT_BLKSZ;
+	struct disk_partition parts[1], info;
+	struct blk_desc *desc;
+	struct udevice *blk;
+	gpt_entry *gpt_pte;
+	void *entries;
+	int i, ret;
+
+	ut_assertok(blkmap_map_mem(dev, 0, lba, buf));
+	ut_assertok(blk_get_from_parent(dev, &blk));
+	desc = dev_get_uclass_plat(blk);
+
+	memset(parts, '\0', sizeof(parts));
+	strcpy((char *)parts[0].name, "first");
+	parts[0].size = 0x100;
+	disk_partition_set_uuid(&parts[0],
+				"a2a0d0eb-e5b9-3344-87c0-68b6b72699c7");
+	ut_assertok(gpt_restore(desc, str_disk_guid, parts, 1));
+
+	for (i = 0; i < ARRAY_SIZE(cases); i++) {
+		u32 header_size = cases[i].header_size;
+		u32 array_size;
+
+		/* start from the primary header as it is on the disk */
+		memset(hdr, '\0', 2 * blksz);
+		ut_asserteq(1, blk_dread(desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
+					 1, hdr));
+
+		hdr->header_size = cpu_to_le32(header_size);
+		hdr->num_partition_entries = cpu_to_le32(cases[i].num_entries);
+		hdr->sizeof_partition_entry = cpu_to_le32(cases[i].entry_size);
+
+		/* the array CRC covers the size as the parser works it out */
+		array_size = cases[i].num_entries * cases[i].entry_size;
+		entries = buf + le64_to_cpu(hdr->partition_entry_lba) * blksz;
+		hdr->partition_entry_array_crc32 =
+			cpu_to_le32(crc32(0, entries, array_size));
+		hdr->header_crc32 = 0;
+		hdr->header_crc32 = cpu_to_le32(crc32(0, (void *)hdr,
+						      header_size));
+		ut_asserteq(1, blk_dwrite(desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
+					  1, hdr));
+
+		/*
+		 * Read it back into a buffer as large as the one above, so
+		 * that a header longer than its block is refused by the size
+		 * check rather than by a CRC over whatever follows the buffer
+		 */
+		memset(hdr, '\0', 2 * blksz);
+		gpt_pte = NULL;
+		ret = gpt_verify_headers(desc, hdr, &gpt_pte);
+		free(gpt_pte);
+		ut_assertf(ret == cases[i].ret,
+			   "case %d: header %u, %u entries of %u: %d\n", i,
+			   header_size, cases[i].num_entries,
+			   cases[i].entry_size, ret);
+
+		/*
+		 * fastboot checks a GPT it is sent in memory the same way.
+		 * There a header longer than its block is also refused by its
+		 * CRC, which then takes in the first entry that follows it.
+		 */
+		ut_asserteq(cases[i].ret, is_valid_gpt_buf(desc, buf));
+
+		/* either way the disk still reads, from the backup if needed */
+		desc->part_type = PART_TYPE_UNKNOWN;
+		part_init(desc);
+		ut_asserteq(PART_TYPE_EFI, desc->part_type);
+		ut_assertok(part_get_info(desc, 1, &info));
+		ut_asserteq_str((char *)parts[0].name, (char *)info.name);
+		ut_asserteq_64(parts[0].size, info.size);
+	}
+
+	return 0;
+}
+
+/*
+ * A GPT header gives its own size and the size of its partition entries, and
+ * its CRCs cover only what those say, so a CRC does not show that either size
+ * is one the parser can use. Check that a primary header with a size the
+ * parser cannot use is refused, CRCs or not, and that the backup is used.
+ */
+static int lib_test_gpt_sizes(struct unit_test_state *uts)
+{
+	const unsigned long blksz = DEFAULT_BLKSZ;
+	const lbaint_t lba = 0x400;
+	struct udevice *dev;
+	gpt_header *hdr;
+	int ret, err;
+	void *buf;
+
+	buf = memalign(ARCH_DMA_MINALIGN, lba * blksz);
+	ut_assertnonnull(buf);
+	memset(buf, '\0', lba * blksz);
+
+	hdr = malloc_cache_aligned(2 * blksz);
+	if (!hdr) {
+		free(buf);
+		ut_assertnonnull(hdr);
+	}
+	ret = blkmap_create(GPT_TEST_LABEL, &dev);
+	if (ret) {
+		free(hdr);
+		free(buf);
+		ut_assertok(ret);
+	}
+
+	/* clean up here, as lib_test_gpt_blkmap() explains */
+	ret = gpt_sizes_check(uts, dev, lba, buf, hdr);
+	err = blkmap_destroy(dev);
+	free(hdr);
+	free(buf);
+	if (ret)
+		return ret;
+	ut_assertok(err);
+
+	return 0;
+}
+LIB_TEST(lib_test_gpt_sizes, 0);
+
 #endif /* BLKMAP */

[PATCH v3 10/14] disk: part_dos: Walk the extended partition tables in a bounded loop

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

part_get_info_extended() and print_partition_extended() follow the
links from one extended partition table to the next by recursing, and
stop at MAX_EXT_PARTS partitions, the limit added for CVE-2019-13103.
But a link adds no partition: an extended partition table holding only
a link back to itself does not advance part_num, so the recursion goes
on until the stack runs out, whenever a logical partition of the device
is looked up. Even within the limit, every level keeps a buffer of one
block on the stack, so a long chain of tables on a device with large
blocks needs megabytes of stack.

Follow the links in a loop instead, reusing the one buffer, with the
code that finds the next link in a helper, dos_next_table(), which both
functions use. As Linux does in parse_extended(), stop after
MAX_EXT_EMPTY (16) tables in a row that hold no partition: tables
linking back to each other are then given up after a few reads. Also
stop after MAX_EXT_TABLES tables in all, as many as MAX_EXT_PARTS
partitions need when each table holds one, so that no walk reads more
than that. A chain that also holds tables without a partition then
reads fewer than MAX_EXT_PARTS partitions; partitioning tools write one
logical partition per table.

print_partition_extended() follows every link of a table, where
part_get_info_extended() follows only the first, so it can list
partitions that cannot be looked up, under numbers that clash, and its
walk is not a chain but a tree. Follow only the first link there too.

Add tests with a table holding only a link between two logical
partitions and then a table that links only to itself, and with 15 and
16 such tables in a row before a logical partition.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(no changes since v2)

Changes in v2:
- Use a for loop rather than a backward goto, and move the code that
  follows a link into a helper used by both functions (Simon)
- Also stop after 16 tables in a row without a partition, as Linux does,
  so that the table in the test costs about 20 reads per lookup at probe
  rather than 256 (Simon)
- Tidy the style of the lines the loop re-indents
- Use the present tense in the commit message (Simon)
- Call part_init() in the test rather than setting part_type (Simon)
- Add a table holding only a link to the test, and a test of the limit
  of 16 tables in a row without a partition

 disk/part_dos.c | 289 +++++++++++++++++++++++++++---------------------
 test/dm/part.c  | 175 +++++++++++++++++++++++++++++
 2 files changed, 340 insertions(+), 124 deletions(-)

diff --git a/disk/part_dos.c b/disk/part_dos.c
index 4b560fde1fb..bc40d9d2829 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -28,6 +28,18 @@
  * to use large numbers of partitions */
 #define MAX_EXT_PARTS 256
 
+/*
+ * A link to the next extended partition table adds no partition, so tables
+ * linking back to each other do not advance part_num. As Linux does in
+ * parse_extended(), stop after MAX_EXT_EMPTY tables in a row that hold no
+ * partition, so that such a loop ends after a few reads. Also stop after
+ * MAX_EXT_TABLES tables in all, as many as MAX_EXT_PARTS partitions need,
+ * so that tables with and without partitions in turn cannot make a walk
+ * read more.
+ */
+#define MAX_EXT_EMPTY	16
+#define MAX_EXT_TABLES	MAX_EXT_PARTS
+
 static inline int is_extended(int part_type)
 {
     return (part_type == DOS_PART_TYPE_EXTENDED ||
@@ -126,6 +138,42 @@ static int part_test_dos(struct blk_desc *desc)
 	return 0;
 }
 
+/**
+ * dos_next_table() - Follow the link from a partition table to the next one
+ *
+ * Only the first link of a table is followed: fdisk numbers the logical
+ * partitions along that chain.
+ *
+ * @buffer:		Partition table just read
+ * @ext_part_sector:	Sector of that table (0 for the MBR), updated to the
+ *			sector of the next table
+ * @relative:		Start of the extended partition, which the links of
+ *			its tables are relative to, set from the link in the
+ *			MBR
+ * Return:		true if the table links to a next one, false if not
+ */
+static bool dos_next_table(unsigned char *buffer, lbaint_t *ext_part_sector,
+			   lbaint_t *relative)
+{
+	dos_partition_t *pt;
+	int i;
+
+	pt = (dos_partition_t *)(buffer + DOS_PART_TBL_OFFSET);
+	for (i = 0; i < 4; i++, pt++) {
+		if (is_extended(pt->sys_ind)) {
+			lbaint_t lba_start = get_unaligned_le32(&pt->start_sect) +
+					     *relative;
+
+			if (!*ext_part_sector)
+				*relative = lba_start;
+			*ext_part_sector = lba_start;
+			return true;
+		}
+	}
+
+	return false;
+}
+
 /*  Print a partition that is relative to its Extended partition table
  */
 static void print_partition_extended(struct blk_desc *desc,
@@ -135,65 +183,61 @@ static void print_partition_extended(struct blk_desc *desc,
 {
 	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, desc->blksz);
 	dos_partition_t *pt;
+	int tables, empty = 0;
 	int i;
 
-	/* set a maximum recursion level */
-	if (part_num > MAX_EXT_PARTS)
-	{
-		printf("** Nested DOS partitions detected, stopping **\n");
-		return;
-    }
-
-	if (blk_dread(desc, ext_part_sector, 1, (ulong *)buffer) != 1) {
-		printf ("** Can't read partition table on %d:" LBAFU " **\n",
-			desc->devnum, ext_part_sector);
-		return;
-	}
-	i=test_block_type(buffer);
-	if (i != DOS_MBR) {
-		printf ("bad MBR sector signature 0x%02x%02x\n",
-			buffer[DOS_PART_MAGIC_OFFSET],
-			buffer[DOS_PART_MAGIC_OFFSET + 1]);
-		return;
-	}
-
-	if (!ext_part_sector)
-		disksig = get_unaligned_le32(&buffer[DOS_PART_DISKSIG_OFFSET]);
+	for (tables = 0; ; tables++) {
+		int prev_num = part_num;
 
-	/* Print all primary/logical partitions */
-	pt = (dos_partition_t *) (buffer + DOS_PART_TBL_OFFSET);
-	for (i = 0; i < 4; i++, pt++) {
-		/*
-		 * fdisk does not show the extended partitions that
-		 * are not in the MBR
-		 */
-
-		if ((pt->sys_ind != 0) &&
-		    (ext_part_sector == 0 || !is_extended (pt->sys_ind)) ) {
-			print_one_part(pt, ext_part_sector, part_num, disksig);
+		/* bound the walk */
+		if (part_num > MAX_EXT_PARTS || tables >= MAX_EXT_TABLES ||
+		    empty >= MAX_EXT_EMPTY) {
+			printf("** Nested DOS partitions detected, stopping **\n");
+			return;
 		}
 
-		/* Reverse engr the fdisk part# assignment rule! */
-		if ((ext_part_sector == 0) ||
-		    (pt->sys_ind != 0 && !is_extended (pt->sys_ind)) ) {
-			part_num++;
+		if (blk_dread(desc, ext_part_sector, 1, (ulong *)buffer) != 1) {
+			printf("** Can't read partition table on %d:" LBAFU " **\n",
+			       desc->devnum, ext_part_sector);
+			return;
+		}
+		i = test_block_type(buffer);
+		if (i != DOS_MBR) {
+			printf("bad MBR sector signature 0x%02x%02x\n",
+			       buffer[DOS_PART_MAGIC_OFFSET],
+			       buffer[DOS_PART_MAGIC_OFFSET + 1]);
+			return;
 		}
-	}
 
-	/* Follows the extended partitions */
-	pt = (dos_partition_t *) (buffer + DOS_PART_TBL_OFFSET);
-	for (i = 0; i < 4; i++, pt++) {
-		if (is_extended (pt->sys_ind)) {
-			lbaint_t lba_start
-				= get_unaligned_le32 (&pt->start_sect) + relative;
+		if (!ext_part_sector)
+			disksig = get_unaligned_le32(&buffer[DOS_PART_DISKSIG_OFFSET]);
+
+		/* Print all primary/logical partitions */
+		pt = (dos_partition_t *)(buffer + DOS_PART_TBL_OFFSET);
+		for (i = 0; i < 4; i++, pt++) {
+			/*
+			 * fdisk does not show the extended partitions that
+			 * are not in the MBR
+			 */
+
+			if ((pt->sys_ind != 0) &&
+			    (ext_part_sector == 0 || !is_extended(pt->sys_ind))) {
+				print_one_part(pt, ext_part_sector, part_num,
+					       disksig);
+			}
 
-			print_partition_extended(desc, lba_start,
-						 !ext_part_sector ? lba_start :
-						 relative, part_num, disksig);
+			/* Reverse engr the fdisk part# assignment rule! */
+			if ((ext_part_sector == 0) ||
+			    (pt->sys_ind != 0 && !is_extended(pt->sys_ind))) {
+				part_num++;
+			}
 		}
-	}
+		empty = part_num > prev_num ? 0 : empty + 1;
 
-	return;
+		/* Follows the extended partitions */
+		if (!dos_next_table(buffer, &ext_part_sector, &relative))
+			break;
+	}
 }
 
 /**
@@ -220,93 +264,90 @@ static int part_get_info_extended(struct blk_desc *desc,
 	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, desc->blksz);
 	struct disk_partition wdinfo = { 0 };
 	dos_partition_t *pt;
+	int tables, empty = 0;
 	int i, ret;
 	int dos_type;
 
-	/* set a maximum recursion level */
-	if (part_num > MAX_EXT_PARTS)
-	{
-		printf("** Nested DOS partitions detected, stopping **\n");
-		return -1;
-    }
+	for (tables = 0; ; tables++) {
+		int prev_num = part_num;
 
-	if (blk_dread(desc, ext_part_sector, 1, (ulong *)buffer) != 1) {
-		printf ("** Can't read partition table on %d:" LBAFU " **\n",
-			desc->devnum, ext_part_sector);
-		return -1;
-	}
-	if (buffer[DOS_PART_MAGIC_OFFSET] != 0x55 ||
-		buffer[DOS_PART_MAGIC_OFFSET + 1] != 0xaa) {
-		printf ("bad MBR sector signature 0x%02x%02x\n",
-			buffer[DOS_PART_MAGIC_OFFSET],
-			buffer[DOS_PART_MAGIC_OFFSET + 1]);
-		return -1;
-	}
+		/* bound the walk */
+		if (part_num > MAX_EXT_PARTS || tables >= MAX_EXT_TABLES ||
+		    empty >= MAX_EXT_EMPTY) {
+			printf("** Nested DOS partitions detected, stopping **\n");
+			return -1;
+		}
 
-	if (CONFIG_IS_ENABLED(PARTITION_UUIDS) && !ext_part_sector)
-		disksig = get_unaligned_le32(&buffer[DOS_PART_DISKSIG_OFFSET]);
+		if (blk_dread(desc, ext_part_sector, 1, (ulong *)buffer) != 1) {
+			printf("** Can't read partition table on %d:" LBAFU " **\n",
+			       desc->devnum, ext_part_sector);
+			return -1;
+		}
+		if (buffer[DOS_PART_MAGIC_OFFSET] != 0x55 ||
+		    buffer[DOS_PART_MAGIC_OFFSET + 1] != 0xaa) {
+			printf("bad MBR sector signature 0x%02x%02x\n",
+			       buffer[DOS_PART_MAGIC_OFFSET],
+			       buffer[DOS_PART_MAGIC_OFFSET + 1]);
+			return -1;
+		}
 
-	if (info) {
-		ret = part_get_info_whole_disk(desc, &wdinfo);
-		if (ret)
-			return ret;
-	}
+		if (CONFIG_IS_ENABLED(PARTITION_UUIDS) && !ext_part_sector)
+			disksig = get_unaligned_le32(&buffer[DOS_PART_DISKSIG_OFFSET]);
 
-	/* Print all primary/logical partitions */
-	pt = (dos_partition_t *) (buffer + DOS_PART_TBL_OFFSET);
-	for (i = 0; i < 4; i++, pt++) {
-		/*
-		 * fdisk does not show the extended partitions that
-		 * are not in the MBR
-		 */
-		if (((pt->boot_ind & ~0x80) == 0) &&
-		    (pt->sys_ind != 0) &&
-		    (part_num == which_part) &&
-		    (ext_part_sector == 0 || is_extended(pt->sys_ind) == 0)) {
-			if (info) {
-				if (wdinfo.blksz > DOS_PART_DEFAULT_SECTOR)
-					info->blksz = wdinfo.blksz;
-				else
-					info->blksz = DOS_PART_DEFAULT_SECTOR;
-				info->start = (lbaint_t)(ext_part_sector +
-							 get_unaligned_le32(&pt->start_sect));
-				info->size  = (lbaint_t)get_unaligned_le32(&pt->nr_sects);
-				part_set_generic_name(desc, part_num,
-						      (char *)info->name);
-				snprintf(info->type, sizeof(info->type), "0x%x", pt->sys_ind);
-				info->bootable = get_bootable(pt);
-				if (CONFIG_IS_ENABLED(PARTITION_UUIDS)) {
-					char str[12];
-
-					sprintf(str, "%08x-%02x", disksig, part_num);
-					disk_partition_set_uuid(info, str);
+		if (info) {
+			ret = part_get_info_whole_disk(desc, &wdinfo);
+			if (ret)
+				return ret;
+		}
+
+		/* Print all primary/logical partitions */
+		pt = (dos_partition_t *)(buffer + DOS_PART_TBL_OFFSET);
+		for (i = 0; i < 4; i++, pt++) {
+			/*
+			 * fdisk does not show the extended partitions that
+			 * are not in the MBR
+			 */
+			if (((pt->boot_ind & ~0x80) == 0) &&
+			    (pt->sys_ind != 0) &&
+			    (part_num == which_part) &&
+			    (ext_part_sector == 0 || is_extended(pt->sys_ind) == 0)) {
+				if (info) {
+					if (wdinfo.blksz > DOS_PART_DEFAULT_SECTOR)
+						info->blksz = wdinfo.blksz;
+					else
+						info->blksz = DOS_PART_DEFAULT_SECTOR;
+					info->start = ext_part_sector +
+						get_unaligned_le32(&pt->start_sect);
+					info->size = get_unaligned_le32(&pt->nr_sects);
+					part_set_generic_name(desc, part_num,
+							      (char *)info->name);
+					snprintf(info->type, sizeof(info->type),
+						 "0x%x", pt->sys_ind);
+					info->bootable = get_bootable(pt);
+					if (CONFIG_IS_ENABLED(PARTITION_UUIDS)) {
+						char str[12];
+
+						sprintf(str, "%08x-%02x", disksig, part_num);
+						disk_partition_set_uuid(info, str);
+					}
+					info->sys_ind = pt->sys_ind;
 				}
-				info->sys_ind = pt->sys_ind;
+				if (mbr)
+					memcpy(mbr, pt, sizeof(*mbr));
+				return 0;
 			}
-			if (mbr)
-				memcpy(mbr, pt, sizeof(*mbr));
-			return 0;
-		}
 
-		/* Reverse engr the fdisk part# assignment rule! */
-		if ((ext_part_sector == 0) ||
-		    (pt->sys_ind != 0 && !is_extended (pt->sys_ind)) ) {
-			part_num++;
+			/* Reverse engr the fdisk part# assignment rule! */
+			if ((ext_part_sector == 0) ||
+			    (pt->sys_ind != 0 && !is_extended(pt->sys_ind))) {
+				part_num++;
+			}
 		}
-	}
+		empty = part_num > prev_num ? 0 : empty + 1;
 
-	/* Follows the extended partitions */
-	pt = (dos_partition_t *) (buffer + DOS_PART_TBL_OFFSET);
-	for (i = 0; i < 4; i++, pt++) {
-		if (is_extended (pt->sys_ind)) {
-			lbaint_t lba_start
-				= get_unaligned_le32 (&pt->start_sect) + relative;
-
-			return part_get_info_extended(desc, lba_start,
-				 ext_part_sector == 0 ? lba_start : relative,
-						      part_num, which_part, info,
-						      mbr, disksig);
-		}
+		/* Follows the extended partitions */
+		if (!dos_next_table(buffer, &ext_part_sector, &relative))
+			break;
 	}
 
 	/* Check for DOS PBR if no partition is found */
diff --git a/test/dm/part.c b/test/dm/part.c
index ad37d7f406f..4938d962144 100644
--- a/test/dm/part.c
+++ b/test/dm/part.c
@@ -3,10 +3,13 @@
  * Copyright (C) 2020 Sean Anderson <sean.anderson@seco.com>
  */
 
+#include <blk.h>
 #include <dm.h>
 #include <env.h>
+#include <memalign.h>
 #include <mmc.h>
 #include <part.h>
+#include <part_dos.h>
 #include <part_efi.h>
 #include <dm/test.h>
 #include <test/ut.h>
@@ -248,3 +251,175 @@ static int dm_test_part_get_info_by_uuid(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_part_get_info_by_uuid, UTF_SCAN_PDATA | UTF_SCAN_FDT);
+
+/**
+ * dos_set_entry() - fill in one entry of a DOS partition table
+ *
+ * @pt:		entry to fill in
+ * @sys_ind:	partition type
+ * @start:	first sector, relative to what the table type says
+ * @size:	number of sectors
+ */
+static void dos_set_entry(dos_partition_t *pt, u8 sys_ind, u32 start,
+			  u32 size)
+{
+	memset(pt, '\0', sizeof(*pt));
+	pt->sys_ind = sys_ind;
+	pt->start_sect = cpu_to_le32(start);
+	pt->nr_sects = cpu_to_le32(size);
+}
+
+/**
+ * dos_write_table() - write a DOS partition table to a sector
+ *
+ * @desc:	block device to write to, with 512-byte blocks
+ * @lba:	sector to write the table to
+ * @pt:		the four entries of the table
+ * Return: 0 if OK, -EIO if the sector could not be written
+ */
+static int dos_write_table(struct blk_desc *desc, lbaint_t lba,
+			   const dos_partition_t pt[4])
+{
+	ALLOC_CACHE_ALIGN_BUFFER(u8, buf, 512);
+
+	memset(buf, '\0', 512);
+	memcpy(buf + DOS_PART_TBL_OFFSET, pt, 4 * sizeof(*pt));
+	buf[DOS_PART_MAGIC_OFFSET] = 0x55;
+	buf[DOS_PART_MAGIC_OFFSET + 1] = 0xaa;
+
+	return blk_dwrite(desc, lba, 1, buf) == 1 ? 0 : -EIO;
+}
+
+/*
+ * A link from one extended partition table to the next adds no partition,
+ * so a walk through tables that link to each other in a loop finds no more
+ * partitions as it goes. Check that it ends anyway, both when looking for a
+ * partition and when listing them, and that a table holding only a link is
+ * still followed.
+ */
+static int dm_test_part_dos_ext_loop(struct unit_test_state *uts)
+{
+	struct disk_partition info;
+	dos_partition_t pt[4];
+	struct blk_desc *desc;
+
+	ut_asserteq(2, blk_get_device_by_str("mmc", "2", &desc));
+	ut_asserteq(512, desc->blksz);
+
+	/* an MBR with one extended partition, from sector 2 */
+	memset(pt, '\0', sizeof(pt));
+	dos_set_entry(&pt[0], DOS_PART_TYPE_EXTENDED, 2, 100);
+	ut_assertok(dos_write_table(desc, 0, pt));
+
+	/*
+	 * logical partition 5 in a table of its own, then a table holding only
+	 * a link, then logical partition 6; a link is relative to the extended
+	 * partition, a partition to its own table
+	 */
+	dos_set_entry(&pt[0], 0x83, 1, 4);
+	dos_set_entry(&pt[1], DOS_PART_TYPE_EXTENDED, 10, 10);
+	ut_assertok(dos_write_table(desc, 2, pt));
+	memset(pt, '\0', sizeof(pt));
+	dos_set_entry(&pt[0], DOS_PART_TYPE_EXTENDED, 20, 10);
+	ut_assertok(dos_write_table(desc, 12, pt));
+	dos_set_entry(&pt[0], 0x83, 1, 4);
+	dos_set_entry(&pt[1], DOS_PART_TYPE_EXTENDED, 30, 10);
+	ut_assertok(dos_write_table(desc, 22, pt));
+
+	/* then a table with no partition, only a link back to itself */
+	memset(pt, '\0', sizeof(pt));
+	dos_set_entry(&pt[0], DOS_PART_TYPE_EXTENDED, 30, 10);
+	ut_assertok(dos_write_table(desc, 32, pt));
+
+	ut_assertok(part_get_info_by_type(desc, 5, PART_TYPE_DOS, &info));
+	ut_asserteq(3, info.start);
+	ut_asserteq(4, info.size);
+	ut_assertok(part_get_info_by_type(desc, 6, PART_TYPE_DOS, &info));
+	ut_asserteq(23, info.start);
+	ut_asserteq(4, info.size);
+
+	/* there is no partition 7, and looking for it has to end */
+	ut_asserteq(-ENOENT,
+		    part_get_info_by_type(desc, 7, PART_TYPE_DOS, &info));
+	ut_assert_nextline("** Nested DOS partitions detected, stopping **");
+	ut_assert_console_end();
+
+	/* and so does listing them, once the table is detected as DOS */
+	part_init(desc);
+	ut_asserteq(PART_TYPE_DOS, desc->part_type);
+	part_print(desc);
+	ut_assert_skip_to_line("%3d\t%-10d\t%-10d\t%08x-%02x\t%02x", 6, 23, 4,
+			       0, 6, 0x83);
+	ut_assert_nextline("** Nested DOS partitions detected, stopping **");
+	ut_assert_console_end();
+
+	return 0;
+}
+DM_TEST(dm_test_part_dos_ext_loop, UTF_SCAN_PDATA | UTF_SCAN_FDT | UTF_CONSOLE);
+
+/**
+ * dos_write_chain() - write a chain of extended partition tables
+ *
+ * The MBR holds an extended partition from sector 2, whose first table
+ * holds logical partition 5 and a link, then come @empty tables holding only
+ * a link, and then a table holding logical partition 6.
+ *
+ * @desc:	block device to write to, with 512-byte blocks
+ * @empty:	number of tables holding only a link
+ * Return: sector where partition 6 starts, or -EIO on error
+ */
+static int dos_write_chain(struct blk_desc *desc, int empty)
+{
+	dos_partition_t pt[4];
+	int i;
+
+	memset(pt, '\0', sizeof(pt));
+	dos_set_entry(&pt[0], DOS_PART_TYPE_EXTENDED, 2, 1000);
+	if (dos_write_table(desc, 0, pt))
+		return -EIO;
+
+	/* table n is at sector 2 + 10 * n, its link 10 * (n + 1) */
+	for (i = 0; i <= empty + 1; i++) {
+		memset(pt, '\0', sizeof(pt));
+		if (i == 0 || i == empty + 1)
+			dos_set_entry(&pt[0], 0x83, 1, 4);
+		if (i <= empty)
+			dos_set_entry(&pt[1], DOS_PART_TYPE_EXTENDED,
+				      10 * (i + 1), 10);
+		if (dos_write_table(desc, 2 + 10 * i, pt))
+			return -EIO;
+	}
+
+	return 2 + 10 * (empty + 1) + 1;
+}
+
+/*
+ * Check that a walk through the extended partition tables follows up to 15
+ * tables in a row holding no partition, as a damaged but valid chain may
+ * have, but gives up at the 16th
+ */
+static int dm_test_part_dos_ext_empty(struct unit_test_state *uts)
+{
+	struct disk_partition info;
+	struct blk_desc *desc;
+	int start;
+
+	ut_asserteq(2, blk_get_device_by_str("mmc", "2", &desc));
+	ut_asserteq(512, desc->blksz);
+
+	start = dos_write_chain(desc, 15);
+	ut_assert(start > 0);
+	ut_assertok(part_get_info_by_type(desc, 6, PART_TYPE_DOS, &info));
+	ut_asserteq(start, info.start);
+	ut_assert_console_end();
+
+	ut_assert(dos_write_chain(desc, 16) > 0);
+	ut_assertok(part_get_info_by_type(desc, 5, PART_TYPE_DOS, &info));
+	ut_asserteq(-ENOENT,
+		    part_get_info_by_type(desc, 6, PART_TYPE_DOS, &info));
+	ut_assert_nextline("** Nested DOS partitions detected, stopping **");
+	ut_assert_console_end();
+
+	return 0;
+}
+DM_TEST(dm_test_part_dos_ext_empty, UTF_SCAN_PDATA | UTF_SCAN_FDT | UTF_CONSOLE);

[PATCH v3 11/14] disk: part_dos: Make room for a three-digit partition number in the UUID

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

part_get_info_extended() makes up the UUID of a DOS partition as
"%08x-%02x" from the disk signature and the partition number, into a
buffer of 12 bytes. That is enough up to partition 255, but
MAX_EXT_PARTS lets the number reach 256, whose three digits make
sprintf() write the terminating NUL past the end of the buffer.

Size the buffer like the UUID it is copied into, and use snprintf().

Add a test with 256 partitions, which catches the overflow on sandbox
with CONFIG_ASAN.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)

 disk/part_dos.c |  6 ++++--
 test/dm/part.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/disk/part_dos.c b/disk/part_dos.c
index bc40d9d2829..3029ee0c31a 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -325,9 +325,11 @@ static int part_get_info_extended(struct blk_desc *desc,
 						 "0x%x", pt->sys_ind);
 					info->bootable = get_bootable(pt);
 					if (CONFIG_IS_ENABLED(PARTITION_UUIDS)) {
-						char str[12];
+						/* part_num may take three digits */
+						char str[UUID_STR_LEN + 1];
 
-						sprintf(str, "%08x-%02x", disksig, part_num);
+						snprintf(str, sizeof(str), "%08x-%02x",
+							 disksig, part_num);
 						disk_partition_set_uuid(info, str);
 					}
 					info->sys_ind = pt->sys_ind;
diff --git a/test/dm/part.c b/test/dm/part.c
index 4938d962144..f8a0fa974c1 100644
--- a/test/dm/part.c
+++ b/test/dm/part.c
@@ -423,3 +423,49 @@ static int dm_test_part_dos_ext_empty(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_part_dos_ext_empty, UTF_SCAN_PDATA | UTF_SCAN_FDT | UTF_CONSOLE);
+
+/*
+ * Partition numbers above 255 take three digits in the UUID made up for a
+ * DOS partition, from the disk signature and the partition number
+ */
+static int dm_test_part_dos_uuid(struct unit_test_state *uts)
+{
+	struct disk_partition info;
+	dos_partition_t pt[4];
+	struct blk_desc *desc;
+	int part;
+
+	if (!CONFIG_IS_ENABLED(PARTITION_UUIDS))
+		return -EAGAIN;
+
+	ut_asserteq(2, blk_get_device_by_str("mmc", "2", &desc));
+	ut_asserteq(512, desc->blksz);
+
+	/* an extended partition from sector 2 */
+	memset(pt, '\0', sizeof(pt));
+	dos_set_entry(&pt[0], DOS_PART_TYPE_EXTENDED, 2, 512);
+	ut_assertok(dos_write_table(desc, 0, pt));
+
+	/*
+	 * holding logical partitions 5 to 256, each of one sector after its
+	 * table, with the tables two sectors apart from sector 2 on
+	 */
+	for (part = 5; part <= 256; part++) {
+		memset(pt, '\0', sizeof(pt));
+		dos_set_entry(&pt[0], 0x83, 1, 1);
+		if (part < 256)
+			dos_set_entry(&pt[1], DOS_PART_TYPE_EXTENDED,
+				      (part - 4) * 2, 2);
+		ut_assertok(dos_write_table(desc, 2 + (part - 5) * 2, pt));
+	}
+
+	ut_assertok(part_get_info_by_type(desc, 255, PART_TYPE_DOS, &info));
+	ut_asserteq(2 + 250 * 2 + 1, info.start);
+	ut_asserteq_str("00000000-ff", disk_partition_uuid(&info));
+	ut_assertok(part_get_info_by_type(desc, 256, PART_TYPE_DOS, &info));
+	ut_asserteq(2 + 251 * 2 + 1, info.start);
+	ut_asserteq_str("00000000-100", disk_partition_uuid(&info));
+
+	return 0;
+}
+DM_TEST(dm_test_part_dos_uuid, UTF_SCAN_PDATA | UTF_SCAN_FDT);

[PATCH v3 12/14] efi_loader: Measure the GPT the partition driver uses

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

tcg2_measure_gpt_data() reads the primary GPT and measures it into
PCR 5 without checking it, while the partition driver uses the backup
GPT when the primary header or entries are not valid. So PCR 5 may hold
a GPT the partitions were not found in: a change to an unused primary
entry, which is not measured, leaves PCR 5 as it was while the
partitions come from the backup GPT. Nor are the sizes in the header
checked, so an entry array size that wraps in 32 bits makes the walks
read past the end of the entry buffer.

Measure the GPT the partitions are found in, read by the partition
driver's own code through a new part_get_gpt(), which checks the sizes
too. When the primary GPT is not valid, measure the backup header as
the primary header that 'gpt repair' writes from it, so that PCR 5 is
as for the repaired disk; a primary header marked to be ignored
(ChromeOS) is never repaired, so measure the backup header as it is
then. If no valid GPT is read, measure no GPT event but return success,
so that the separators are still measured.

EDK2 measures the GPT its partition driver uses too, since 7c12d4359abd
("SecurityPkg/DxeTpm2MeasureBootLib: Use GptLib parser"), with the
primary header that driver restores from the backup.

Add a sandbox test that checks PCR 5 for a valid GPT, an entry array
size that wraps, a backup GPT that differs from a broken primary one
and an ignored primary header, and check part_get_gpt() in
lib_test_gpt_sizes.

Fixes: ce3dbc5d080d ("efi_loader: add UEFI GPT measurement")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
Changes in v3:
- Rename part_get_gpt_primary() to part_get_gpt(), and say in its
  comment when a backup header is turned into a primary one (Simon)
- When the primary header is marked IGNOREME (ChromeOS), measure the
  backup header as it is, since gpt repair never rebuilds the primary
  one then, and correct the comment and commit message that said
  otherwise (Simon); test that case too
- Name the device in the warning when no valid GPT is read (Simon)
- Shorten the commit message (Simon)

Changes in v2:
- New subject: measure the GPT the partition driver uses, read with its
  own code through a new part_get_gpt_primary(), so that the CRC of the
  entries is checked too, as EDK2 does (Simon)
- When the partitions come from the backup GPT, measure that, with the
  primary header gpt repair makes from it, rather than no GPT, as EDK2
  measures the GPT its partition driver restores (Ilias)
- Say in the commit message what the bug does to PCR 5 (Ilias)
- Keep validate_gpt_header() static, as efi_tcg2 no longer reads the GPT
  itself
- Test a backup GPT that differs from the primary one, and rename the
  test to test_efi_tcg2_gpt

 disk/part_efi.c                |  27 ++++
 include/part.h                 |  27 ++++
 lib/efi_loader/efi_tcg2.c      |  52 ++++---
 test/lib/part_efi.c            |  61 +++++++-
 test/py/tests/test_efi_tcg2.py | 246 +++++++++++++++++++++++++++++++++
 5 files changed, 385 insertions(+), 28 deletions(-)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 85945634915..a274a268764 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -875,6 +875,33 @@ int gpt_repair_headers(struct blk_desc *desc)
 	return ret;
 }
 
+int part_get_gpt(struct blk_desc *desc, gpt_header *gpt_h, gpt_entry **gpt_pte)
+{
+	int r;
+
+	/* Find the GPT as find_valid_gpt() does */
+	r = is_gpt_valid(desc, GPT_PRIMARY_PARTITION_TABLE_LBA, gpt_h, gpt_pte);
+	if (r == 1)
+		return 0;
+
+	if (is_gpt_valid(desc, desc->lba - 1, gpt_h, gpt_pte) != 1) {
+		/* is_gpt_valid() may leave a freed array here */
+		*gpt_pte = NULL;
+		return -EINVAL;
+	}
+
+	/*
+	 * If the primary GPT is not valid, give the backup header as the
+	 * primary header that gpt_repair_headers() writes from it. A primary
+	 * header marked to be ignored (ChromeOS) is never repaired, so give
+	 * the backup header as it is then.
+	 */
+	if (r != 2)
+		restore_primary_gpt_header(gpt_h, desc);
+
+	return 0;
+}
+
 int gpt_verify_partitions(struct blk_desc *desc,
 			  struct disk_partition *partitions, int parts,
 			  gpt_header *gpt_head, gpt_entry **gpt_pte)
diff --git a/include/part.h b/include/part.h
index 1dc510cb6b1..4b1d40511db 100644
--- a/include/part.h
+++ b/include/part.h
@@ -680,6 +680,33 @@ int get_disk_guid(struct blk_desc *desc, char *guid);
  */
 int part_get_gpt_pte(struct blk_desc *desc, int part, gpt_entry *gpt_e);
 
+/**
+ * part_get_gpt() - Get the GPT the partitions of a disk are found in
+ *
+ * Read the GPT as the partition driver does: the primary one if it is valid,
+ * else the backup one. If the primary GPT is not valid, the backup header is
+ * turned into the primary header that gpt_repair_headers() writes from it. If
+ * the primary header is marked to be ignored (ChromeOS 'IGNOREME'), the backup
+ * header is given as it is. Nothing is written to the disk.
+ *
+ * @desc:	block device descriptor
+ * @gpt_h:	buffer of @desc->blksz bytes, filled with the header
+ * @gpt_pte:	set to the partition entries, which the caller frees, or to
+ *		NULL on error
+ *
+ * Return:	0 on success, -EINVAL if no valid GPT was read from the disk,
+ *		-ENOSYS if GPT support is not built in
+ */
+int part_get_gpt(struct blk_desc *desc, gpt_header *gpt_h, gpt_entry **gpt_pte);
+
+#else
+static inline int part_get_gpt(struct blk_desc *desc, gpt_header *gpt_h,
+			       gpt_entry **gpt_pte)
+{
+	*gpt_pte = NULL;
+
+	return -ENOSYS;
+}
 #endif
 
 #if CONFIG_IS_ENABLED(DOS_PARTITION)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 5f9bbe97455..97fce462138 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -16,6 +16,8 @@
 #include <efi_tcg2.h>
 #include <log.h>
 #include <malloc.h>
+#include <memalign.h>
+#include <part.h>
 #include <smbios.h>
 #include <version_string.h>
 #include <tpm_api.h>
@@ -1157,11 +1159,11 @@ tcg2_measure_gpt_data(struct udevice *dev,
 {
 	efi_status_t ret;
 	efi_handle_t handle;
-	struct efi_handler *dp_handler, *io_handler;
+	struct efi_handler *dp_handler;
 	struct efi_device_path *orig_device_path;
 	struct efi_device_path *device_path;
 	struct efi_device_path *dp;
-	struct efi_block_io *block_io;
+	struct blk_desc *desc;
 	struct efi_gpt_data *event = NULL;
 	efi_guid_t null_guid = NULL_GUID;
 	gpt_header *gpt_h;
@@ -1170,7 +1172,6 @@ tcg2_measure_gpt_data(struct udevice *dev,
 	u32 num_of_valid_entry = 0;
 	u32 event_size;
 	u32 i;
-	u32 total_gpt_entry_size;
 
 	ret = efi_search_protocol(&loaded_image->header,
 				  &efi_guid_loaded_image_device_path,
@@ -1202,40 +1203,37 @@ tcg2_measure_gpt_data(struct udevice *dev,
 	if (ret != EFI_SUCCESS)
 		goto out1;
 
-	ret = efi_search_protocol(handle, &efi_block_io_guid, &io_handler);
-	if (ret != EFI_SUCCESS)
+	/*
+	 * Measure the GPT the partitions were found in: read it with the
+	 * partition driver's own code, which checks it and uses the backup GPT
+	 * if the primary one is not valid. Only a block device of U-Boot's has
+	 * partitions found by it.
+	 */
+	if (!handle->dev || device_get_uclass_id(handle->dev) != UCLASS_BLK) {
+		log_debug("Not a U-Boot block device, GPT not measured\n");
+		ret = EFI_SUCCESS;
 		goto out1;
-	block_io = io_handler->protocol_interface;
+	}
+	desc = dev_get_uclass_plat(handle->dev);
 
-	gpt_h = memalign(block_io->media->io_align, block_io->media->block_size);
+	gpt_h = malloc_cache_aligned(desc->blksz);
 	if (!gpt_h) {
 		ret = EFI_OUT_OF_RESOURCES;
 		goto out2;
 	}
 
-	ret = EFI_CALL(block_io->read_blocks(block_io,
-					     block_io->media->media_id, 1,
-					     block_io->media->block_size,
-					     gpt_h));
-	if (ret != EFI_SUCCESS)
-		goto out2;
-
-	/* read GPT entry */
-	total_gpt_entry_size = gpt_h->num_partition_entries *
-			       gpt_h->sizeof_partition_entry;
-	entry = memalign(block_io->media->io_align, total_gpt_entry_size);
-	if (!entry) {
-		ret = EFI_OUT_OF_RESOURCES;
+	/*
+	 * The disk may have changed since its partitions were found, or not
+	 * read. Measure no GPT then, but return success, as an error would
+	 * skip the separators.
+	 */
+	if (part_get_gpt(desc, gpt_h, &entry)) {
+		log_warning("%s: No valid GPT read, not measured\n",
+			    handle->dev->name);
+		ret = EFI_SUCCESS;
 		goto out2;
 	}
 
-	ret = EFI_CALL(block_io->read_blocks(block_io,
-					     block_io->media->media_id,
-					     gpt_h->partition_entry_lba,
-					     total_gpt_entry_size, entry));
-	if (ret != EFI_SUCCESS)
-		goto out2;
-
 	/* count valid GPT entry */
 	gpt_e = entry;
 	for (i = 0; i < gpt_h->num_partition_entries; i++) {
diff --git a/test/lib/part_efi.c b/test/lib/part_efi.c
index 42de9109360..696b73f28a7 100644
--- a/test/lib/part_efi.c
+++ b/test/lib/part_efi.c
@@ -328,6 +328,7 @@ static int gpt_sizes_check(struct unit_test_state *uts, struct udevice *dev,
 	struct blk_desc *desc;
 	struct udevice *blk;
 	gpt_entry *gpt_pte;
+	gpt_header orig, written, backup;
 	void *entries;
 	int i, ret;
 
@@ -341,6 +342,8 @@ static int gpt_sizes_check(struct unit_test_state *uts, struct udevice *dev,
 	disk_partition_set_uuid(&parts[0],
 				"a2a0d0eb-e5b9-3344-87c0-68b6b72699c7");
 	ut_assertok(gpt_restore(desc, str_disk_guid, parts, 1));
+	ut_asserteq(1, blk_dread(desc, GPT_PRIMARY_PARTITION_TABLE_LBA, 1, hdr));
+	memcpy(&orig, hdr, sizeof(orig));
 
 	for (i = 0; i < ARRAY_SIZE(cases); i++) {
 		u32 header_size = cases[i].header_size;
@@ -365,6 +368,22 @@ static int gpt_sizes_check(struct unit_test_state *uts, struct udevice *dev,
 						      header_size));
 		ut_asserteq(1, blk_dwrite(desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
 					  1, hdr));
+		memcpy(&written, hdr, sizeof(written));
+
+		/*
+		 * efi_tcg2 measures the GPT this gives: the header just
+		 * written if it is valid, else the backup header turned into
+		 * the primary one, which is then the header gpt_restore() wrote
+		 */
+		memset(hdr, '\0', 2 * blksz);
+		gpt_pte = NULL;
+		ret = part_get_gpt(desc, hdr, &gpt_pte);
+		free(gpt_pte);
+		ut_assertf(!ret, "case %d: GPT not found: %d\n", i, ret);
+		if (cases[i].ret)
+			ut_asserteq_mem(&orig, hdr, sizeof(gpt_header));
+		else
+			ut_asserteq_mem(&written, hdr, sizeof(gpt_header));
 
 		/*
 		 * Read it back into a buffer as large as the one above, so
@@ -396,6 +415,45 @@ static int gpt_sizes_check(struct unit_test_state *uts, struct udevice *dev,
 		ut_asserteq_64(parts[0].size, info.size);
 	}
 
+	/*
+	 * A primary header marked to be ignored is never repaired, so the
+	 * backup header is given as it is
+	 */
+	memset(hdr, '\0', 2 * blksz);
+	memcpy(hdr, &orig, sizeof(orig));
+	hdr->signature = cpu_to_le64(GPT_HEADER_CHROMEOS_IGNORE);
+	ut_asserteq(1, blk_dwrite(desc, GPT_PRIMARY_PARTITION_TABLE_LBA, 1,
+				  hdr));
+	ut_asserteq(1, blk_dread(desc, lba - 1, 1, hdr));
+	memcpy(&backup, hdr, sizeof(backup));
+	memset(hdr, '\0', 2 * blksz);
+	gpt_pte = NULL;
+	ret = part_get_gpt(desc, hdr, &gpt_pte);
+	free(gpt_pte);
+	ut_assertok(ret);
+	ut_asserteq_64(lba - 1, le64_to_cpu(hdr->my_lba));
+	ut_asserteq_mem(&backup, hdr, sizeof(gpt_header));
+
+	/*
+	 * With the primary header valid again but not its entries, and the
+	 * backup header broken, there is no GPT left, and no entries either
+	 */
+	memset(hdr, '\0', 2 * blksz);
+	memcpy(hdr, &orig, sizeof(orig));
+	ut_asserteq(1, blk_dwrite(desc, GPT_PRIMARY_PARTITION_TABLE_LBA, 1,
+				  hdr));
+	ut_asserteq(1, blk_dread(desc, le64_to_cpu(orig.partition_entry_lba),
+				 1, hdr));
+	((gpt_entry *)hdr)->partition_name[0] ^= 1;
+	ut_asserteq(1, blk_dwrite(desc, le64_to_cpu(orig.partition_entry_lba),
+				  1, hdr));
+	ut_asserteq(1, blk_dread(desc, lba - 1, 1, hdr));
+	hdr->signature = 0;
+	ut_asserteq(1, blk_dwrite(desc, lba - 1, 1, hdr));
+	gpt_pte = (gpt_entry *)hdr;
+	ut_asserteq(-EINVAL, part_get_gpt(desc, hdr, &gpt_pte));
+	ut_assertnull(gpt_pte);
+
 	return 0;
 }
 
@@ -403,7 +461,8 @@ static int gpt_sizes_check(struct unit_test_state *uts, struct udevice *dev,
  * A GPT header gives its own size and the size of its partition entries, and
  * its CRCs cover only what those say, so a CRC does not show that either size
  * is one the parser can use. Check that a primary header with a size the
- * parser cannot use is refused, CRCs or not, and that the backup is used.
+ * parser cannot use is refused, CRCs or not, and that the backup is used, for
+ * the GPT efi_tcg2 measures too.
  */
 static int lib_test_gpt_sizes(struct unit_test_state *uts)
 {
diff --git a/test/py/tests/test_efi_tcg2.py b/test/py/tests/test_efi_tcg2.py
new file mode 100644
index 00000000000..4a17b839ab4
--- /dev/null
+++ b/test/py/tests/test_efi_tcg2.py
@@ -0,0 +1,246 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+"""Test the measurements made by the EFI_TCG2_PROTOCOL on sandbox
+
+The sandbox TPM starts with every PCR clear, and the events measured when an
+EFI application is started (boot variables, GPT, separators) are only measured
+the first time one is. So each case restarts U-Boot, starts one application
+and checks PCR 5 against the events it should then hold.
+"""
+
+import hashlib
+import os
+import re
+import shutil
+import struct
+import zlib
+
+import pytest
+
+import utils
+from tests.fs_helper import FsHelper
+
+# EFI System Partition
+ESP_TYPE = 'C12A7328-F81F-11D2-BA4B-00A0C93EC93B'
+SECTOR = 512
+
+# Fields of the GPT header, by offset
+GPT_HEADER_SIZE = 92
+GPT_HDR_SIZE_OFF = 12
+GPT_HDR_CRC_OFF = 16
+GPT_ALTERNATE_OFF = 32
+GPT_ENTRIES_OFF = 72    # partition_entry_lba, num_partition_entries, ...
+GPT_NUM_ENTRIES_OFF = 80
+GPT_ENTRY_SIZE = 128
+
+
+def extend(pcr, data):
+    """Extend a SHA256 PCR value with the digest of some data
+
+    Args:
+        pcr (bytes): PCR value before
+        data (bytes): Event data
+
+    Returns:
+        bytes: PCR value after
+    """
+    return hashlib.sha256(pcr + hashlib.sha256(data).digest()).digest()
+
+
+def make_gpt_disk(ubman, fname):
+    """Write a GPT disk image with helloworld.efi on its only partition
+
+    Args:
+        ubman (ConsoleBase): U-Boot console
+        fname (str): Filename of the disk image
+    """
+    with FsHelper(ubman.config, 'vfat', 1, 'tcg2') as fsh:
+        shutil.copy(os.path.join(ubman.config.build_dir,
+                                 'lib/efi_loader/helloworld.efi'),
+                    fsh.srcdir)
+        fsh.mk_fs()
+        with open(fsh.fs_img, 'rb') as inf:
+            fs_data = inf.read()
+
+    # 1MiB for the primary GPT, the partition, 1MiB for the backup GPT
+    with open(fname, 'wb') as outf:
+        outf.truncate((2 << 20) + len(fs_data))
+    utils.run_and_log(ubman, f'sfdisk {fname}',
+                      stdin=(f'label: gpt\nstart=2048, '
+                             f'size={len(fs_data) // SECTOR}, '
+                             f'type={ESP_TYPE}\n').encode())
+    with open(fname, 'r+b') as outf:
+        outf.seek(1 << 20)
+        outf.write(fs_data)
+
+
+def gpt_event(fname, backup=False):
+    """Get the EV_EFI_GPT_EVENT data for a GPT of a disk image
+
+    Args:
+        fname (str): Filename of the disk image
+        backup (bool): True for the backup GPT, as it is on the disk, else
+            the primary one
+
+    Returns:
+        bytes: The header, the number of partitions, then their entries
+    """
+    with open(fname, 'rb') as inf:
+        disk = inf.read()
+    start = (len(disk) - SECTOR) if backup else SECTOR
+    hdr = disk[start:start + GPT_HEADER_SIZE]
+    entry_lba, num, size = struct.unpack_from('<QII', hdr, GPT_ENTRIES_OFF)
+    entries = []
+    for i in range(num):
+        start = entry_lba * SECTOR + i * size
+        entry = disk[start:start + size]
+        if any(entry[:16]):
+            entries.append(entry)
+    return hdr + struct.pack('<Q', len(entries)) + b''.join(entries)
+
+
+def set_num_entries(fname, num):
+    """Set num_partition_entries in the primary GPT header and fix its CRC
+
+    Args:
+        fname (str): Filename of the disk image
+        num (int): Number of partition entries
+    """
+    with open(fname, 'r+b') as outf:
+        outf.seek(SECTOR)
+        hdr = bytearray(outf.read(SECTOR))
+        struct.pack_into('<I', hdr, GPT_NUM_ENTRIES_OFF, num)
+        struct.pack_into('<I', hdr, GPT_HDR_CRC_OFF, 0)
+        hdr_size = struct.unpack_from('<I', hdr, GPT_HDR_SIZE_OFF)[0]
+        struct.pack_into('<I', hdr, GPT_HDR_CRC_OFF,
+                         zlib.crc32(hdr[:hdr_size]))
+        outf.seek(SECTOR)
+        outf.write(hdr)
+
+
+def ignore_primary(fname):
+    """Mark the primary GPT header to be ignored, as ChromeOS does
+
+    Args:
+        fname (str): Filename of the disk image
+    """
+    with open(fname, 'r+b') as outf:
+        outf.seek(SECTOR)
+        outf.write(b'IGNOREME')
+
+
+def corrupt_entries(fname):
+    """Change an unused entry of the primary GPT, leaving the header as it is
+
+    The CRC of the entries in the header no longer matches them.
+
+    Args:
+        fname (str): Filename of the disk image
+    """
+    with open(fname, 'r+b') as outf:
+        outf.seek(SECTOR)
+        hdr = outf.read(GPT_HEADER_SIZE)
+        entry_lba = struct.unpack_from('<Q', hdr, GPT_ENTRIES_OFF)[0]
+        # the name of the second entry, whose type GUID stays zero
+        outf.seek(entry_lba * SECTOR + GPT_ENTRY_SIZE + 56)
+        outf.write(b'x')
+
+
+def copy_backup_gpt(src, dst):
+    """Copy the backup GPT of a disk image to another of the same size
+
+    Args:
+        src (str): Filename of the disk image to copy it from
+        dst (str): Filename of the disk image to copy it to
+    """
+    with open(src, 'rb') as inf:
+        inf.seek(SECTOR)
+        hdr = inf.read(GPT_HEADER_SIZE)
+        backup_lba = struct.unpack_from('<Q', hdr, GPT_ALTERNATE_OFF)[0]
+        inf.seek(backup_lba * SECTOR)
+        hdr = inf.read(GPT_HEADER_SIZE)
+        entry_lba = struct.unpack_from('<Q', hdr, GPT_ENTRIES_OFF)[0]
+        inf.seek(entry_lba * SECTOR)
+        data = inf.read()
+    with open(dst, 'r+b') as outf:
+        outf.seek(entry_lba * SECTOR)
+        outf.write(data)
+
+
+def boot_and_read_pcr5(ubman, fname):
+    """Start helloworld.efi from a disk image on a fresh U-Boot
+
+    Args:
+        ubman (ConsoleBase): U-Boot console
+        fname (str): Filename of the disk image
+
+    Returns:
+        bytes: PCR 5 after the application has run
+    """
+    addr = utils.find_ram_base(ubman) + (4 << 20)
+    ubman.restart_uboot()
+    ubman.run_command(f'host bind 0 {fname}')
+    output = ubman.run_command(f'load host 0:1 {addr:x} helloworld.efi')
+    assert 'bytes read' in output
+    output = ubman.run_command(f'bootefi {addr:x}')
+    assert 'Hello, world!' in output
+    output = ubman.run_command(f'tpm2 pcr_read 5 {addr:x}')
+    assert 'PCR #5 sha256 32 byte content' in output
+    digest = output.split('content', 1)[1].split('\n', 1)[1]
+    return bytes.fromhex(''.join(re.findall(r'\b[0-9a-f]{2}\b', digest)))
+
+
+@pytest.mark.boardspec('sandbox')
+# EFI applications are built with the sanitizer but cannot run with it
+@pytest.mark.notbuildconfigspec('asan')
+@pytest.mark.buildconfigspec('efi_tcg2_protocol')
+@pytest.mark.buildconfigspec('efi_partition')
+@pytest.mark.buildconfigspec('cmd_tpm_v2')
+@pytest.mark.buildconfigspec('bootefi_hello_compile')
+def test_efi_tcg2_gpt(ubman):
+    """Check that PCR 5 holds the GPT the partitions are found in
+
+    PCR 5 holds the GPT event and then the separator. When the partition
+    driver refuses the primary GPT, it finds the partitions in the backup
+    one, and the GPT event holds that, with the primary header it restores,
+    or the backup header as it is when the primary one is marked to be
+    ignored.
+    """
+    fname = os.path.join(ubman.config.persistent_data_dir, 'tcg2_gpt.img')
+    other = os.path.join(ubman.config.persistent_data_dir, 'tcg2_other.img')
+    separator = struct.pack('<I', 0)
+    try:
+        make_gpt_disk(ubman, fname)
+        expect = extend(extend(bytes(32), gpt_event(fname)), separator)
+        assert boot_and_read_pcr5(ubman, fname) == expect
+
+        # an entry array whose size comes to one block in 32 bits: the
+        # backup GPT is the same GPT, so PCR 5 is too
+        set_num_entries(fname, (1 << 32) // GPT_ENTRY_SIZE +
+                        SECTOR // GPT_ENTRY_SIZE)
+        assert boot_and_read_pcr5(ubman, fname) == expect
+
+        # a backup GPT whose partition has another GUID, and primary entries
+        # that do not match their CRC, though the change is in an unused
+        # entry, which is not measured
+        make_gpt_disk(ubman, fname)
+        shutil.copy(fname, other)
+        utils.run_and_log(ubman, f'sfdisk --part-uuid {other} 1 '
+                          '0a6a1f54-1b55-4d0c-9a52-6d1e5e0e5d01')
+        expect = extend(extend(bytes(32), gpt_event(other)), separator)
+        copy_backup_gpt(other, fname)
+        corrupt_entries(fname)
+        assert boot_and_read_pcr5(ubman, fname) == expect
+
+        # a primary header marked to be ignored, which is never repaired:
+        # PCR 5 holds the backup GPT as it is
+        make_gpt_disk(ubman, fname)
+        ignore_primary(fname)
+        expect = extend(extend(bytes(32), gpt_event(fname, backup=True)),
+                        separator)
+        assert boot_and_read_pcr5(ubman, fname) == expect
+    finally:
+        for path in (fname, other):
+            if os.path.exists(path):
+                os.remove(path)
+        ubman.restart_uboot()

[PATCH v3 13/14] efi_loader: Index the GPT entries directly in tcg2_measure_gpt_data()

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

tcg2_measure_gpt_data() now measures the GPT that part_get_gpt() gives,
whose entries are always sizeof(gpt_entry) bytes, as
validate_gpt_header() refuses any other size. So index them as an array
rather than stepping by sizeof_partition_entry, and read
num_partition_entries with le32_to_cpu(), as the rest of the GPT code
does.

No functional change on a little-endian machine.

Suggested-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
Changes in v3:
- New patch (Simon)

 lib/efi_loader/efi_tcg2.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 97fce462138..4045af9afa3 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -1168,8 +1168,8 @@ tcg2_measure_gpt_data(struct udevice *dev,
 	efi_guid_t null_guid = NULL_GUID;
 	gpt_header *gpt_h;
 	gpt_entry *entry = NULL;
-	gpt_entry *gpt_e;
 	u32 num_of_valid_entry = 0;
+	u32 num_entries;
 	u32 event_size;
 	u32 i;
 
@@ -1235,17 +1235,15 @@ tcg2_measure_gpt_data(struct udevice *dev,
 	}
 
 	/* count valid GPT entry */
-	gpt_e = entry;
-	for (i = 0; i < gpt_h->num_partition_entries; i++) {
-		if (guidcmp(&null_guid, &gpt_e->partition_type_guid))
+	num_entries = le32_to_cpu(gpt_h->num_partition_entries);
+	for (i = 0; i < num_entries; i++) {
+		if (guidcmp(&null_guid, &entry[i].partition_type_guid))
 			num_of_valid_entry++;
-
-		gpt_e = (gpt_entry *)((u8 *)gpt_e + gpt_h->sizeof_partition_entry);
 	}
 
 	/* prepare event data for measurement */
 	event_size = sizeof(struct efi_gpt_data) +
-		(num_of_valid_entry * gpt_h->sizeof_partition_entry);
+		(num_of_valid_entry * sizeof(gpt_entry));
 	event = calloc(1, event_size);
 	if (!event) {
 		ret = EFI_OUT_OF_RESOURCES;
@@ -1255,17 +1253,13 @@ tcg2_measure_gpt_data(struct udevice *dev,
 	put_unaligned_le64(num_of_valid_entry, &event->number_of_partitions);
 
 	/* copy valid GPT entry */
-	gpt_e = entry;
 	num_of_valid_entry = 0;
-	for (i = 0; i < gpt_h->num_partition_entries; i++) {
-		if (guidcmp(&null_guid, &gpt_e->partition_type_guid)) {
-			memcpy((u8 *)event->partitions +
-			       (num_of_valid_entry * gpt_h->sizeof_partition_entry),
-			       gpt_e, gpt_h->sizeof_partition_entry);
+	for (i = 0; i < num_entries; i++) {
+		if (guidcmp(&null_guid, &entry[i].partition_type_guid)) {
+			memcpy(&event->partitions[num_of_valid_entry],
+			       &entry[i], sizeof(gpt_entry));
 			num_of_valid_entry++;
 		}
-
-		gpt_e = (gpt_entry *)((u8 *)gpt_e + gpt_h->sizeof_partition_entry);
 	}
 
 	ret = measure_event(dev, 5, EV_EFI_GPT_EVENT, event_size, (u8 *)event);

[PATCH v3 14/14] mmc: Always use a 512-byte block length

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

mmc_startup() sets the block size of the card's block device from the
READ_BL_LEN field of its CSD and only caps it at 512 bytes from above. A
card reporting a READ_BL_LEN below 9, which the SD specification
reserves, gives the block device a block size below 512 bytes. A block
that small cannot hold an MBR or a GPT header, and the partition and
filesystem code assumes at least 512 bytes, as the patch adding
blk_valid_blksz() describes. Nor does the code bound an MMC card's
WRITE_BL_LEN from below, though mmc_bwrite() then writes blocks of that
length where the block layer passes 512-byte ones.

The check in blk_post_probe() does not catch this. It is left out of
SPL, TPL and VPL, which commonly boot from MMC. In U-Boot proper,
mmc_startup() runs again whenever the card is initialised again, by
'mmc rescan' or 'mmc dev', or after the card-detect poll has seen it
removed, and sets the block size of a block device that is already
probed.

Use 512-byte blocks whatever READ_BL_LEN and WRITE_BL_LEN say, as the
code already did in DDR mode and as Linux does for its transfers; the
capacity is still computed with the block length the card reports. Only
some old MultiMediaCards could have blocks that really are smaller, and
the partition and filesystem code could not use those either. This makes
the DDR case redundant, so drop it. The SPL does not grow: imx28_btt3's,
close to its size limit, gets 16 bytes smaller.

Make the READ_BL_LEN that the sandbox MMC emulator reports settable, and
test that a card initialised again with a READ_BL_LEN of 0, 8 or 11 gets
512-byte blocks and can be written and read back.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
Changes in v3:
- New patch: blk_post_probe() does not see the block size the MMC code
  sets when it initialises a card again, as the v2 cover letter said

 arch/sandbox/include/asm/test.h | 10 +++++++++
 drivers/mmc/mmc.c               | 34 +++++++++-------------------
 drivers/mmc/sandbox_mmc.c       | 11 ++++++++-
 test/dm/mmc.c                   | 40 +++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 24 deletions(-)

diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h
index 76e2f11b059..f73bf51ef35 100644
--- a/arch/sandbox/include/asm/test.h
+++ b/arch/sandbox/include/asm/test.h
@@ -143,6 +143,16 @@ void sandbox_flash_set_block_size(struct udevice *dev, int block_size);
  */
 void sandbox_scsi_set_block_size(struct udevice *dev, int block_size);
 
+/**
+ * sandbox_mmc_set_bl_len_shift() - Set the READ_BL_LEN an SD card reports
+ *
+ * @dev:	Sandbox MMC controller
+ * @bl_len_shift: log2 of the block length, as the CSD's READ_BL_LEN field
+ *
+ * The host reads it when it next initialises the card.
+ */
+void sandbox_mmc_set_bl_len_shift(struct udevice *dev, int bl_len_shift);
+
 /**
  * sandbox_osd_get_mem() - get the internal memory of a sandbox OSD
  *
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 4835ce0c64b..90331823926 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -2555,7 +2555,7 @@ error:
 static int mmc_startup(struct mmc *mmc)
 {
 	int err, i;
-	uint mult, freq;
+	uint mult, freq, bl_len;
 	u64 cmult, csize;
 	struct mmc_cmd cmd;
 	struct blk_desc *bdesc;
@@ -2652,14 +2652,7 @@ static int mmc_startup(struct mmc *mmc)
 	mmc_select_mode(mmc, MMC_LEGACY);
 
 	mmc->dsr_imp = ((cmd.response[1] >> 12) & 0x1);
-	mmc->read_bl_len = 1 << ((cmd.response[1] >> 16) & 0xf);
-#if CONFIG_IS_ENABLED(MMC_WRITE)
-
-	if (IS_SD(mmc))
-		mmc->write_bl_len = mmc->read_bl_len;
-	else
-		mmc->write_bl_len = 1 << ((cmd.response[3] >> 22) & 0xf);
-#endif
+	bl_len = 1 << ((cmd.response[1] >> 16) & 0xf);
 
 	if (mmc->high_capacity) {
 		csize = (mmc->csd[1] & 0x3f) << 16
@@ -2672,18 +2665,21 @@ static int mmc_startup(struct mmc *mmc)
 	}
 
 	mmc->capacity_user = (csize + 1) << (cmult + 2);
-	mmc->capacity_user *= mmc->read_bl_len;
+	mmc->capacity_user *= bl_len;
 	mmc->capacity_boot = 0;
 	mmc->capacity_rpmb = 0;
 	for (i = 0; i < 4; i++)
 		mmc->capacity_gp[i] = 0;
 
-	if (mmc->read_bl_len > MMC_MAX_BLOCK_LEN)
-		mmc->read_bl_len = MMC_MAX_BLOCK_LEN;
-
+	/*
+	 * Use 512-byte blocks whatever READ_BL_LEN and WRITE_BL_LEN say. A
+	 * larger value is the most the card takes in one block, and a smaller
+	 * one, which the SD specification reserves, would give the block
+	 * device a block size the partition and filesystem code cannot use.
+	 */
+	mmc->read_bl_len = MMC_MAX_BLOCK_LEN;
 #if CONFIG_IS_ENABLED(MMC_WRITE)
-	if (mmc->write_bl_len > MMC_MAX_BLOCK_LEN)
-		mmc->write_bl_len = MMC_MAX_BLOCK_LEN;
+	mmc->write_bl_len = MMC_MAX_BLOCK_LEN;
 #endif
 
 	if ((mmc->dsr_imp) && (0xffffffff != mmc->dsr)) {
@@ -2765,14 +2761,6 @@ static int mmc_startup(struct mmc *mmc)
 
 	mmc->best_mode = mmc->selected_mode;
 
-	/* Fix the block length for DDR mode */
-	if (mmc->ddr_mode) {
-		mmc->read_bl_len = MMC_MAX_BLOCK_LEN;
-#if CONFIG_IS_ENABLED(MMC_WRITE)
-		mmc->write_bl_len = MMC_MAX_BLOCK_LEN;
-#endif
-	}
-
 	/* fill in device description */
 	bdesc = mmc_get_blk_desc(mmc);
 	bdesc->lun = 0;
diff --git a/drivers/mmc/sandbox_mmc.c b/drivers/mmc/sandbox_mmc.c
index a24520f2e78..34a5c7506ab 100644
--- a/drivers/mmc/sandbox_mmc.c
+++ b/drivers/mmc/sandbox_mmc.c
@@ -29,6 +29,7 @@ struct sandbox_mmc_plat {
 struct sandbox_mmc_priv {
 	char *buf;
 	int csize;	/* CSIZE value to report */
+	int bl_len_shift;	/* READ_BL_LEN value to report */
 	int size;
 };
 
@@ -62,7 +63,7 @@ static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
 		break;
 	case MMC_CMD_SEND_CSD:
 		cmd->response[0] = 0;
-		cmd->response[1] = (MMC_BL_LEN_SHIFT << 16) |
+		cmd->response[1] = (priv->bl_len_shift << 16) |
 				   ((priv->csize >> 16) & 0x3f);
 		cmd->response[2] = (priv->csize & 0xffff) << 16;
 		cmd->response[3] = 0;
@@ -144,6 +145,13 @@ static const struct dm_mmc_ops sandbox_mmc_ops = {
 	.get_cd = sandbox_mmc_get_cd,
 };
 
+void sandbox_mmc_set_bl_len_shift(struct udevice *dev, int bl_len_shift)
+{
+	struct sandbox_mmc_priv *priv = dev_get_priv(dev);
+
+	priv->bl_len_shift = bl_len_shift;
+}
+
 static int sandbox_mmc_of_to_plat(struct udevice *dev)
 {
 	struct sandbox_mmc_plat *plat = dev_get_plat(dev);
@@ -169,6 +177,7 @@ static int sandbox_mmc_probe(struct udevice *dev)
 	struct sandbox_mmc_priv *priv = dev_get_priv(dev);
 	int ret;
 
+	priv->bl_len_shift = MMC_BL_LEN_SHIFT;
 	if (plat->fname) {
 		ret = os_map_file(plat->fname, OS_O_RDWR | OS_O_CREAT,
 				  (void **)&priv->buf, &priv->size);
diff --git a/test/dm/mmc.c b/test/dm/mmc.c
index cdebb955196..d393883855c 100644
--- a/test/dm/mmc.c
+++ b/test/dm/mmc.c
@@ -6,6 +6,7 @@
 #include <dm.h>
 #include <mmc.h>
 #include <part.h>
+#include <asm/test.h>
 #include <dm/test.h>
 #include <test/test.h>
 #include <test/ut.h>
@@ -51,3 +52,42 @@ static int dm_test_mmc_blk(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_mmc_blk, UTF_SCAN_PDATA | UTF_SCAN_FDT);
+
+/* Test that a card with a READ_BL_LEN other than 9 gets 512-byte blocks */
+static int dm_test_mmc_bl_len(struct unit_test_state *uts)
+{
+	static const int shift[] = { 0, 8, 11 };
+	struct blk_desc *dev_desc;
+	char write[512], read[512];
+	struct udevice *dev;
+	struct mmc *mmc;
+	int i, j;
+
+	ut_assertok(uclass_get_device_by_seq(UCLASS_MMC, 0, &dev));
+	ut_assertok(blk_get_device_by_str("mmc", "0", &dev_desc));
+	mmc = mmc_get_mmc_dev(dev);
+
+	for (i = 0; i < ARRAY_SIZE(shift); i++) {
+		/* Initialise the card again, as 'mmc rescan' does */
+		sandbox_mmc_set_bl_len_shift(dev, shift[i]);
+		mmc->has_init = 0;
+		ut_assertok(mmc_init(mmc));
+
+		ut_asserteq(512, mmc->read_bl_len);
+		ut_asserteq(512, mmc->write_bl_len);
+		ut_asserteq(512, dev_desc->blksz);
+		ut_asserteq(9, dev_desc->log2blksz);
+
+		/* mmc0 reports 1024 blocks of the length in READ_BL_LEN */
+		ut_asserteq((1024ULL << shift[i]) / 512, dev_desc->lba);
+
+		for (j = 0; j < sizeof(write); j++)
+			write[j] = i + j;
+		ut_asserteq(1, blk_dwrite(dev_desc, 1, 1, write));
+		ut_asserteq(1, blk_dread(dev_desc, 1, 1, read));
+		ut_asserteq_mem(write, read, sizeof(write));
+	}
+
+	return 0;
+}
+DM_TEST(dm_test_mmc_bl_len, UTF_SCAN_PDATA | UTF_SCAN_FDT);

Re: [PATCH v3 11/14] disk: part_dos: Make room for a three-digit partition number in the UUID

[Heinrich Schuchardt]

Am 1. November 2026 08:02:50 MEZ schrieb Ana Bobillo <anabobillo3@gmail.com>:
>From: Ana Bobillo <anabobillo3@gmail.com>
>
>part_get_info_extended() makes up the UUID of a DOS partition as
>"%08x-%02x" from the disk signature and the partition number, into a
>buffer of 12 bytes. That is enough up to partition 255, but
>MAX_EXT_PARTS lets the number reach 256, whose three digits make
>sprintf() write the terminating NUL past the end of the buffer.
>
>Size the buffer like the UUID it is copied into, and use snprintf().
>
>Add a test with 256 partitions, which catches the overflow on sandbox
>with CONFIG_ASAN.
>
>Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
>Reviewed-by: Simon Glass <sjg@chromium.org>
>---
>(no changes since v1)
>
> disk/part_dos.c |  6 ++++--
> test/dm/part.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+), 2 deletions(-)
>
>diff --git a/disk/part_dos.c b/disk/part_dos.c
>index bc40d9d2829..3029ee0c31a 100644
>--- a/disk/part_dos.c
>+++ b/disk/part_dos.c
>@@ -325,9 +325,11 @@ static int part_get_info_extended(struct blk_desc *desc,
> 						 "0x%x", pt->sys_ind);
> 					info->bootable = get_bootable(pt);
> 					if (CONFIG_IS_ENABLED(PARTITION_UUIDS)) {
>-						char str[12];
>+						/* part_num may take three digits */
>+						char str[UUID_STR_LEN + 1];
> 
>-						sprintf(str, "%08x-%02x", disksig, part_num);
>+						snprintf(str, sizeof(str), "%08x-%02x",
>+							 disksig, part_num);
> 						disk_partition_set_uuid(info, str);
> 					}
> 					info->sys_ind = pt->sys_ind;
>diff --git a/test/dm/part.c b/test/dm/part.c
>index 4938d962144..f8a0fa974c1 100644
>--- a/test/dm/part.c
>+++ b/test/dm/part.c
>@@ -423,3 +423,49 @@ static int dm_test_part_dos_ext_empty(struct unit_test_state *uts)
> 	return 0;
> }
> DM_TEST(dm_test_part_dos_ext_empty, UTF_SCAN_PDATA | UTF_SCAN_FDT | UTF_CONSOLE);
>+
>+/*
>+ * Partition numbers above 255 take three digits in the UUID made up for a
>+ * DOS partition, from the disk signature and the partition number
>+ */
>+static int dm_test_part_dos_uuid(struct unit_test_state *uts)
>+{
>+	struct disk_partition info;
>+	dos_partition_t pt[4];
>+	struct blk_desc *desc;
>+	int part;
>+
>+	if (!CONFIG_IS_ENABLED(PARTITION_UUIDS))
>+		return -EAGAIN;
>+
>+	ut_asserteq(2, blk_get_device_by_str("mmc", "2", &desc));
>+	ut_asserteq(512, desc->blksz);
>+
>+	/* an extended partition from sector 2 */
>+	memset(pt, '\0', sizeof(pt));
>+	dos_set_entry(&pt[0], DOS_PART_TYPE_EXTENDED, 2, 512);
>+	ut_assertok(dos_write_table(desc, 0, pt));
>+
>+	/*
>+	 * holding logical partitions 5 to 256, each of one sector after its
>+	 * table, with the tables two sectors apart from sector 2 on
>+	 */
>+	for (part = 5; part <= 256; part++) {
>+		memset(pt, '\0', sizeof(pt));
>+		dos_set_entry(&pt[0], 0x83, 1, 1);
>+		if (part < 256)
>+			dos_set_entry(&pt[1], DOS_PART_TYPE_EXTENDED,
>+				      (part - 4) * 2, 2);
>+		ut_assertok(dos_write_table(desc, 2 + (part - 5) * 2, pt));
>+	}
>+
>+	ut_assertok(part_get_info_by_type(desc, 255, PART_TYPE_DOS, &info));
>+	ut_asserteq(2 + 250 * 2 + 1, info.start);
>+	ut_asserteq_str("00000000-ff", disk_partition_uuid(&info));
>+	ut_assertok(part_get_info_by_type(desc, 256, PART_TYPE_DOS, &info));
>+	ut_asserteq(2 + 251 * 2 + 1, info.start);
>+	ut_asserteq_str("00000000-100", disk_partition_uuid(&info));
>+

Linux include/linux/blkdev.h has:

"MSDOS UUID is 11 characters."
So this change looks wrong.

Best regards

Heinrich


>+	return 0;
>+}
>+DM_TEST(dm_test_part_dos_uuid, UTF_SCAN_PDATA | UTF_SCAN_FDT);

Re: [PATCH v3 11/14] disk: part_dos: Make room for a three-digit partition number in the UUID

[Ana Bobillo]

Hi Heinrich,

On Sun, 1 Nov 2026 at 09:03, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> +	ut_asserteq_str("00000000-100", disk_partition_uuid(&info));
>
> Linux include/linux/blkdev.h has:
>
> "MSDOS UUID is 11 characters."
> So this change looks wrong.

You're right. Linux never numbers a partition above 255: the partition
parsers stop at DISK_MAX_PARTS - 1 (parse_extended() stops as soon as
state->next reaches state->limit) and add_partition() refuses a higher
number. So the "%08x-%02x" UUID it makes up for a DOS partition always
has 11 characters, and a "-100" one could never match a Linux PARTUUID
anyway.

The bug is rather that part_dos.c numbers partitions past that:
MAX_EXT_PARTS is 256, and it is only checked once per table, so the
entries of the last table read can take part_num up to 259 and overflow
the 12-byte buffer.

So in v4 I'll leave the buffer and the sprintf() as they are, and
instead lower MAX_EXT_PARTS to 255 and check it before each entry, in
both walks (part_get_info_extended() and print_partition_extended()),
as parse_extended() does. The test then checks that partition 255 gets
"00000000-ff" and that there is no partition 256, both when looking a
partition up and when listing them. The patch becomes "disk: part_dos:
Number at most 255 partitions, as Linux does", and I'll drop Simon's
Reviewed-by from it since it has changed.

I'll wait a few days for comments on the rest of the series before
sending v4.

Thanks,
Ana

Re: [PATCH v3 12/14] efi_loader: Measure the GPT the partition driver uses

[Simon Glass]

On 2026-11-01T07:01:22+11:00, Ana Bobillo <anabobillo3@gmail.com> wrote:
> efi_loader: Measure the GPT the partition driver uses
>
> tcg2_measure_gpt_data() reads the primary GPT and measures it into
> PCR 5 without checking it, while the partition driver uses the backup
> GPT when the primary header or entries are not valid. So PCR 5 may hold
> a GPT the partitions were not found in: a change to an unused primary
> entry, which is not measured, leaves PCR 5 as it was while the
> partitions come from the backup GPT. Nor are the sizes in the header
> checked, so an entry array size that wraps in 32 bits makes the walks
> read past the end of the entry buffer.
>
> Measure the GPT the partitions are found in, read by the partition
> driver's own code through a new part_get_gpt(), which checks the sizes
> too. When the primary GPT is not valid, measure the backup header as
> the primary header that 'gpt repair' writes from it, so that PCR 5 is
> as for the repaired disk; a primary header marked to be ignored
> (ChromeOS) is never repaired, so measure the backup header as it is
> then. If no valid GPT is read, measure no GPT event but return success,
> so that the separators are still measured.
>
> EDK2 measures the GPT its partition driver uses too, since 7c12d4359abd
> ("SecurityPkg/DxeTpm2MeasureBootLib: Use GptLib parser"), with the
> primary header that driver restores from the backup.
>
> Add a sandbox test that checks PCR 5 for a valid GPT, an entry array
> size that wraps, a backup GPT that differs from a broken primary one
> and an ignored primary header, and check part_get_gpt() in
> lib_test_gpt_sizes.
>
> Fixes: ce3dbc5d080d ("efi_loader: add UEFI GPT measurement")
> Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
>
>  disk/part_efi.c                |  27 +++++
>  include/part.h                 |  27 +++++
>  lib/efi_loader/efi_tcg2.c      |  52 +++++----
>  test/lib/part_efi.c            |  61 +++++++++-
>  test/py/tests/test_efi_tcg2.py | 246 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 385 insertions(+), 28 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v3 13/14] efi_loader: Index the GPT entries directly in tcg2_measure_gpt_data()

[Simon Glass]

On 2026-11-01T07:01:22+11:00, Ana Bobillo <anabobillo3@gmail.com> wrote:
> efi_loader: Index the GPT entries directly in tcg2_measure_gpt_data()
>
> tcg2_measure_gpt_data() now measures the GPT that part_get_gpt() gives,
> whose entries are always sizeof(gpt_entry) bytes, as
> validate_gpt_header() refuses any other size. So index them as an array
> rather than stepping by sizeof_partition_entry, and read
> num_partition_entries with le32_to_cpu(), as the rest of the GPT code
> does.
>
> No functional change on a little-endian machine.
>
> Suggested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
>
>  lib/efi_loader/efi_tcg2.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v3 14/14] mmc: Always use a 512-byte block length

[Simon Glass]

Hi Ana,

On 2026-11-01T07:01:22+11:00, Ana Bobillo <anabobillo3@gmail.com> wrote:
> mmc: Always use a 512-byte block length
>
> mmc_startup() sets the block size of the card's block device from the
> READ_BL_LEN field of its CSD and only caps it at 512 bytes from above. A
> card reporting a READ_BL_LEN below 9, which the SD specification
> reserves, gives the block device a block size below 512 bytes. A block
> that small cannot hold an MBR or a GPT header, and the partition and
> filesystem code assumes at least 512 bytes, as the patch adding
> blk_valid_blksz() describes. Nor does the code bound an MMC card's

Two nits, neither worth a resend unless something else comes up.

Once applied, 'the patch adding blk_valid_blksz()' won't point
anywhere useful. Please can you give the subject of that commit, or
drop the reference, since the sentence makes the point without it.

> code already did in DDR mode and as Linux does for its transfers; the

Please use 'does' here: before this commit the code still behaves this
way in DDR mode.

> WRITE_BL_LEN from below, though mmc_bwrite() then writes blocks of that
> length where the block layer passes 512-byte ones.
>
> The check in blk_post_probe() does not catch this. It is left out of
> SPL, TPL and VPL, which commonly boot from MMC. In U-Boot proper,
> mmc_startup() runs again whenever the card is initialised again, by
> 'mmc rescan' or 'mmc dev', or after the card-detect poll has seen it
> removed, and sets the block size of a block device that is already
> probed.
>
> Use 512-byte blocks whatever READ_BL_LEN and WRITE_BL_LEN say, as the
> [...]
>
>  arch/sandbox/include/asm/test.h | 10 ++++++++++
>  drivers/mmc/mmc.c               | 34 +++++++++++-----------------------
>  drivers/mmc/sandbox_mmc.c       | 11 ++++++++++-
>  test/dm/mmc.c                   | 40 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 71 insertions(+), 24 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon