Sashiko discussions
 help / color / mirror / Atom feed
* 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

only message in thread, other threads:[~2026-05-07 14:14 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260505121718.3572346-20-jremus@linux.ibm.com>
     [not found] ` <20260505184556.B94A0C2BCB4@smtp.kernel.org>
2026-05-07 14:14   ` [PATCH v14 19/19] unwind_user/sframe: Add prctl() interface for registering .sframe sections Jens Remus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox