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 [off-list ref]
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.
quoted hunk ↗ jump to hunk
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?
quoted hunk ↗ jump to hunk
+ ret = eh_frame_do_def_cfa_expression(expr, expr_len, target_ip, &ctx->state);
+ if (ret)
+ return ret;
+ cur += expr_len;
+ break;
+ }
+
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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?
quoted hunk ↗ jump to hunk
+ 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/20260821195259.2688377-1-jremus@linux.ibm.com?part=17