Fix building vboot on i686
parent
7fbcb7be95
commit
221206b4da
|
@ -1,26 +0,0 @@
|
||||||
From 49569081d248e6eea3d4fb8a2cfb70154d9fd91f Mon Sep 17 00:00:00 2001
|
|
||||||
From: Leah Rowe <info@minifree.org>
|
|
||||||
Date: Tue, 21 May 2024 23:14:51 +0100
|
|
||||||
Subject: [PATCH 1/1] don't treat warnings as errors
|
|
||||||
|
|
||||||
Signed-off-by: Leah Rowe <info@minifree.org>
|
|
||||||
---
|
|
||||||
Makefile | 2 +-
|
|
||||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
|
||||||
|
|
||||||
diff --git a/Makefile b/Makefile
|
|
||||||
index 8f1bd148..67a282ee 100644
|
|
||||||
--- a/Makefile
|
|
||||||
+++ b/Makefile
|
|
||||||
@@ -124,7 +124,7 @@ endif
|
|
||||||
# Provide default CC and CFLAGS for firmware builds; if you have any -D flags,
|
|
||||||
# please add them after this point (e.g., -DVBOOT_DEBUG).
|
|
||||||
DEBUG_FLAGS := $(if ${DEBUG},-g -O0,-g -Os)
|
|
||||||
-WERROR := -Werror
|
|
||||||
+WERROR := -Wno-error -w
|
|
||||||
FIRMWARE_FLAGS := -nostdinc -ffreestanding -fno-builtin -fno-stack-protector
|
|
||||||
COMMON_FLAGS := -pipe ${WERROR} -Wall -Wstrict-prototypes -Wtype-limits \
|
|
||||||
-Wundef -Wmissing-prototypes -Wno-trigraphs -Wredundant-decls -Wshadow \
|
|
||||||
--
|
|
||||||
2.39.2
|
|
||||||
|
|
|
@ -0,0 +1,178 @@
|
||||||
|
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
|
||||||
|
|
|
@ -1,35 +0,0 @@
|
||||||
From d94300a671688746f2fb3d77eefa631a3ed90306 Mon Sep 17 00:00:00 2001
|
|
||||||
From: Leah Rowe <info@minifree.org>
|
|
||||||
Date: Sun, 19 May 2024 23:35:52 +0100
|
|
||||||
Subject: [PATCH 1/1] don't treat warnings as errors
|
|
||||||
|
|
||||||
Signed-off-by: Leah Rowe <info@minifree.org>
|
|
||||||
---
|
|
||||||
Makefile | 4 ++--
|
|
||||||
1 file changed, 2 insertions(+), 2 deletions(-)
|
|
||||||
|
|
||||||
diff --git a/Makefile b/Makefile
|
|
||||||
index 4cb265b2..4ba8b2da 100644
|
|
||||||
--- a/Makefile
|
|
||||||
+++ b/Makefile
|
|
||||||
@@ -114,7 +114,7 @@ endif
|
|
||||||
# Provide default CC and CFLAGS for firmware builds; if you have any -D flags,
|
|
||||||
# please add them after this point (e.g., -DVBOOT_DEBUG).
|
|
||||||
DEBUG_FLAGS := $(if $(filter-out 0,${DEBUG}),-g -Og,-g -Os)
|
|
||||||
-WERROR := -Werror
|
|
||||||
+WERROR := -Wno-error -w
|
|
||||||
FIRMWARE_FLAGS := -nostdinc -ffreestanding -fno-builtin -fno-stack-protector
|
|
||||||
COMMON_FLAGS := -pipe ${WERROR} -Wall -Wstrict-prototypes -Wtype-limits \
|
|
||||||
-Wundef -Wmissing-prototypes -Wno-trigraphs -Wredundant-decls -Wshadow \
|
|
||||||
@@ -128,7 +128,7 @@ COMMON_FLAGS := -pipe ${WERROR} -Wall -Wstrict-prototypes -Wtype-limits \
|
|
||||||
# returns: $(1) if compiler was successful, empty string otherwise
|
|
||||||
test_ccflag = $(shell \
|
|
||||||
printf "$(2)\nvoid _start(void) {}\n" | \
|
|
||||||
- $(CC) -nostdlib -Werror $(1) -xc -c - -o /dev/null \
|
|
||||||
+ $(CC) -nostdlib -Wno-error -w $(1) -xc -c - -o /dev/null \
|
|
||||||
>/dev/null 2>&1 && echo "$(1)")
|
|
||||||
|
|
||||||
COMMON_FLAGS += $(call test_ccflag,-Wimplicit-fallthrough)
|
|
||||||
--
|
|
||||||
2.39.2
|
|
||||||
|
|
|
@ -0,0 +1,178 @@
|
||||||
|
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
|
||||||
|
|
|
@ -1,35 +0,0 @@
|
||||||
From d94300a671688746f2fb3d77eefa631a3ed90306 Mon Sep 17 00:00:00 2001
|
|
||||||
From: Leah Rowe <info@minifree.org>
|
|
||||||
Date: Sun, 19 May 2024 23:35:52 +0100
|
|
||||||
Subject: [PATCH 1/1] don't treat warnings as errors
|
|
||||||
|
|
||||||
Signed-off-by: Leah Rowe <info@minifree.org>
|
|
||||||
---
|
|
||||||
Makefile | 4 ++--
|
|
||||||
1 file changed, 2 insertions(+), 2 deletions(-)
|
|
||||||
|
|
||||||
diff --git a/Makefile b/Makefile
|
|
||||||
index 4cb265b2..4ba8b2da 100644
|
|
||||||
--- a/Makefile
|
|
||||||
+++ b/Makefile
|
|
||||||
@@ -114,7 +114,7 @@ endif
|
|
||||||
# Provide default CC and CFLAGS for firmware builds; if you have any -D flags,
|
|
||||||
# please add them after this point (e.g., -DVBOOT_DEBUG).
|
|
||||||
DEBUG_FLAGS := $(if $(filter-out 0,${DEBUG}),-g -Og,-g -Os)
|
|
||||||
-WERROR := -Werror
|
|
||||||
+WERROR := -Wno-error -w
|
|
||||||
FIRMWARE_FLAGS := -nostdinc -ffreestanding -fno-builtin -fno-stack-protector
|
|
||||||
COMMON_FLAGS := -pipe ${WERROR} -Wall -Wstrict-prototypes -Wtype-limits \
|
|
||||||
-Wundef -Wmissing-prototypes -Wno-trigraphs -Wredundant-decls -Wshadow \
|
|
||||||
@@ -128,7 +128,7 @@ COMMON_FLAGS := -pipe ${WERROR} -Wall -Wstrict-prototypes -Wtype-limits \
|
|
||||||
# returns: $(1) if compiler was successful, empty string otherwise
|
|
||||||
test_ccflag = $(shell \
|
|
||||||
printf "$(2)\nvoid _start(void) {}\n" | \
|
|
||||||
- $(CC) -nostdlib -Werror $(1) -xc -c - -o /dev/null \
|
|
||||||
+ $(CC) -nostdlib -Wno-error -w $(1) -xc -c - -o /dev/null \
|
|
||||||
>/dev/null 2>&1 && echo "$(1)")
|
|
||||||
|
|
||||||
COMMON_FLAGS += $(call test_ccflag,-Wimplicit-fallthrough)
|
|
||||||
--
|
|
||||||
2.39.2
|
|
||||||
|
|
|
@ -0,0 +1,178 @@
|
||||||
|
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
|
||||||
|
|
|
@ -1,26 +0,0 @@
|
||||||
From 2c5c8d9a5c999a5eedd9f17acb0bd3924524657d Mon Sep 17 00:00:00 2001
|
|
||||||
From: Leah Rowe <info@minifree.org>
|
|
||||||
Date: Tue, 21 May 2024 23:21:14 +0100
|
|
||||||
Subject: [PATCH 1/1] don't treat warnings as errors
|
|
||||||
|
|
||||||
Signed-off-by: Leah Rowe <info@minifree.org>
|
|
||||||
---
|
|
||||||
Makefile | 2 +-
|
|
||||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
|
||||||
|
|
||||||
diff --git a/Makefile b/Makefile
|
|
||||||
index 0539e8d7..5d8c240c 100644
|
|
||||||
--- a/Makefile
|
|
||||||
+++ b/Makefile
|
|
||||||
@@ -136,7 +136,7 @@ endif
|
|
||||||
#
|
|
||||||
# Flag ordering: arch, then -f, then -m, then -W
|
|
||||||
DEBUG_FLAGS := $(if ${DEBUG},-g -O0,-g -Os)
|
|
||||||
-WERROR := -Werror
|
|
||||||
+WERROR := -Wno-error -w
|
|
||||||
FIRMWARE_FLAGS := -nostdinc -ffreestanding -fno-builtin -fno-stack-protector
|
|
||||||
COMMON_FLAGS := -pipe ${WERROR} -Wall -Wstrict-prototypes -Wtype-limits \
|
|
||||||
-Wundef -Wmissing-prototypes -Wno-trigraphs -Wredundant-decls -Wshadow \
|
|
||||||
--
|
|
||||||
2.39.2
|
|
||||||
|
|
|
@ -0,0 +1,178 @@
|
||||||
|
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
|
||||||
|
|
|
@ -1,26 +0,0 @@
|
||||||
From 2c5c8d9a5c999a5eedd9f17acb0bd3924524657d Mon Sep 17 00:00:00 2001
|
|
||||||
From: Leah Rowe <info@minifree.org>
|
|
||||||
Date: Tue, 21 May 2024 23:21:14 +0100
|
|
||||||
Subject: [PATCH 1/1] don't treat warnings as errors
|
|
||||||
|
|
||||||
Signed-off-by: Leah Rowe <info@minifree.org>
|
|
||||||
---
|
|
||||||
Makefile | 2 +-
|
|
||||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
|
||||||
|
|
||||||
diff --git a/Makefile b/Makefile
|
|
||||||
index 0539e8d7..5d8c240c 100644
|
|
||||||
--- a/Makefile
|
|
||||||
+++ b/Makefile
|
|
||||||
@@ -136,7 +136,7 @@ endif
|
|
||||||
#
|
|
||||||
# Flag ordering: arch, then -f, then -m, then -W
|
|
||||||
DEBUG_FLAGS := $(if ${DEBUG},-g -O0,-g -Os)
|
|
||||||
-WERROR := -Werror
|
|
||||||
+WERROR := -Wno-error -w
|
|
||||||
FIRMWARE_FLAGS := -nostdinc -ffreestanding -fno-builtin -fno-stack-protector
|
|
||||||
COMMON_FLAGS := -pipe ${WERROR} -Wall -Wstrict-prototypes -Wtype-limits \
|
|
||||||
-Wundef -Wmissing-prototypes -Wno-trigraphs -Wredundant-decls -Wshadow \
|
|
||||||
--
|
|
||||||
2.39.2
|
|
||||||
|
|
|
@ -0,0 +1,178 @@
|
||||||
|
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
|
||||||
|
|
|
@ -1,35 +0,0 @@
|
||||||
From dd263a01c6f1b63fc12a2a3e96e87a8cee8d987c Mon Sep 17 00:00:00 2001
|
|
||||||
From: Leah Rowe <info@minifree.org>
|
|
||||||
Date: Tue, 21 May 2024 23:21:49 +0100
|
|
||||||
Subject: [PATCH 1/1] don't treat warnings as errors
|
|
||||||
|
|
||||||
Signed-off-by: Leah Rowe <info@minifree.org>
|
|
||||||
---
|
|
||||||
Makefile | 4 ++--
|
|
||||||
1 file changed, 2 insertions(+), 2 deletions(-)
|
|
||||||
|
|
||||||
diff --git a/Makefile b/Makefile
|
|
||||||
index e3739dc0..a11dccbd 100644
|
|
||||||
--- a/Makefile
|
|
||||||
+++ b/Makefile
|
|
||||||
@@ -112,7 +112,7 @@ endif
|
|
||||||
# Provide default CC and CFLAGS for firmware builds; if you have any -D flags,
|
|
||||||
# please add them after this point (e.g., -DVBOOT_DEBUG).
|
|
||||||
DEBUG_FLAGS := $(if $(filter-out 0,${DEBUG}),-g -Og,-g -Os)
|
|
||||||
-WERROR := -Werror
|
|
||||||
+WERROR := -Wno-error -w
|
|
||||||
FIRMWARE_FLAGS := -nostdinc -ffreestanding -fno-builtin -fno-stack-protector
|
|
||||||
COMMON_FLAGS := -pipe ${WERROR} -Wall -Wstrict-prototypes -Wtype-limits \
|
|
||||||
-Wundef -Wmissing-prototypes -Wno-trigraphs -Wredundant-decls -Wshadow \
|
|
||||||
@@ -126,7 +126,7 @@ COMMON_FLAGS := -pipe ${WERROR} -Wall -Wstrict-prototypes -Wtype-limits \
|
|
||||||
# returns: $(1) if compiler was successful, empty string otherwise
|
|
||||||
test_ccflag = $(shell \
|
|
||||||
printf "$(2)\nvoid _start(void) {}\n" | \
|
|
||||||
- $(CC) -nostdlib -Werror $(1) -xc -c - -o /dev/null \
|
|
||||||
+ $(CC) -nostdlib -Wno-error -w $(1) -xc -c - -o /dev/null \
|
|
||||||
>/dev/null 2>&1 && echo "$(1)")
|
|
||||||
|
|
||||||
COMMON_FLAGS += $(call test_ccflag,-Wimplicit-fallthrough)
|
|
||||||
--
|
|
||||||
2.39.2
|
|
||||||
|
|
|
@ -0,0 +1,178 @@
|
||||||
|
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
|
||||||
|
|
|
@ -1,35 +0,0 @@
|
||||||
From 59c393f2a928770c2d397bc93d388543c2d94dab Mon Sep 17 00:00:00 2001
|
|
||||||
From: Leah Rowe <info@minifree.org>
|
|
||||||
Date: Tue, 21 May 2024 23:22:50 +0100
|
|
||||||
Subject: [PATCH 1/1] don't treat warnings as errors
|
|
||||||
|
|
||||||
Signed-off-by: Leah Rowe <info@minifree.org>
|
|
||||||
---
|
|
||||||
Makefile | 4 ++--
|
|
||||||
1 file changed, 2 insertions(+), 2 deletions(-)
|
|
||||||
|
|
||||||
diff --git a/Makefile b/Makefile
|
|
||||||
index a4390522..d2ec9aa3 100644
|
|
||||||
--- a/Makefile
|
|
||||||
+++ b/Makefile
|
|
||||||
@@ -125,7 +125,7 @@ endif
|
|
||||||
# Provide default CC and CFLAGS for firmware builds; if you have any -D flags,
|
|
||||||
# please add them after this point (e.g., -DVBOOT_DEBUG).
|
|
||||||
DEBUG_FLAGS := $(if $(filter-out 0,${DEBUG}),-g -Og,-g -Os)
|
|
||||||
-WERROR := -Werror
|
|
||||||
+WERROR := -Wno-error -w
|
|
||||||
FIRMWARE_FLAGS := -nostdinc -ffreestanding -fno-builtin -fno-stack-protector
|
|
||||||
COMMON_FLAGS := -pipe ${WERROR} -Wall -Wstrict-prototypes -Wtype-limits \
|
|
||||||
-Wundef -Wmissing-prototypes -Wno-trigraphs -Wredundant-decls -Wshadow \
|
|
||||||
@@ -162,7 +162,7 @@ CFLAGS += -std=gnu11
|
|
||||||
# returns: $(1) if compiler was successful, empty string otherwise
|
|
||||||
test_ccflag = $(shell \
|
|
||||||
printf "$(2)\nvoid _start(void) {}\n" | \
|
|
||||||
- $(CC) -nostdlib -Werror $(1) -xc -c - -o /dev/null \
|
|
||||||
+ $(CC) -nostdlib -Wno-error -w $(1) -xc -c - -o /dev/null \
|
|
||||||
>/dev/null 2>&1 && echo "$(1)")
|
|
||||||
|
|
||||||
COMMON_FLAGS += $(call test_ccflag,-Wimplicit-fallthrough)
|
|
||||||
--
|
|
||||||
2.39.2
|
|
||||||
|
|
|
@ -0,0 +1,178 @@
|
||||||
|
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
|
||||||
|
|
Loading…
Reference in New Issue