Re: [PATCH v14 14/19] unwind_user: Flexible FP/RA recovery rules

On 5/5/2026 8:34 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Unprivileged users can trigger a kernel warning (and panic if panic_on_warn is set) by providing crafted SFrame data that forces the use of unimplemented register recovery rules.
> --
> 
> commit 32f67c3538981cd61e598218f28891cb4051e567
> Author: Jens Remus <[email protected]>
> 
> unwind_user: Flexible FP/RA recovery rules
> 
> This commit adds support for flexible frame pointer and return address 
> recovery rules to the user unwinder. It introduces rules to retain values
> or read them from registers.
> 
>> diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
>> index bc2edae399556..92cdf38c8aded 100644
>> --- a/include/linux/unwind_user.h
>> +++ b/include/linux/unwind_user.h
>> @@ -32,6 +32,15 @@ static inline int unwind_user_get_ra_reg(unsigned long *val)
>>  #define unwind_user_get_ra_reg unwind_user_get_ra_reg
>>  #endif
>>  
>> +#ifndef unwind_user_get_reg
>> +static inline int unwind_user_get_reg(unsigned long *val, unsigned int regnum)
>> +{
>> +	WARN_ON_ONCE(1);
>> +	return -EINVAL;

Replace with the following to resolve below issue:

	dbg_once("unwind_user_get_reg(%u) not implemented\n", regnum);

>> +}
>> +#define unwind_user_get_reg unwind_user_get_reg
>> +#endif
> 
> Could an unprivileged user trigger these WARN_ON_ONCE macros by providing
> crafted SFrame data during profiling?

Valid.  On architectures that do not support SFrame V3 flexible FDEs and
therefore do not implement unwind_user_get_ra_reg(), such as arm64,
above WARN_ON_ONCE(1) can be triggered using forged .sframe that
maliciously has SFrame flexible FDE/FRE that encode the FP/RA recovery
rule as register + offset or *(register + offset), with register != SP/FP.

> 
> If an architecture does not implement unwind_user_get_reg or
> unwind_user_get_ra_reg, falling back to these stubs will execute a kernel
> warning. On systems with panic_on_warn enabled, this could lead to a
> kernel panic.
> 
> [ ... ]
> 
>> diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
>> index 9ceef9b2b8db0..89aecfbe3e849 100644
>> --- a/kernel/unwind/user.c
>> +++ b/kernel/unwind/user.c
>> @@ -61,22 +61,61 @@ static int unwind_user_next_common(struct unwind_user_state *state,
> [ ... ]
>> +	switch (frame->ra.rule) {
>> +	case UNWIND_USER_RULE_RETAIN:
>>  		if (!state->topmost || unwind_user_get_ra_reg(&ra))
>>  			return -EINVAL;
>> +		break;
> [ ... ]
>> +	default:
>> +		WARN_ON_ONCE(1);
>> +		return -EINVAL;
>>  	}
> 
> A similar issue seems to exist in the default case here, as well as the
> default case for frame->fp.rule below it.

Resolve by explicitly handling all defined enum values.  Keep the
WARN_ON_ONCE(1) to detect errors if extending the related enums.

> 
> Since sframe_init_rule_data() sets the rule based on untrusted user memory
> (for example, setting UNWIND_USER_RULE_RETAIN when ra_off is 0), can a
> crafted binary intentionally hit these paths?
> 
> Would it be safer to silently fail and return -EINVAL when an unsupported
> or unimplemented rule is encountered, rather than warning?
> 

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/