* Re: [PATCH v14 19/19] unwind_user/sframe: Add prctl() interface for registering .sframe sections
[not found] ` <20260505184556.B94A0C2BCB4@smtp.kernel.org>
@ 2026-05-07 14:14 ` Jens Remus
0 siblings, 0 replies; only message in thread
From: Jens Remus @ 2026-05-07 14:14 UTC (permalink / raw)
To: Steven Rostedt; +Cc: bpf, sashiko, Josh Poimboeuf, Indu Bhagat
On 5/5/2026 8:45 PM, sashiko-bot@kernel.org 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 <jpoimboe@kernel.org>
>
> 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/20260429114355.6c712e6a@gandalf.local.home/
>
>> 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)
jremus@de.ibm.com / jremus@linux.ibm.com
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/
^ permalink raw reply [flat|nested] only message in thread