Re: [RFC PATCH v1 18/25] unwind_user/eh_frame: Add support for DWARF expressions
Jens Remus <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-s390,org.kernel.vger.linux-trace-kernel |
|---|---|
| Organization | IBM Deutschland Research & Development GmbH |
| Message-ID | <[email protected]> |
On 8/18/2026 5:13 PM, [email protected] wrote: > This commit enables architectures to handle DWARF expressions in DWARF CFI > instructions. It limits the maximum expression length to a reasonable size > and provides inline stubs for architectures to override. >> diff --git a/include/linux/unwind_user_eh_frame_types.h b/include/linux/unwind_user_eh_frame_types.h >> +struct eh_frame_reg_state { >> + /* CFA recovery rule */ >> + enum eh_frame_cfa_rule cfa_rule; >> + unsigned long cfa_regnum; >> + long cfa_offset; > > [Severity: High] > What ensures cfa_offset is initialized when processing CFI programs? > > When struct eh_frame_cfi_context ctx is allocated on the kernel stack in > __do_cfi_program(), its state.cfa_rule is initialized, but state.cfa_offset > is not. > > If a user-supplied .eh_frame begins with DW_CFA_def_cfa_register, it sets > ctx.state.cfa_rule = CFA_REG_OFFSET but leaves cfa_offset untouched. Fixed in "unwind_user/eh_frame: Add support for reading .eh_frame section". > > When __find_frame_row() executes, it verifies cfa_rule == CFA_REG_OFFSET > and unconditionally reads the uninitialized kernel stack memory. Could this > cause the unwinder to use unpredictable values for the CFA offset? >> diff --git a/kernel/unwind/eh_frame.c b/kernel/unwind/eh_frame.c >> + case DW_CFA_def_cfa_expression: { >> + unsigned long expr_len; >> + char expr[EH_FRAME_MAX_EXPRESSION_LENGTH]; >> + >> + ret = read_uleb128(&cur, end, &expr_len); >> + if (ret) >> + return ret; >> + >> + if (cur + expr_len > end) if (cur + expr_len < cur || cur + expr_len > end) > > [Severity: High] > Can the pointer arithmetic wrap around if expr_len is maliciously large? > > Since expr_len is parsed from untrusted user input as an unsigned long, a > large value could overflow cur + expr_len and wrap to a small value, > bypassing this check against end. > > If this occurs, the parsing loop would advance cur by the overflowed > expr_len, moving cur backwards. Would this lead to an infinite parsing loop > and a hard lockup? Valid. Improved overflow check. > >> + return -EINVAL; >> + >> + if (expr_len > sizeof(expr)) >> + return -EOPNOTSUPP; >> + unsafe_copy_from_user(&expr, (void __user *)cur, expr_len, Efault); > > [Severity: Critical] > Might unsafe_copy_from_user() improperly toggle SMAP during expression > parsing? > > Because this parsing runs inside a scoped_user_read_access_size block, > calling unsafe_copy_from_user() on x86 defaults to __copy_from_user(), > which executes its own stac() and clac() instructions. > > The inner clac() instruction clears the AC flag prematurely, ending the > UACCESS region while the outer block assumes it is still open. > > When the loop continues to parse the next instruction using > UNSAFE_GET_USER_INC, it will attempt a direct user memory read with the AC > flag cleared. Will this result in a fatal page fault? False. IIUC config option HAVE_UNWIND_USER_EH_FRAME cannot be manually enabled. Therefore this code cannot be enabled on any architecture, including x86. unsafe_copy_from_user() is added for x86 by subsequent patch "[RFC PATCH v1 19/25] x86/uaccess: Add unsafe_copy_from_user() implementation". > >> + ret = eh_frame_do_def_cfa_expression(expr, expr_len, target_ip, &ctx->state); > > [Severity: Medium] > Is it safe to call an architecture hook inside a UACCESS region? > > If an architecture overrides eh_frame_do_def_cfa_expression() with an > out-of-line external function, it will trigger objtool's 'call to XXX() > with UACCESS enabled' warning. > > Executing arbitrary functions with the AC flag set is strictly forbidden > on x86 to prevent SMAP bypasses. Steven, Josh, what are your thoughts? > >> + if (ret) >> + return ret; >> + cur += expr_len; >> + break; >> + } Thanks and regards, Jens -- Jens Remus Linux on Z Development (D3303) [email protected] / [email protected] IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294 IBM Data Privacy Statement: https://www.ibm.com/privacy/