Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Unaligned stack buffer passed to unsafe_copy_from_user() causes undefined behavior --
--- Patch [17]: [RFC PATCH v2 17/24] unwind_user/eh_frame: Add support for DWARF expressions --- commit 38e0b8657cdb2bd1e95dd3bc74b1aa71e0063c9a Author: Jens Remus <[email protected]> unwind_user/eh_frame: Add support for DWARF expressions Enable architectures to handle DWARF expressions in DWARF CFI instructions DW_CFA_def_cfa_expression, DW_CFA_expression, and DW_CFA_val_expression. Limit the maximum expression length to a reasonable size, while enabling architectures to override the limit. > diff --git a/kernel/unwind/eh_frame.c b/kernel/unwind/eh_frame.c > index d185187b0c34e..a98804dbe5886 100644 > --- a/kernel/unwind/eh_frame.c > +++ b/kernel/unwind/eh_frame.c > @@ -803,6 +772,27 @@ static __always_inline int __do_cfi_insn(struct > eh_frame_section *sec, > break; > } > > + 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 < cur || cur + expr_len > end) > + return -EINVAL; > + > + if (expr_len > sizeof(expr)) > + return -EOPNOTSUPP; > + unsafe_copy_from_user(&expr, (void __user *)cur, > expr_len, Efault); [Severity: Low] Does this unaligned stack buffer passed to unsafe_copy_from_user() cause undefined behavior? Passing a 1-byte aligned char array to unsafe_copy_from_user() violates C alignment rules. In architecture-specific implementations, the macro explicitly casts the destination pointer to larger integer types (e.g., u64 *) to perform optimized word-sized memory assignments. Dereferencing an unaligned pointer is Undefined Behavior in C. While x86 hardware gracefully handles unaligned memory accesses, the compiler's optimizer is legally allowed to assume u64 * is 8-byte aligned and could emit trapping instructions. Furthermore, if this generic .eh_frame code is later enabled on architectures with strict hardware alignment enforcement, it will immediately trigger alignment exceptions. Could this buffer be declared with __aligned(8) or as an array of u64? > + ret = eh_frame_do_def_cfa_expression(expr, expr_len, > target_ip, &ctx->state); > + if (ret) > + return ret; > + cur += expr_len; > + break; > + } > + [ ... ] > @@ -970,12 +960,22 @@ static __always_inline int __do_cfi_insn(struct > eh_frame_section *sec, > if (ret) > return ret; > > - if (cur + expr_len > end) > + if (cur + expr_len < cur || cur + expr_len > end) > return -EINVAL; > > - if (reg == EH_FRAME_REG_SP || reg == EH_FRAME_REG_FP || > reg == EH_FRAME_REG_RA) > - return -EOPNOTSUPP; > + if (reg == EH_FRAME_REG_SP || reg == EH_FRAME_REG_FP || > reg == EH_FRAME_REG_RA) { > + char expr[EH_FRAME_MAX_EXPRESSION_LENGTH]; > > + if (expr_len > sizeof(expr)) > + return -EOPNOTSUPP; > + unsafe_copy_from_user(&expr, (void __user > *)cur, expr_len, Efault); [Severity: Low] Is this the same alignment issue as above? Could this buffer also be declared with __aligned(8) or as an array of u64? > + if (opcode == DW_CFA_expression) > + ret = eh_frame_do_expression(reg, expr, > expr_len, target_ip, &ctx->state); > + else > + ret = eh_frame_do_val_expression(reg, > expr, expr_len, target_ip, &ctx->state); > + if (ret) > + return ret; > + } > cur += expr_len; > break; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=17
