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/