summaryrefslogtreecommitdiff
path: root/config/submodule/coreboot/fam15h_rdimm/vboot/patches
diff options
context:
space:
mode:
Diffstat (limited to 'config/submodule/coreboot/fam15h_rdimm/vboot/patches')
-rw-r--r--config/submodule/coreboot/fam15h_rdimm/vboot/patches/0001-extract_vmlinuz.c-Fix-the-bounds-check-on-vmlinuz_he.patch178
1 files changed, 0 insertions, 178 deletions
diff --git a/config/submodule/coreboot/fam15h_rdimm/vboot/patches/0001-extract_vmlinuz.c-Fix-the-bounds-check-on-vmlinuz_he.patch b/config/submodule/coreboot/fam15h_rdimm/vboot/patches/0001-extract_vmlinuz.c-Fix-the-bounds-check-on-vmlinuz_he.patch
deleted file mode 100644
index 1ac41de6..00000000
--- a/config/submodule/coreboot/fam15h_rdimm/vboot/patches/0001-extract_vmlinuz.c-Fix-the-bounds-check-on-vmlinuz_he.patch
+++ /dev/null
@@ -1,178 +0,0 @@
-From 195f61375aeec9eec16604ec59f6eda2e6058cc1 Mon Sep 17 00:00:00 2001
-From: "Luke T. Shumaker" <lukeshu@lukeshu.com>
-Date: Thu, 30 May 2024 14:08:33 -0600
-Subject: [PATCH 1/1] extract_vmlinuz.c: Fix the bounds check on
- vmlinuz_header_{offset,size}
-
-The check on vmlinuz_header_offset and vmlinuz_header_size is obviously
-wrong:
-
- if (!vmlinuz_header_size ||
- kpart_data + vmlinuz_header_offset + vmlinuz_header_size >
- kpart_data) {
- return 1;
- }
-
-`kpart_data + some_unsigned_values` can obviously never be `> kpart_data`,
-unless something has overflowed! And `vmlinuz_header_offset` hasn't even
-been set yet (besides being initialized to zero)!
-
-GCC will deduce that if the check didn't cause the function to bail, then
-vmlinuz_header_size (a uint32_t) must be "negative"; that is: in the range
-[2GiB,4GiB).
-
-On platforms where size_t is 32-bits, this is *especially* broken.
-memcpy's size argument must be in the range [0,2GiB). Because GCC has
-proved that vmlinuz_header_size is higher than that, it will fail to
-compile:
-
- host/lib/extract_vmlinuz.c:67:9: error: 'memcpy' specified bound between 2147483648 and 4294967295 exceeds maximum object size 2147483647 [-Werror=stringop-overflow=]
-
-So, fix the check.
-
-I can now say that what I suspect the original author meant to write would
-be the following patch, if `vmlinuz_header_offset` were already set:
-
- -kpart_data + vmlinuz_header_offset + vmlinuz_header_size > kpart_data
- +now + vmlinuz_header_offset + vmlinuz_header_size > kpart_size
-
-This hypothesis is supported by `now` not getting incremented by
-`kblob_size` the way it is for the keyblock and preamble sizes.
-
-However, we can also see that even this "corrected" bounds check is
-insufficient: it does not detect the vmlinuz_header overflowing into
-kblob_data.
-
-OK, so let's describe the fix:
-
-Have a `*vmlinuz_header` pointer instead of a
-`uint64_t vmlinuz_header_offset`, to be more similar to all the other
-regions. With this change, the correct check becomes a simple
-
- vmlinuz_header + vmlinuz_header_size > kblob_data
-
-While we're at it, make some changes that could have helped avoid this in
-the first place:
-
- - Add comments.
- - Calculate the vmlinuz_header offset right away, instead of waiting.
- - Go ahead and increment `now` by `kblob_size`, to increase regularity.
-
-Change-Id: I5c03e49070b6dd2e04459566ef7dd129d27736e4
----
- host/lib/extract_vmlinuz.c | 72 +++++++++++++++++++++++++++-----------
- 1 file changed, 51 insertions(+), 21 deletions(-)
-
-diff --git a/host/lib/extract_vmlinuz.c b/host/lib/extract_vmlinuz.c
-index 4ccfcf33..d2c09443 100644
---- a/host/lib/extract_vmlinuz.c
-+++ b/host/lib/extract_vmlinuz.c
-@@ -15,16 +15,44 @@
-
- int ExtractVmlinuz(void *kpart_data, size_t kpart_size,
- void **vmlinuz_out, size_t *vmlinuz_size) {
-+ // We're going to be extracting `vmlinuz_header` and
-+ // `kblob_data`, and returning the concatenation of them.
-+ //
-+ // kpart_data = +-[kpart_size]------------------------------------+
-+ // | |
-+ // keyblock = | +-[keyblock->keyblock_size]-------------------+ |
-+ // | | struct vb2_keyblock keyblock | |
-+ // | | char [] ...data... | |
-+ // | +---------------------------------------------+ |
-+ // | |
-+ // preamble = | +-[preamble->preamble_size]-------------------+ |
-+ // | | struct vb2_kernel_preamble preamble | |
-+ // | | char [] ...data... | |
-+ // | | char [] vmlinuz_header | |
-+ // | | char [] ...data... | |
-+ // | +---------------------------------------------+ |
-+ // | |
-+ // kblob_data= | +-[preamble->body_signature.data_size]--------+ |
-+ // | | char [] ...data... | |
-+ // | +---------------------------------------------+ |
-+ // | |
-+ // +-------------------------------------------------+
-+
- size_t now = 0;
-+ // The 3 sections of kpart_data.
-+ struct vb2_keyblock *keyblock = NULL;
- struct vb2_kernel_preamble *preamble = NULL;
- uint8_t *kblob_data = NULL;
- uint32_t kblob_size = 0;
-+ // vmlinuz_header
-+ uint8_t *vmlinuz_header = NULL;
- uint32_t vmlinuz_header_size = 0;
-- uint64_t vmlinuz_header_address = 0;
-- uint64_t vmlinuz_header_offset = 0;
-+ // The concatenated result.
- void *vmlinuz = NULL;
-
-- struct vb2_keyblock *keyblock = (struct vb2_keyblock *)kpart_data;
-+ // Isolate the 3 sections of kpart_data.
-+
-+ keyblock = (struct vb2_keyblock *)kpart_data;
- now += keyblock->keyblock_size;
- if (now > kpart_size)
- return 1;
-@@ -36,37 +64,39 @@ int ExtractVmlinuz(void *kpart_data, size_t kpart_size,
-
- kblob_data = kpart_data + now;
- kblob_size = preamble->body_signature.data_size;
--
-- if (!kblob_data || (now + kblob_size) > kpart_size)
-+ now += kblob_size;
-+ if (now > kpart_size)
- return 1;
-
-+ // Find `vmlinuz_header` within `preamble`.
-+
- if (preamble->header_version_minor > 0) {
-- vmlinuz_header_address = preamble->vmlinuz_header_address;
-+ // calculate the vmlinuz_header offset from
-+ // the beginning of the kpart_data. The kblob doesn't
-+ // include the body_load_offset, but does include
-+ // the keyblock and preamble sections.
-+ size_t vmlinuz_header_offset =
-+ preamble->vmlinuz_header_address -
-+ preamble->body_load_address +
-+ keyblock->keyblock_size +
-+ preamble->preamble_size;
-+
-+ vmlinuz_header = kpart_data + vmlinuz_header_offset;
- vmlinuz_header_size = preamble->vmlinuz_header_size;
- }
-
-- if (!vmlinuz_header_size ||
-- kpart_data + vmlinuz_header_offset + vmlinuz_header_size >
-- kpart_data) {
-+ if (!vmlinuz_header ||
-+ !vmlinuz_header_size ||
-+ vmlinuz_header + vmlinuz_header_size > kblob_data) {
- return 1;
- }
-
-- // calculate the vmlinuz_header offset from
-- // the beginning of the kpart_data. The kblob doesn't
-- // include the body_load_offset, but does include
-- // the keyblock and preamble sections.
-- vmlinuz_header_offset = vmlinuz_header_address -
-- preamble->body_load_address +
-- keyblock->keyblock_size +
-- preamble->preamble_size;
-+ // Concatenate and return.
-
- vmlinuz = malloc(vmlinuz_header_size + kblob_size);
- if (vmlinuz == NULL)
- return 1;
--
-- memcpy(vmlinuz, kpart_data + vmlinuz_header_offset,
-- vmlinuz_header_size);
--
-+ memcpy(vmlinuz, vmlinuz_header, vmlinuz_header_size);
- memcpy(vmlinuz + vmlinuz_header_size, kblob_data, kblob_size);
-
- *vmlinuz_out = vmlinuz;
---
-2.45.1
-