linux-kernel - Re: [RFC] uretprobe: change syscall number, again

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZqzIbU_T6sN01o5B@krava>
Date: Fri, 2 Aug 2024 13:52:13 +0200
From: Jiri Olsa <olsajiri@...il.com>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: Arnd Bergmann <arnd@...nel.org>, Andy Lutomirski <luto@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	Arnd Bergmann <arnd@...db.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Palmer Dabbelt <palmer@...belt.com>, Guo Ren <guoren@...nel.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Kees Cook <kees@...nel.org>,
	"peterz@...radead.org" <peterz@...radead.org>,
	"H.J. Lu" <hjl.tools@...il.com>,
	Sohil Mehta <sohil.mehta@...el.com>,
	Oleg Nesterov <oleg@...hat.com>,
	Andrii Nakryiko <andrii@...nel.org>, linux-kernel@...r.kernel.org,
	linux-arch@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [RFC] uretprobe: change syscall number, again

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@...nel.org> wrote:
> 
> > From: Arnd Bergmann <arnd@...db.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@...ux-foundation.org>
> > Signed-off-by: Arnd Bergmann <arnd@...db.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@...nel.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@...nel.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)