On 8/21/2026 10:16 PM, [email protected] wrote:
> In preparation for using .eh_frame to unwind user space stacks, add an
> eh_frame_find() interface for finding the .eh_frame[_hdr] information
> associated with a given text address.
>> diff --git a/kernel/unwind/eh_frame.c b/kernel/unwind/eh_frame.c
>> +static __always_inline int __read_cie(struct eh_frame_section *sec,
>> + unsigned long cie_addr,
>> + struct eh_frame_cie *cie)
>> +{
>> + void __user *cie_ptr = (void __user *)cie_addr;
>> + unsigned long cur = cie_addr, end;
>
> [ ... ]
>
>> + scoped_user_read_access_size(cie_ptr, 4 + length, Efault) {
>> + /* Read CIE_ID (must be 0 for CIE; FDE otherwise) */
>> + UNSAFE_GET_USER_INC(cie_id, cur, end, Efault);
>
> [Severity: High]
> Are we bypassing the pointer masking mitigation here?
True. Fixed in next version likewise as for eh_frame_read_header() in
preceeding "[RFC PATCH v2 07/24] unwind_user/eh_frame: Add support for
reading .eh_frame_hdr section".
>
> The scoped_user_read_access_size() macro shadows the passed pointer (cie_ptr)
> with a masked version to prevent speculative kernel accesses. However, the
> memory read via UNSAFE_GET_USER_INC() uses the separate unmasked cur variable.
>
> Could this allow speculative out-of-bounds reads to access kernel memory?
>
>> + if (cie_id != EH_FRAME_CIE_ID)
>> + return -EINVAL;
>> +static __always_inline int __do_cfi_insn(struct eh_frame_section *sec,
>> + struct eh_frame_fde *fde,
>> + unsigned long *cur_ptr,
>> + unsigned long end,
>> + unsigned long *ip_ptr,
>> + unsigned long target_ip,
>> + struct eh_frame_cfi_context *ctx)
>> +{
>
> [ ... ]
>
>> + case DW_CFA_restore: {
>> + u8 reg = DW_CFA_operand(opcode);
>> + int idx;
>> +
>> + if (ctx->cie)
>> + return -EINVAL;
>> +
>> + idx = reg_to_index(reg);
>> + if (idx >= 0)
if (idx >= 0) {
>> + ctx->state.reg_rule[idx] = ctx->cie_state.reg_rule[idx];
ctx->state.reg_regnum[idx] =
ctx->cie_state.reg_regnum[idx];
ctx->state.reg_offset[idx] =
ctx->cie_state.reg_offset[idx];
}
>> + break;
>> + }
>
> [Severity: High]
> Does this properly restore the complete register state?
>
> It appears only the rule type is restored from ctx->cie_state, but the
> offset (reg_offset) and register number (reg_regnum) are not restored and
> will remain at their previously modified values.
Valid. I was in a rush to get all of the RFC v1 Sashiko AI review
resolved and was used to the struct unwind_user_rule_data approach,
where a single assignment is sufficient. I will do likewise here
and introduce struct eh_frame_{cfa|reg}_rule_data:
struct eh_frame_cfa_rule_data {
enum eh_frame_cfa_rule rule;
unsigned long regnum;
long offset;
};
struct eh_frame_reg_rule_data {
enum eh_frame_reg_rule rule;
unsigned long regnum;
long offset;
};
struct eh_frame_state {
struct eh_frame_cfa_rule_data cfa;
struct eh_frame_reg_rule_data reg[NR_REGS];
};
struct eh_frame_cfi_context {
struct eh_frame_state state;
struct eh_frame_state state_stack[EH_FRAME_MAX_STATE_STACK];
struct eh_frame_state cie_state;
...
Another option would be to use struct unwind_user[_cfa]_rule_data.
For that purpose those would need to be updated to represent the
offset and register number as (unsigned) long. The question is
how to deal with UNWIND_USER_CFA_RULE_{SP|FP}_OFFSET then.
eh_frame could internally use UNWIND_USER_CFA_RULE_REG_OFFSET and
only before returning the info to unwind user convert to
UNWIND_USER_CFA_RULE_{SP|FP}_OFFSET if appropriate.
I will explore that in a future version.
>
> [ ... ]
>
>> + case DW_CFA_advance_loc4: {
>> + unsigned long offset;
>> + u32 delta;
>> +
>> + UNSAFE_GET_USER_INC(delta, cur, end, Efault);
>> + offset = delta * fde->cie.code_align;
if (check_mul_overflow(delta, fde->cie.code_align,
&offset))
return -EINVAL;
>
> [Severity: Medium]
> Can this calculation overflow?
Valid. Will add a check in next version.
>
> The multiplication of delta (a u32) and fde->cie.code_align (an unsigned int)
> evaluates as a 32-bit unsigned integer before being assigned to the 64-bit
> unsigned long offset.
>
> If the product exceeds 32 bits, the high bits would be truncated, which might
> cause the unwinder to desync from the actual PC.
>
>> + ip += offset;
if (check_add_overflow(loc, offset, &loc))
return -EINVAL;
Likewise for all other DW_CFA_advance_loc* flavors.
>> + break;
>> + }
>
> [ ... ]
>
>> + case DW_CFA_restore_extended: {
>> + unsigned long reg;
>> + int idx;
>> +
>> + if (ctx->cie)
>> + return -EINVAL;
>> +
>> + ret = read_uleb128(&cur, end, ®);
>> + if (ret)
>> + return ret;
>> +
>> + idx = reg_to_index(reg);
>> + if (idx >= 0)
>> + ctx->state.reg_rule[idx] =
>> ctx->cie_state.reg_rule[idx];
>> + break;
>> + }
>
> [Severity: High]
> This seems to have the same register state restoration issue as
> DW_CFA_restore.
> Should reg_offset and reg_regnum also be restored here alongside the rule
> type?
Same as for DW_CFA_restore above.
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/
