| Message ID | 20240730154500.3155437-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [RFC] uretprobe: change syscall number, again | expand |
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, > > I've queued up this version in the asm-generic tree so I can send a > pull request in the next few days, but I'm fine with doing this a > differently if someone has a stronger opinion on what numbers to > assign for it on earch architecture. > > arch/x86/entry/syscalls/syscall_64.tbl | 2 +- > include/uapi/asm-generic/unistd.h | 5 +---- > scripts/syscall.tbl | 1 - > 3 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl > index 83073fa3c989..7093ee21c0d1 100644 > --- a/arch/x86/entry/syscalls/syscall_64.tbl > +++ b/arch/x86/entry/syscalls/syscall_64.tbl > @@ -344,6 +344,7 @@ > 332 common statx sys_statx > 333 common io_pgetevents sys_io_pgetevents > 334 common rseq sys_rseq > +335 common uretprobe sys_uretprobe > # don't use numbers 387 through 423, add new calls after the last > # 'common' entry > 424 common pidfd_send_signal sys_pidfd_send_signal > @@ -385,7 +386,6 @@ > 460 common lsm_set_self_attr sys_lsm_set_self_attr > 461 common lsm_list_modules sys_lsm_list_modules > 462 common mseal sys_mseal > -467 common uretprobe sys_uretprobe > > # > # Due to a historical design error, certain syscalls are numbered differently > diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h > index 985a262d0f9e..5bf6148cac2b 100644 > --- a/include/uapi/asm-generic/unistd.h > +++ b/include/uapi/asm-generic/unistd.h > @@ -841,11 +841,8 @@ __SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules) > #define __NR_mseal 462 > __SYSCALL(__NR_mseal, sys_mseal) > > -#define __NR_uretprobe 463 > -__SYSCALL(__NR_uretprobe, sys_uretprobe) > - > #undef __NR_syscalls > -#define __NR_syscalls 464 > +#define __NR_syscalls 463 > > /* > * 32 bit systems traditionally used different > diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl > index 591d85e8ca7e..797e20ea99a2 100644 > --- a/scripts/syscall.tbl > +++ b/scripts/syscall.tbl > @@ -402,4 +402,3 @@ > 460 common lsm_set_self_attr sys_lsm_set_self_attr > 461 common lsm_list_modules sys_lsm_list_modules > 462 common mseal sys_mseal > -467 common uretprobe sys_uretprobe > -- > 2.39.2 >
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)
On Fri, Aug 2, 2024, at 11:14, Masami Hiramatsu wrote: > On Tue, 30 Jul 2024 17:43:36 +0200 Arnd Bergmann <arnd@kernel.org> wrote: >> --- >> 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? I folded the change that Jiri suggested now. > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thanks, Arnd
On Fri, 02 Aug 2024 15:18:52 +0200 "Arnd Bergmann" <arnd@arndb.de> wrote: > On Fri, Aug 2, 2024, at 11:14, Masami Hiramatsu wrote: > > On Tue, 30 Jul 2024 17:43:36 +0200 Arnd Bergmann <arnd@kernel.org> wrote: > >> --- > >> 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? > > I folded the change that Jiri suggested now. Thanks Jiri and Arnd for fix that! > > > Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Thanks, > > Arnd >
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl index 83073fa3c989..7093ee21c0d1 100644 --- a/arch/x86/entry/syscalls/syscall_64.tbl +++ b/arch/x86/entry/syscalls/syscall_64.tbl @@ -344,6 +344,7 @@ 332 common statx sys_statx 333 common io_pgetevents sys_io_pgetevents 334 common rseq sys_rseq +335 common uretprobe sys_uretprobe # don't use numbers 387 through 423, add new calls after the last # 'common' entry 424 common pidfd_send_signal sys_pidfd_send_signal @@ -385,7 +386,6 @@ 460 common lsm_set_self_attr sys_lsm_set_self_attr 461 common lsm_list_modules sys_lsm_list_modules 462 common mseal sys_mseal -467 common uretprobe sys_uretprobe # # Due to a historical design error, certain syscalls are numbered differently diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h index 985a262d0f9e..5bf6148cac2b 100644 --- a/include/uapi/asm-generic/unistd.h +++ b/include/uapi/asm-generic/unistd.h @@ -841,11 +841,8 @@ __SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules) #define __NR_mseal 462 __SYSCALL(__NR_mseal, sys_mseal) -#define __NR_uretprobe 463 -__SYSCALL(__NR_uretprobe, sys_uretprobe) - #undef __NR_syscalls -#define __NR_syscalls 464 +#define __NR_syscalls 463 /* * 32 bit systems traditionally used different diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl index 591d85e8ca7e..797e20ea99a2 100644 --- a/scripts/syscall.tbl +++ b/scripts/syscall.tbl @@ -402,4 +402,3 @@ 460 common lsm_set_self_attr sys_lsm_set_self_attr 461 common lsm_list_modules sys_lsm_list_modules 462 common mseal sys_mseal -467 common uretprobe sys_uretprobe