Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Mismatched boundary checks for XBC_DATA_MAX cause inconsistent fallback behavior for embedded bootconfigs. - [Low] Misleading error message when bootconfig size exceeds XBC_DATA_MAX. --
commit a24325c6fe0374f1cfc3fc8cbe63efbad0f22f32 Author: Masami Hiramatsu (Google) <[email protected]> bootconfig: Fix integer overflow in initrd size check This commit fixes an integer overflow where a crafted initrd with a huge bootconfig size can cause pointer arithmetic wrap-around. It ensures the initrd bounds are respected and verifies the size does not exceed max limits. Closes: https://lore.kernel.org/all/[email protected]/ > diff --git a/init/main.c b/init/main.c > index a9362b2cc258a..de24c41ce815e 100644 > --- a/init/main.c > +++ b/init/main.c [ ... ] > @@ -294,16 +295,21 @@ static void * __init get_boot_config_from_initrd(size_t > *_size) > > found: > hdr = (u8 *)(data - 8); > + if ((unsigned long)hdr < initrd_start) > + return NULL; > + > size = get_unaligned_le32(hdr); > csum = get_unaligned_le32(hdr + 4); > > - data = ((void *)hdr) - size; > - if ((unsigned long)data < initrd_start) { > - pr_err("bootconfig size %d is greater than initrd size %ld\n", > + if (size > XBC_DATA_MAX || [Severity: Medium] If the size is exactly equal to XBC_DATA_MAX, this check passes and the function returns a valid data pointer. This prevents the caller from falling back to the embedded bootconfig: init/main.c:setup_boot_config() { ... data = get_boot_config_from_initrd(&size); if (!data) { data = xbc_get_embedded_bootconfig(&size); ... } ... } However, setup_boot_config() uses inclusive inequality for XBC_DATA_MAX: init/main.c:setup_boot_config() { ... if (size >= XBC_DATA_MAX) { pr_err("bootconfig size %ld greater than max size %d\n", ...); return; } ... } Because the limits don't match, an exact size of XBC_DATA_MAX causes setup_boot_config() to reject the payload and abort without attempting the fallback. Should this check use size >= XBC_DATA_MAX to match? > + size > ((unsigned long)hdr - initrd_start)) { > + pr_err("bootconfig size %u is greater than initrd size %lu\n", > + size, initrd_end - initrd_start); [Severity: Low] If this error path is taken specifically because size > XBC_DATA_MAX, the error message claims the bootconfig size is greater than the initrd size. Can this message be updated to accurately reflect which limit was exceeded? > return NULL; > } > > + data = ((void *)hdr) - size; > + > if (xbc_calc_checksum(data, size) != csum) { > pr_err("bootconfig checksum failed\n"); > return NULL; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/178900555453.200943.4719889510443114485.stgit@devnote2?part=2
