[PATCH 0/2] uprobe: Fix uretprobe syscall wiring

[PATCH 0/2] uprobe: Fix uretprobe syscall wiring

[Jiri Olsa]

hi,
the uretprobe syscall clashed in linux-next tree with xattrat syscalls,
so after discussing with Arnd, changing the syscall number to 467.

I'm also changing the ABI to 'common' which will ease up the global
scripts/syscall.tbl management.

I split the change into syscall_64.tbl and selftest change, but it could
be merged together if needed.

The patches are based on:
  https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
  probes/for-next

thanks,
jirka


---
Jiri Olsa (2):
      uprobe: Change uretprobe syscall scope and number
      selftests/bpf: Change uretprobe syscall number in uprobe_syscall test

 arch/x86/entry/syscalls/syscall_64.tbl                  | 2 +-
 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

[PATCH 1/2] uprobe: Change uretprobe syscall scope and number

[Jiri Olsa]

After discussing with Arnd [1] it's preferable to change uretprobe
syscall number to 467 to omit the merge conflict with xattrat syscalls.

Also changing the ABI to 'common' which will ease up the global
scripts/syscall.tbl management. One consequence is we generate uretprobe
syscall numbers for ABIs that do not support uretprobe syscall, but the
syscall still returns -ENOSYS when called in that ABI.

[1] https://lore.kernel.org/lkml/784a34e5-4654-44c9-9c07-f9f4ffd952a0@app.fastmail.com/

Fixes: 190fec72df4a ("uprobe: Wire up uretprobe system call")
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 arch/x86/entry/syscalls/syscall_64.tbl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 6452c2ec469a..dabf1982de6d 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -384,7 +384,7 @@
 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
-463	64	uretprobe		sys_uretprobe
+467	common	uretprobe		sys_uretprobe
 
 #
 # Due to a historical design error, certain syscalls are numbered differently
-- 
2.45.2

[PATCH 2/2] selftests/bpf: Change uretprobe syscall number in uprobe_syscall test

[Jiri Olsa]

Fixing the syscall number value.

Fixes: 9e7f74e64ae5 ("selftests/bpf: Add uretprobe syscall call from user space test")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index c8517c8f5313..bd8c75b620c2 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 463
+#define __NR_uretprobe 467
 #endif
 
 __naked unsigned long uretprobe_syscall_call_1(void)
-- 
2.45.2

Re: [PATCH 1/2] uprobe: Change uretprobe syscall scope and number

[Andrii Nakryiko]

On Fri, Jul 12, 2024 at 6:52 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> After discussing with Arnd [1] it's preferable to change uretprobe
> syscall number to 467 to omit the merge conflict with xattrat syscalls.
>
> Also changing the ABI to 'common' which will ease up the global
> scripts/syscall.tbl management. One consequence is we generate uretprobe
> syscall numbers for ABIs that do not support uretprobe syscall, but the
> syscall still returns -ENOSYS when called in that ABI.
>
> [1] https://lore.kernel.org/lkml/784a34e5-4654-44c9-9c07-f9f4ffd952a0@app.fastmail.com/
>
> Fixes: 190fec72df4a ("uprobe: Wire up uretprobe system call")
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  arch/x86/entry/syscalls/syscall_64.tbl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

sure, why not

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index 6452c2ec469a..dabf1982de6d 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -384,7 +384,7 @@
>  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
> -463    64      uretprobe               sys_uretprobe
> +467    common  uretprobe               sys_uretprobe
>
>  #
>  # Due to a historical design error, certain syscalls are numbered differently
> --
> 2.45.2
>

Re: [PATCH 2/2] selftests/bpf: Change uretprobe syscall number in uprobe_syscall test

[Andrii Nakryiko]

On Fri, Jul 12, 2024 at 6:53 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Fixing the syscall number value.
>
> Fixes: 9e7f74e64ae5 ("selftests/bpf: Add uretprobe syscall call from user space test")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

is this selftest in probes/for-next already? If yes, I'd combine these
two patches to avoid any bisection problems

but either way

Acked-by: Andrii Nakryiko <andrii@kernel.org>


> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> index c8517c8f5313..bd8c75b620c2 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 463
> +#define __NR_uretprobe 467
>  #endif
>
>  __naked unsigned long uretprobe_syscall_call_1(void)
> --
> 2.45.2
>

Re: [PATCH 2/2] selftests/bpf: Change uretprobe syscall number in uprobe_syscall test

[Masami Hiramatsu]

On Fri, 12 Jul 2024 11:27:30 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> On Fri, Jul 12, 2024 at 6:53 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Fixing the syscall number value.
> >
> > Fixes: 9e7f74e64ae5 ("selftests/bpf: Add uretprobe syscall call from user space test")
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> 
> is this selftest in probes/for-next already? If yes, I'd combine these
> two patches to avoid any bisection problems
> 
> but either way
> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>

Thanks, let me pick it to for-next branch.

Thank you,

> 
> 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > index c8517c8f5313..bd8c75b620c2 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 463
> > +#define __NR_uretprobe 467
> >  #endif
> >
> >  __naked unsigned long uretprobe_syscall_call_1(void)
> > --
> > 2.45.2
> >


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 2/2] selftests/bpf: Change uretprobe syscall number in uprobe_syscall test

[Jiri Olsa]

On Mon, Jul 15, 2024 at 02:46:51PM +0900, Masami Hiramatsu wrote:
> On Fri, 12 Jul 2024 11:27:30 -0700
> Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> 
> > On Fri, Jul 12, 2024 at 6:53 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Fixing the syscall number value.
> > >
> > > Fixes: 9e7f74e64ae5 ("selftests/bpf: Add uretprobe syscall call from user space test")
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > >  tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > 
> > is this selftest in probes/for-next already? If yes, I'd combine these
> > two patches to avoid any bisection problems

yes it's all there.. I don't mind squashing it, I just did not want
to combine kernel and user space parts.. up to Masami I guess

> > 
> > but either way
> > 
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
> Thanks, let me pick it to for-next branch.

thanks,
jirka

> 
> Thank you,
> 
> > 
> > 
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > > index c8517c8f5313..bd8c75b620c2 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 463
> > > +#define __NR_uretprobe 467
> > >  #endif
> > >
> > >  __naked unsigned long uretprobe_syscall_call_1(void)
> > > --
> > > 2.45.2
> > >
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 1/2] uprobe: Change uretprobe syscall scope and number

[Dmitry V. Levin]

On Fri, Jul 12, 2024 at 03:52:27PM +0200, Jiri Olsa wrote:
> After discussing with Arnd [1] it's preferable to change uretprobe
> syscall number to 467 to omit the merge conflict with xattrat syscalls.
> 
> Also changing the ABI to 'common' which will ease up the global
> scripts/syscall.tbl management. One consequence is we generate uretprobe
> syscall numbers for ABIs that do not support uretprobe syscall, but the
> syscall still returns -ENOSYS when called in that ABI.
> 
> [1] https://lore.kernel.org/lkml/784a34e5-4654-44c9-9c07-f9f4ffd952a0@app.fastmail.com/
> 
> Fixes: 190fec72df4a ("uprobe: Wire up uretprobe system call")
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  arch/x86/entry/syscalls/syscall_64.tbl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index 6452c2ec469a..dabf1982de6d 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -384,7 +384,7 @@
>  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
> -463	64	uretprobe		sys_uretprobe
> +467	common	uretprobe		sys_uretprobe
>  
>  #
>  # Due to a historical design error, certain syscalls are numbered differently

Isn't include/uapi/asm-generic/unistd.h expected to be updated as well?
As of mainline commit v6.10-12246-g786c8248dbd3, it still contains

#define __NR_uretprobe 463


-- 
ldv

Re: [PATCH 1/2] uprobe: Change uretprobe syscall scope and number

[Arnd Bergmann]

On Wed, Jul 24, 2024, at 09:46, Dmitry V. Levin wrote:
> On Fri, Jul 12, 2024 at 03:52:27PM +0200, Jiri Olsa wrote:
>> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
>> index 6452c2ec469a..dabf1982de6d 100644
>> --- a/arch/x86/entry/syscalls/syscall_64.tbl
>> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
>> @@ -384,7 +384,7 @@
>>  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
>> -463	64	uretprobe		sys_uretprobe
>> +467	common	uretprobe		sys_uretprobe
>>  
>>  #
>>  # Due to a historical design error, certain syscalls are numbered differently
>
> Isn't include/uapi/asm-generic/unistd.h expected to be updated as well?
> As of mainline commit v6.10-12246-g786c8248dbd3, it still contains
>
> #define __NR_uretprobe 463

The file is currently unused and replaced with scripts/syscall.tbl,
my plan was to remove the old file in the 6.12 syscall cleanups.

The number in scripts/syscall.tbl is now 467, so its users (arc,
arm64, csky, hegagon, loongarch, nios2 openrisc and riscv) have
the same number as on x86.
However, the corresponding change did not make it into the
other syscall.tbl files (alpha, arm, m68k, microblaze, parisc,
powerpc, s390, sh, sparc and xtensa), which is rather
inconsistent.

I think we should definitely make all non-x86 architectures
behave the same way, either with or without an entry for
uretprobe. There are three ways do do this:

a) remove it from both include/uapi/asm/unistd.h and
   scripts/syscall.tbl, and change the x86-64 system call
   to a private number such as 335

b) remove it from both include/uapi/asm/unistd.h and
   scripts/syscall.tbl, but leave the number at 467

c) add the syscall to all other architectures for
   consistency, but continue to have it return -ENOSYS.

From Linus' earlier comments, I would guess that a) would
be the least bad of those. I'm also unsure about the status
of the xattrat patches, which were the reason for
changing uretprobe from 463 to 467. Those patches are still
not merged either, and disappeared from linux-next between
Friday and Monday.

      Arnd