Re: [RFC PATCH v1 14/25] unwind_user/eh_frame: Add .eh_frame[_hdr] validation option
From: sashiko-bot@kernel.org
Date: 2026-08-18 15:08:23
Also in:
linux-s390
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] Calling printk() inside a scoped_user_read_access_size block causes a UACCESS rule violation.
- [High] Infinite loop leading to local Denial of Service due to a 32-bit loop counter overflowing against a 64-bit upper bound.
- [Medium] CPU soft lockup due to unbounded loop over potentially millions of user-controlled entries without yielding.
- [Low] Compilation failure due to referencing a non-existent struct member `has_binary_search_table`.
- [Low] Spelling error in Kconfig help text ('entrire' instead of 'entire').
Pre-existing issues:
- [High] Information leak of uninitialized kernel stack memory to userspace via the unwinder.
--
--- Patch [14]: [RFC PATCH v1 14/25] unwind_user/eh_frame: Add .eh_frame[_hdr] validation option ---commit e7ce89dcff055122fac1a936bcef539a42d26943 Author: Jens Remus [off-list ref] unwind_user/eh_frame: Add .eh_frame[_hdr] validation option Add a debug feature to validate all .eh_frame[_hdr] sections when first loading the file rather than on demand.
quoted hunk ↗ jump to hunk
diff --git a/arch/Kconfig b/arch/Kconfig --- a/arch/Kconfig +++ b/arch/Kconfig@@ -490,6 +490,28 @@ config HAVE_UNWIND_USER_EH_FRAME bool select UNWIND_USER +config EH_FRAME_VALIDATION + bool "Enable .eh_frame[_hdr] section debugging" + depends on HAVE_UNWIND_USER_EH_FRAME + depends on DYNAMIC_DEBUG + help + When adding an .eh_frame_hdr section for a test, validate the + entire section and its referenced entrire .eh_frame section
[Severity: Low] Is there a typo here? (entrire -> entire)
+ immediately rather than on demand.
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/kernel/unwind/eh_frame.c b/kernel/unwind/eh_frame.c --- a/kernel/unwind/eh_frame.c +++ b/kernel/unwind/eh_frame.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1107,11 +1107,11 @@ static __always_inline int __find_frame_row(struct eh_frame_section *sec, struct eh_frame_fde *fde, unsigned long ip, struct unwind_user_frame *frame) { struct eh_frame_cfi_context ctx;
[Severity: High] This is a pre-existing issue, but can this leak uninitialized kernel stack memory to userspace via the unwinder? In __find_frame_row(), ctx is allocated on the kernel stack without initialization. If an attacker provides a crafted .eh_frame starting with DW_CFA_def_cfa_register, __do_cfi_program() sets cfa_rule = CFA_REG_OFFSET but leaves ctx.state.cfa_offset uninitialized. This uninitialized kernel stack value is assigned to frame->cfa.offset and used by the unwinder to compute the user stack pointer. [ ... ]
quoted hunk ↗ jump to hunk
@@ -1163,6 +1163,95 @@ int eh_frame_find(unsigned long ip, struct unwind_user_frame *frame) return ret; } +#ifdef CONFIG_EH_FRAME_VALIDATION + +static int eh_frame_validate_section(struct eh_frame_section *sec) +{ + void __user *table_start_ptr; + unsigned long table_size; + u8 table_enc; + int entry_size; + unsigned long prev_func_addr; + unsigned int i; + + if (!sec->has_binary_search_table)
[Severity: Low] Does this cause a build failure? It appears has_binary_search_table was never added to struct eh_frame_section.
+ return 0;
+
+ if (!sec->fde_count) {
+ dbg_sec(".eh_frame_hdr: invalid FDE count\n");
+ return -EINVAL;
+ }
+
+ table_enc = sec->binary_search_table_enc;
+ entry_size = 2 * encoded_pointer_size(table_enc);
+ if (!entry_size) {
+ dbg_sec(".eh_frame_hdr: invalid binary search table entry size\n");
+ return -EINVAL;
+ }
+ table_start_ptr = (void __user *)sec->binary_search_table_start;
+ table_size = sec->binary_search_table_end - sec->binary_search_table_start;
+
+ for (i = 0; i < sec->fde_count; i++) {[Severity: High] Can this loop become infinite and cause a hard lockup? The loop counter i is a 32-bit unsigned int, while sec->fde_count is a 64-bit unsigned long. If sec->fde_count exceeds UINT_MAX, i will wrap around to 0, and the condition i < sec->fde_count will always evaluate to true. [Severity: Medium] Additionally, can this unbounded loop over potentially millions of entries cause a CPU soft lockup or RCU stall? This loop executes entirely without a cond_resched() call, allowing a local user to load a moderately large ELF file and hold the CPU uninterrupted for seconds.
+ struct eh_frame_fde fde;
+ unsigned long cur;
+ unsigned long func_addr, fde_addr;
+ int ret;
+
+ cur = sec->binary_search_table_start + i * entry_size;
+
+ scoped_user_read_access_size(table_start_ptr, table_size, Efault) {
+ /* Read function start address from table */
+ ret = read_encoded_pointer(sec, NULL, &cur,
+ sec->binary_search_table_end,
+ table_enc, &func_addr);
+ if (ret) {
+ dbg_sec_ehfh(cur, "table[%u]: failed to read function start address\n", i);[Severity: High] Does calling dbg_sec_ehfh() inside scoped_user_read_access_size violate UACCESS rules? The macro scoped_user_read_access_size enables UACCESS. dbg_sec_ehfh expands to pr_debug, which calls printk. Calling complex or sleepable functions like printk with UACCESS enabled can trigger page faults, take locks, or schedule, potentially leading to kernel oopses or panics.
+ return ret; + }
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260818144954.2320378-1-jremus@linux.ibm.com?part=14