Re: [PATCH v14 19/19] unwind_user/sframe: Add prctl() interface for registering .sframe sections

Jens Remus <[email protected]> Thu, 7 May 2026 16:14:24 +0200
Newsgroups dev.linux.lists.sashiko,org.kernel.vger.bpf
Organization IBM Deutschland Research & Development GmbH
Message-ID <[email protected]>
On 5/5/2026 8:45 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Missing `break;` statement causes `PR_SET_CFI` to fall through to `PR_ADD_SFRAME`.
> - [Medium] Patch introduces a test-only interface into the permanent UAPI.
> --
> 
> commit 9171aa16917963dd1070d9ca3467d9086cd66393
> Author: Josh Poimboeuf <[email protected]>
> 
> unwind_user/sframe: Add prctl() interface for registering .sframe sections
> 
> The commit adds prctl() interfaces which allow glibc to tell the kernel
> where to find .sframe sections. It mentions that the patch is for testing
> purposes only.
> 
>>     [
>>       This adds an interface for prctl() for testing loading of sframes for
>>       libraries. But this interface should really be a system call. This patch
>>       is for testing purposes only and should not be applied to mainline.
>>     ]
> 
> Is it intended to include this in a mainline patch series? Adding temporary
> or test-only interfaces into the UAPI represents a permanent ABI commitment
> once released.

This patch will get replaced by a new syscall that Steven is working on:

[RFC][PATCH] unwind: Add stacktrace_setup system call
https://lore.kernel.org/all/[email protected]/

> 
>> diff --git a/kernel/sys.c b/kernel/sys.c
>> index 62e842055cc9c..6e81e82bc9911 100644
>> --- a/kernel/sys.c
>> +++ b/kernel/sys.c
> [ ... ]
>> @@ -2906,6 +2907,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>>  			break;
>>  		if (arg3 & PR_CFI_LOCK && !(arg3 & PR_CFI_DISABLE))
>>  			error = arch_prctl_lock_branch_landing_pad_state(me);
>> +	case PR_ADD_SFRAME:
>> +		error = sframe_add_section(arg2, arg3, arg4, arg5);
>> +		break;
> 
> Does this cause the previous case to fall through into PR_ADD_SFRAME?
> 
> It appears the new case was inserted before the break statement of the
> previous case. This would cause execution to unintentionally continue into
> sframe_add_section and overwrite the original error status.

Doh!  Stupid mistake during rebase.  Fixed in next version.

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/