From: Ana Bobillo <[email protected]>
cros_read_info() reads the kernel command line, the x86 setup block and
the bootloader into a buffer of bootloader_size + 8KB bytes, from an
offset worked out from bootloader_address - body_load_address. All three
fields come from the kernel preamble on disk and nothing checks them, so:
- on 32-bit machines, bootloader_size + 8KB can wrap, leaving the buffer
too small for the command line and setup block, which cros_read_buf()
reads from it
- cros_read_kernel() later points the x86 setup block at an offset into
the kernel body worked out from the same fields, which can be outside
the body
Reject the partition unless the command line and setup block lie in the
kernel body before the bootloader, and the bootloader ends within the
body, as the ChromiumOS kernel layout has it.
cros_read_info() can now fail before it sets priv->info_buf, which
cros_read_bootflow() frees on failure. Since priv comes from malloc(),
that frees whatever the heap left there, as a read error already could.
Allocate priv with calloc() so that the pointer is NULL until it is set.
Extend the ChromiumOS test image with two kernel partitions whose
bootloader is too near the start of the body or ends past the end of the
body. Neither may produce a bootflow.
Fixes: f861b1ee70aa ("bootstd: cros: Add a function to read info from
partition")
Fixes: 71f634b822ae ("bootstd: cros: Allow detection of any kernel partition")
Signed-off-by: Ana Bobillo <[email protected]>
---
(no changes since v1)
boot/bootmeth_cros.c | 24 +++++++++++++++++++++++-
test/boot/bootflow.c | 7 ++++---
test/py/tests/test_ut.py | 22 +++++++++++++++++-----
3 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c
index 685d5335e86..805680f71ad 100644
--- a/boot/bootmeth_cros.c
+++ b/boot/bootmeth_cros.c
@@ -272,6 +272,7 @@ static int cros_read_info(struct bootflow *bflow, const
char *uuid,
struct udevice *blk = bflow->blk;
struct blk_desc *desc = dev_get_uclass_plat(blk);
ulong offset, size, before_base;
+ u64 bl_offset;
void *buf;
int ret;
@@ -285,6 +286,27 @@ static int cros_read_info(struct bootflow *bflow, const
char *uuid,
(ulong)preamble->bootloader_address,
(ulong)preamble->bootloader_size);
+ /*
+ * The command line and setup block come just before the bootloader,
+ * all within the kernel body. These fields come from the disk and
+ * cannot be trusted, so check that the regions are where they should be
+ */
+ if (preamble->bootloader_address < preamble->body_load_address) {
+ log_debug("- invalid bootloader address\n");
+ return log_msg_ret("bla", -ERANGE);
+ }
+ bl_offset = preamble->bootloader_address -
+ preamble->body_load_address;
+ if (bl_offset < -CMDLINE_OFFSET ||
+ bl_offset > preamble->body_signature.data_size ||
+ preamble->bootloader_size >
+ preamble->body_signature.data_size - bl_offset) {
+ log_debug("- invalid bootloader offset %llx size %x\n",
+ (unsigned long long)bl_offset,
+ preamble->bootloader_size);
+ return log_msg_ret("bls", -ERANGE);
+ }
+
priv->body_size = preamble->body_signature.data_size;
priv->body_load_address = preamble->body_load_address;
priv->bootloader_address = preamble->bootloader_address;
@@ -406,7 +428,7 @@ static int cros_read_bootflow(struct udevice *dev, struct
bootflow *bflow)
return log_msg_ret("bod", -ERANGE);
}
- priv = malloc(sizeof(struct cros_priv));
+ priv = calloc(1, sizeof(struct cros_priv));
if (!priv) {
free(hdr);
return log_msg_ret("buf", -ENOMEM);
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index f24d79a34d9..e5b3e1fc665 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -1488,9 +1488,10 @@ static int bootflow_cros(struct unit_test_state *uts)
{
/*
* mmc5 has valid kernels in partitions 2 and 4 and corrupt kernel
- * metadata in partitions 6, 13, 14 and 15 (oversized keyblock,
- * undersized body offset, wrapped body offset, undersized keyblock),
- * none of which may produce a bootflow
+ * metadata in partitions 6, 13, 14, 15, 16 and 17 (oversized keyblock,
+ * undersized body offset, wrapped body offset, undersized keyblock,
+ * bootloader too near the start of the body, bootloader past the end
+ * of the body), none of which may produce a bootflow
*/
ut_assertok(scan_mmc_bootdev(uts, "mmc5", true));
ut_assertok(run_command("bootflow list", 0));
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index a33f188d984..5e3b02bb100 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -408,6 +408,8 @@ def setup_cros_image(ubman):
{'num': 0xd, 'label':'KERN_D', 'type': uuid_kern, 'size': '1'},
{'num': 0xe, 'label':'KERN_E', 'type': uuid_kern, 'size': '1'},
{'num': 0xf, 'label':'KERN_F', 'type': uuid_kern, 'size': '1'},
+ {'num': 0x10, 'label':'KERN_G', 'type': uuid_kern, 'size': '1'},
+ {'num': 0x11, 'label':'KERN_H', 'type': uuid_kern, 'size': '1'},
]
for part in required_parts:
@@ -462,18 +464,21 @@ def setup_cros_image(ubman):
set_part_data(2, pack_kernel(ubman, 'x86', kern, dummy))
set_part_data(4, pack_kernel(ubman, 'arm', kern, dummy))
- def bad_kernel_part(keyblock_size, preamble_size):
+ def bad_kernel_part(keyblock_size, preamble_size,
+ bootloader_address=0x103000, bootloader_size=0x1000):
"""Build corrupt-kernel-partition data that U-Boot must reject
The keyblock carries a valid magic and the given keyblock_size; a
- preamble with the given preamble_size follows it. The bootloader
- address/size fields are chosen so that, were the partition not
- rejected, scanning it would succeed and produce a spurious ready
+ preamble with the given preamble_size follows it. The default
+ bootloader address/size fields are chosen so that, were the partition
+ not rejected, scanning it would succeed and produce a spurious ready
bootflow, which the bootflow-list assertions would then catch.
Args:
keyblock_size (int): Value for the keyblock_size field
preamble_size (int): Value for the preamble_size field
+ bootloader_address (int): Value for the bootloader_address field
+ bootloader_size (int): Value for the bootloader_size field
Return:
bytes: Data to place at the start of the partition
@@ -484,7 +489,8 @@ def setup_cros_image(ubman):
# bootloader_size at offsets 48 / 56 / 64, body-signature data_size
# at offset 88
pre = struct.pack('<I', preamble_size).ljust(48, b'\0')
- pre += struct.pack('<QQI', 0x100000, 0x103000, 0x1000)
+ pre += struct.pack('<QQI', 0x100000, bootloader_address,
+ bootloader_size)
pre = pre.ljust(88, b'\0') + struct.pack('<I', 0x4000)
return kblock + pre
@@ -496,10 +502,16 @@ def setup_cros_image(ubman):
# - partition 14 (KERN_E): preamble_size wrapping the 32-bit body offset
# - partition 15 (KERN_F): keyblock_size smaller than the keyblock header,
# placing the claimed preamble inside the header
+ # - partition 16 (KERN_G): bootloader_address placing the command line
+ # before the start of the kernel body
+ # - partition 17 (KERN_H): bootloader_size extending the bootloader past
+ # the end of the kernel body
set_part_data(6, bad_kernel_part(0xffffffff, 0x800))
set_part_data(0xd, bad_kernel_part(0x100, 0x800))
set_part_data(0xe, bad_kernel_part(0x100, 0xffffff10))
set_part_data(0xf, bad_kernel_part(0x18, 0x1000))
+ set_part_data(0x10, bad_kernel_part(0x100, 0x1000, 0x101000))
+ set_part_data(0x11, bad_kernel_part(0x100, 0x1000, 0x103000, 0x10000))
with open(fname, 'wb') as outf:
outf.write(disk_data)
