Re: [RFC] uretprobe: change syscall number, again

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Aug 02, 2024 at 06:14:37PM +0900, Masami Hiramatsu wrote:
> On Tue, 30 Jul 2024 17:43:36 +0200
> Arnd Bergmann <arnd@kernel.org> wrote:
> 
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > Despite multiple attempts to get the syscall number assignment right
> > for the newly added uretprobe syscall, we ended up with a bit of a mess:
> > 
> >  - The number is defined as 467 based on the assumption that the
> >    xattrat family of syscalls would use 463 through 466, but those
> >    did not make it into 6.11.
> 
> OK... that was not expected.
> 
> > 
> >  - The include/uapi/asm-generic/unistd.h file still lists the number
> >    463, but the new scripts/syscall.tbl that was supposed to have the
> >    same data lists 467 instead as the number for arc, arm64, csky,
> >    hexagon, loongarch, nios2, openrisc and riscv. None of these
> >    architectures actually provide a uretprobe syscall.
> 
> Oops, thanks for finding.
> 
> > 
> >  - All the other architectures (powerpc, arm, mips, ...) don't list
> >    this syscall at all.
> 
> OK, so even if it is not supported on those, we need to put it as a
> placeholder.
> 
> > 
> > There are two ways to make it consistent again: either list it with
> > the same syscall number on all architectures, or only list it on x86
> > but not in scripts/syscall.tbl and asm-generic/unistd.h.
> > 
> > Based on the most recent discussion, it seems like we won't need it
> > anywhere else, so just remove the inconsistent assignment and instead
> > move the x86 number to the next available one in the architecture
> > specific range, which is 335.
> > 
> > Fixes: 5c28424e9a34 ("syscalls: Fix to add sys_uretprobe to syscall.tbl")
> > Fixes: 190fec72df4a ("uprobe: Wire up uretprobe system call")
> > Fixes: 63ded110979b ("uprobe: Change uretprobe syscall scope and number")
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > I think we should fix this as soon as possible. Please let me know if
> > you agree on this approach, or prefer one of the alternatives.
> 
> OK, I think it is good. But you missed to fix a selftest code which
> also needs to be updated.
> 
> Could you revert commit 3e301b431b91 ("selftests/bpf: Change uretprobe
>  syscall number in uprobe_syscall test") too?
> 
> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Thank you,

yes, it still needs the selftest change like below
otherwise if that new number works for you then lgtm

Reviewed-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka


---
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index bd8c75b620c2..5f78edca6540 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -216,7 +216,7 @@ static void test_uretprobe_regs_change(void)
 }
 
 #ifndef __NR_uretprobe
-#define __NR_uretprobe 467
+#define __NR_uretprobe 335
 #endif
 
 __naked unsigned long uretprobe_syscall_call_1(void)