tools/bootconfig: Fix integer overflow in size check
Subsystems (2)
Masami Hiramatsu (Google)
maintainerPATCH v2 1/1 tools/bootconfig: Fix integer overflow in size check
From: Masami Hiramatsu (Google) <[email protected]>
Sashiko reported that on 32-bit systems, if an attacker crafts size in the bootconfig footer such that adding BOOTCONFIG_FOOTER_SIZE wraps around (for instance, if size is 0xFFFFFFFF), the size check in load_xbc_from_initrd() can be bypassed:
if (stat.st_size < size + BOOTCONFIG_FOOTER_SIZE) {
pr_err("bootconfig size is too big\n");
return -E2BIG;
}This would lead to load_xbc_fd() being called with a negative size, resulting in an undersized allocation, heap overflow on read, and out-of-bounds write on the terminating null byte.
Avoid the integer overflow by checking whether size is greater than stat.st_size - BOOTCONFIG_FOOTER_SIZE. Since stat.st_size is already verified to be at least BOOTCONFIG_FOOTER_SIZE earlier in the function, this subtraction will never underflow.
Also reject size > XBC_DATA_MAX upfront in load_xbc_from_initrd() to avoid allocating and reading oversized configurations into memory before xbc_init() validation.
---
Changes in v2:
- Add Cc: stable.
- Also reject if "size > XBC_DATA_MAX", that is obviously wrong.
--- tools/bootconfig/main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
tools/bootconfig/main.c +2 -1
@@ -218,7 +218,8 @@ static int load_xbc_from_initrd(int fd, char **buf) csum =le32toh (csum);/* Wrong size error */ -if (stat.st_size < size +BOOTCONFIG_FOOTER_SIZE ) { +if (size >XBC_DATA_MAX || + size > stat.st_size -BOOTCONFIG_FOOTER_SIZE ) {pr_err ("bootconfig size is too big\n" );return -E2BIG ; }
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The integer overflow vulnerability fixed in user-space still exists in the kernel's bootconfig loader (init/main.c), allowing a crafted initrd to bypass bounds checks and cause an out-of-bounds read and kernel panic.
- [High] Truncation of off_t to int causes a heap buffer overflow and underflow when processing large standalone bootconfig files.
--
commit 1cf2181ef71cd516068d73bd6993bef0811e1b9d
Author: Masami Hiramatsu (Google) <[email protected]>
Subject: tools/bootconfig: Fix integer overflow in size check
This patch fixes an integer overflow vulnerability in load_xbc_from_initrd() within the bootconfig tool. On 32-bit systems, a maliciously large bootconfig size could wrap around, bypassing bounds checks and causing memory corruption. The fix rejects overly large sizes upfront and verifies against stat.st_size in a way that avoids underflow.
tools/bootconfig/main.c static int load_xbc_from_initrd(int fd, char **buf) · line 222
Quoted 11 lines
tools/bootconfig/main.c +2
-1@@ -218,7 +218,8 @@ static int load_xbc_from_initrd(int fd, char **buf) csum =le32toh (csum);/* Wrong size error */ -if (stat.st_size < size +BOOTCONFIG_FOOTER_SIZE ) { +if (size >XBC_DATA_MAX || + size > stat.st_size -BOOTCONFIG_FOOTER_SIZE ) {
[Severity: High]
This isn't a bug introduced by this patch, but the exact same integer overflow vulnerability appears to exist in the kernel's bootconfig loader.
On 32-bit systems, a crafted initrd with a huge bootconfig size (e.g., 0xFFFFFFFF) can cause the pointer arithmetic to wrap around:
init/main.c:get_boot_config_from_initrd() {
...
data = ((void *)hdr) - size;
if ((unsigned long)data < initrd_start) {
pr_err("bootconfig size %d is greater than initrd size %ld\n",
...
}Because data wraps around, the bounds check (unsigned long)data < initrd_start evaluates to false.
The kernel then calls xbc_calc_checksum(data, size), attempting to read 4GB of memory, which hits unmapped pages and triggers a fatal page fault.
Could we apply a similar bounds check to get_boot_config_from_initrd() in init/main.c?
[Severity: High]
This is a pre-existing issue, but does load_xbc_file() have a truncation issue that can cause a heap buffer overflow?
When processing large standalone bootconfig files, load_xbc_file() passes the 64-bit off_t file size into load_xbc_fd(), which expects a 32-bit int:
tools/bootconfig/main.c:load_xbc_file() {
...
ret = load_xbc_fd(fd, buf, stat.st_size);
...
}tools/bootconfig/main.c:load_xbc_fd(int fd, char **buf, int size) {
...
*buf = malloc(size + 1);
if (!*buf)
return -ENOMEM;
ret = read(fd, *buf, size);
if (ret < 0)
return -errno;
(*buf)[size] = '\0';
...
}If a user or script runs bootconfig -a on an extremely large file (exactly 4GB - 1 byte in size), stat.st_size truncates to -1 when passed as the int size parameter.
malloc(-1 + 1) correctly evaluates to malloc(0) and succeeds. read(fd, *buf, -1) is then called, where -1 implicitly casts to SIZE_MAX, causing the kernel to write up to ~2GB of file data into the 0-byte buffer.
Finally, (*buf)[size] = '\0' writes a null byte at index -1, corrupting heap metadata.
Should load_xbc_fd() take a size_t instead of an int?
Quoted 3 lines
tools/bootconfig/main.c +0
-0pr_err ("bootconfig size is too big\n" );return -E2BIG ; }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/178900114498.187028.9558747732041210116.stgit@devnote2?part=1