linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/1] Remove forget_syscall() from start_thread_common()
@ 2022-06-08 16:24 Francis Laniel
  2022-06-08 16:24 ` [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread Francis Laniel
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Francis Laniel @ 2022-06-08 16:24 UTC (permalink / raw)
  To: linux-trace-devel
  Cc: James Morse, Daniel Kiss, Francis Laniel, Catalin Marinas,
	Will Deacon, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Mark Brown, Peter Collingbourne, Kees Cook, Mark Rutland,
	Christophe Leroy, linux-arm-kernel, linux-kernel, linux-riscv

Hi.


First, I hope you are fine and the same for your relatives.

With this contribution, I enabled using syscalls:sys_exit_execve and
syscalls:sys_exit_execveat as tracepoints on arm64.
Indeed, before this contribution, the above tracepoint would not print their
information as syscall number was set to -1 by calling forget_syscall().

I tested it by compiling a kernel for arm64 and running it within a VM:
# Perf was compiled with linux kernel source.
root@vm-arm64:~# perf record -ag -e 'syscalls:sys_exit_execve' -e 'syscalls:sys_enter_execve' &
[1] 263
root@vm-arm64:~# ls
perf.data  share
root@vm-arm64:~# fg
perf record -ag -e 'syscalls:sys_exit_execve' -e 'syscalls:sys_enter_execve'
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.061 MB perf.data (2 samples) ]
root@vm-arm64:~# perf script
bash   264 [000]    66.220187: syscalls:sys_enter_execve: filename: 0xaaab05d9d
...
# Below line does not appear without this patch.
ls   264 [000]    66.226848:  syscalls:sys_exit_execve: 0x0
...

Forgetting the syscall number before starting a new thread was confirmed to be
a bug [1].
Particularly, the following architectures do not forget the syscall number
before starting a new thread:
* arm (32 bits) EABI: start_thread() sets r7 to previous r7 for ELF FDPIC  and
to 0 for other binfmts [2].
* arm (32 bits) OABI: syscall number is set to -1 if
ptrace_report_syscall_entry() failed [3].
* mips: start_thread() does not modify current_thread_info->syscall which is
taken directly from v0 [4, 5].
* riscv: start_thread() does not modify a7 [6].
* x86_64: start_thread_common() does not touch orig_ax which seems to contain
the syscall number [7].

If you see any way to improve this contribution, feel free to share!

Change since:
v1:
 * Remove call to forget_syscall() and store previous syscall number in
 regs->syscallno unconditionnaly.

Francis Laniel (1):
  arm64: Do not forget syscall when starting a new thread.

 arch/arm64/include/asm/processor.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


Best regards and thank you in advance.
---
[1] https://lore.kernel.org/linux-arm-kernel/YoT1iLPEbteRTQGZ@arm.com/
[2] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/arm/include/asm/
processor.h#L52
[3] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/arm/kernel/
ptrace.c#L847
[4] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/mips/kernel/
process.c#L52
[5] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/mips/kernel/
scall64-n64.S#L85
[6] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/riscv/kernel/
process.c#L87
[7] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/x86/kernel/
process_64.c#L505
-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread.
  2022-06-08 16:24 [PATCH v2 0/1] Remove forget_syscall() from start_thread_common() Francis Laniel
@ 2022-06-08 16:24 ` Francis Laniel
  2022-06-28 13:58   ` Will Deacon
  2022-06-23 14:09 ` [PATCH v2 0/1] Remove forget_syscall() from start_thread_common() Francis Laniel
  2022-07-01 15:41 ` Will Deacon
  2 siblings, 1 reply; 10+ messages in thread
From: Francis Laniel @ 2022-06-08 16:24 UTC (permalink / raw)
  To: linux-trace-devel
  Cc: James Morse, Daniel Kiss, Francis Laniel, Catalin Marinas,
	Will Deacon, Mark Brown, Peter Collingbourne, Kees Cook,
	Mark Rutland, Christophe Leroy, linux-arm-kernel, linux-kernel

This patch enables exeve*() to be traced with syscalls:sys_exit_execve
tracepoint.
Previous to it, by calling forget_syscall(), this tracepoint would not
print its information as syscall is -1.
So, this patch removes call to forget_syscall() and set regs->syscallno
to its previous value.

Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
---
 arch/arm64/include/asm/processor.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 9e58749db21d..86eb0bfe3b38 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -272,8 +272,9 @@ void tls_preserve_current_state(void);
 
 static inline void start_thread_common(struct pt_regs *regs, unsigned long pc)
 {
+	s32 previous_syscall = regs->syscallno;
 	memset(regs, 0, sizeof(*regs));
-	forget_syscall(regs);
+	regs->syscallno = previous_syscall;
 	regs->pc = pc;
 
 	if (system_uses_irq_prio_masking())
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/1] Remove forget_syscall() from start_thread_common()
  2022-06-08 16:24 [PATCH v2 0/1] Remove forget_syscall() from start_thread_common() Francis Laniel
  2022-06-08 16:24 ` [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread Francis Laniel
@ 2022-06-23 14:09 ` Francis Laniel
  2022-07-01 15:41 ` Will Deacon
  2 siblings, 0 replies; 10+ messages in thread
From: Francis Laniel @ 2022-06-23 14:09 UTC (permalink / raw)
  To: linux-trace-devel
  Cc: James Morse, Daniel Kiss, Catalin Marinas, Will Deacon,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Mark Brown,
	Peter Collingbourne, Kees Cook, Mark Rutland, Christophe Leroy,
	linux-arm-kernel, linux-kernel, linux-riscv

Hi.

Le mercredi 8 juin 2022, 18:24:45 CEST Francis Laniel a écrit :
> Hi.
> 
> 
> First, I hope you are fine and the same for your relatives.
> 
> With this contribution, I enabled using syscalls:sys_exit_execve and
> syscalls:sys_exit_execveat as tracepoints on arm64.
> Indeed, before this contribution, the above tracepoint would not print their
> information as syscall number was set to -1 by calling forget_syscall().
> 
> I tested it by compiling a kernel for arm64 and running it within a VM:
> # Perf was compiled with linux kernel source.
> root@vm-arm64:~# perf record -ag -e 'syscalls:sys_exit_execve' -e
> 'syscalls:sys_enter_execve' & [1] 263
> root@vm-arm64:~# ls
> perf.data  share
> root@vm-arm64:~# fg
> perf record -ag -e 'syscalls:sys_exit_execve' -e 'syscalls:sys_enter_execve'
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.061 MB perf.data (2 samples) ]
> root@vm-arm64:~# perf script
> bash   264 [000]    66.220187: syscalls:sys_enter_execve: filename:
> 0xaaab05d9d ...
> # Below line does not appear without this patch.
> ls   264 [000]    66.226848:  syscalls:sys_exit_execve: 0x0
> ...
> 
> Forgetting the syscall number before starting a new thread was confirmed to
> be a bug [1].
> Particularly, the following architectures do not forget the syscall number
> before starting a new thread:
> * arm (32 bits) EABI: start_thread() sets r7 to previous r7 for ELF FDPIC 
> and to 0 for other binfmts [2].
> * arm (32 bits) OABI: syscall number is set to -1 if
> ptrace_report_syscall_entry() failed [3].
> * mips: start_thread() does not modify current_thread_info->syscall which is
> taken directly from v0 [4, 5].
> * riscv: start_thread() does not modify a7 [6].
> * x86_64: start_thread_common() does not touch orig_ax which seems to
> contain the syscall number [7].
> 
> If you see any way to improve this contribution, feel free to share!
> 
> Change since:
> v1:
>  * Remove call to forget_syscall() and store previous syscall number in
>  regs->syscallno unconditionnaly.
> 
> Francis Laniel (1):
>   arm64: Do not forget syscall when starting a new thread.
> 
>  arch/arm64/include/asm/processor.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> 
> Best regards and thank you in advance.
> ---
> [1] https://lore.kernel.org/linux-arm-kernel/YoT1iLPEbteRTQGZ@arm.com/
> [2] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/arm/include/asm/
> processor.h#L52
> [3] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/arm/kernel/
> ptrace.c#L847
> [4] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/mips/kernel/
> process.c#L52
> [5] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/mips/kernel/
> scall64-n64.S#L85
> [6] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/riscv/kernel/
> process.c#L87
> [7] https://elixir.bootlin.com/linux/v5.18-rc6/source/arch/x86/kernel/
> process_64.c#L505

Can someone please take a quick look at this patch?


Best regards and thank you in advance.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread.
  2022-06-08 16:24 ` [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread Francis Laniel
@ 2022-06-28 13:58   ` Will Deacon
  2022-06-28 19:26     ` Francis Laniel
  0 siblings, 1 reply; 10+ messages in thread
From: Will Deacon @ 2022-06-28 13:58 UTC (permalink / raw)
  To: Francis Laniel
  Cc: linux-trace-devel, James Morse, Daniel Kiss, Catalin Marinas,
	Mark Brown, Peter Collingbourne, Kees Cook, Mark Rutland,
	Christophe Leroy, linux-arm-kernel, linux-kernel

On Wed, Jun 08, 2022 at 05:24:46PM +0100, Francis Laniel wrote:
> This patch enables exeve*() to be traced with syscalls:sys_exit_execve
> tracepoint.
> Previous to it, by calling forget_syscall(), this tracepoint would not
> print its information as syscall is -1.
> So, this patch removes call to forget_syscall() and set regs->syscallno
> to its previous value.
> 
> Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> ---
>  arch/arm64/include/asm/processor.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index 9e58749db21d..86eb0bfe3b38 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -272,8 +272,9 @@ void tls_preserve_current_state(void);
>  
>  static inline void start_thread_common(struct pt_regs *regs, unsigned long pc)
>  {
> +	s32 previous_syscall = regs->syscallno;
>  	memset(regs, 0, sizeof(*regs));
> -	forget_syscall(regs);
> +	regs->syscallno = previous_syscall;

I'm still unsure about this. Even if we preserve the syscall number here,
won't all the arguments be reported as 0?

I also looked quickly at the 32-bit arch/arm/ code and it looks like the
same behaviour exists there (module CONFIG_BINFMT_ELF_FDPIC).

Will

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread.
  2022-06-28 13:58   ` Will Deacon
@ 2022-06-28 19:26     ` Francis Laniel
  2022-06-30 17:16       ` Francis Laniel
  0 siblings, 1 reply; 10+ messages in thread
From: Francis Laniel @ 2022-06-28 19:26 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-trace-devel, James Morse, Daniel Kiss, Catalin Marinas,
	Mark Brown, Peter Collingbourne, Kees Cook, Mark Rutland,
	Christophe Leroy, linux-arm-kernel, linux-kernel

Hi.

Le mardi 28 juin 2022, 15:58:35 CEST Will Deacon a écrit :
> On Wed, Jun 08, 2022 at 05:24:46PM +0100, Francis Laniel wrote:
> > This patch enables exeve*() to be traced with syscalls:sys_exit_execve
> > tracepoint.
> > Previous to it, by calling forget_syscall(), this tracepoint would not
> > print its information as syscall is -1.
> > So, this patch removes call to forget_syscall() and set regs->syscallno
> > to its previous value.
> > 
> > Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> > ---
> > 
> >  arch/arm64/include/asm/processor.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/include/asm/processor.h
> > b/arch/arm64/include/asm/processor.h index 9e58749db21d..86eb0bfe3b38
> > 100644
> > --- a/arch/arm64/include/asm/processor.h
> > +++ b/arch/arm64/include/asm/processor.h
> > @@ -272,8 +272,9 @@ void tls_preserve_current_state(void);
> > 
> >  static inline void start_thread_common(struct pt_regs *regs, unsigned
> >  long pc) {
> > 
> > +	s32 previous_syscall = regs->syscallno;
> > 
> >  	memset(regs, 0, sizeof(*regs));
> > 
> > -	forget_syscall(regs);
> > +	regs->syscallno = previous_syscall;
> 
> I'm still unsure about this. Even if we preserve the syscall number here,
> won't all the arguments be reported as 0?

I am not really sure what you meant about arguments, can you please precise 
between command line arguments (ls -al) and syscall arguments (argp, envp, 
etc.)?
Indeed, if my understanding is correct syscall arguments are showed by 
sys_enter_* while sys_exit_* only reports the syscall return code.

Regarding the return code I think the value is correct as it is used in 
syscall_trace_exit() but set in invoke_syscall() after the syscall finishes [1, 
2].
The comparison of arm64 and amd64 output also shows no difference:
# amd64
ls 435739 [002] 24689.292479:  syscalls:sys_exit_execve: 0x0
            7fc43732e100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)
# arm64
ls   266 [000]    34.708444:  syscalls:sys_exit_execve: 0x0
                    1140 [unknown] (/usr/lib/aarch64-linux-gnu/ld-2.31.so)
 
> I also looked quickly at the 32-bit arch/arm/ code and it looks like the
> same behaviour exists there (module CONFIG_BINFMT_ELF_FDPIC).

I can try to fix it for this architecture too.
Can you please point me the part of the code which shows the same behavior?
 
> Will


Best regards.
---
[1] https://elixir.bootlin.com/linux/v5.18/source/arch/arm64/kernel/
ptrace.c#L1868
[2] https://elixir.bootlin.com/linux/v5.18/source/arch/arm64/kernel/
syscall.c#L57



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread.
  2022-06-28 19:26     ` Francis Laniel
@ 2022-06-30 17:16       ` Francis Laniel
  2022-07-01 11:36         ` Will Deacon
  0 siblings, 1 reply; 10+ messages in thread
From: Francis Laniel @ 2022-06-30 17:16 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-trace-devel, James Morse, Daniel Kiss, Catalin Marinas,
	Mark Brown, Peter Collingbourne, Kees Cook, Mark Rutland,
	Christophe Leroy, linux-arm-kernel, linux-kernel

Hi.

Le mardi 28 juin 2022, 21:26:32 CEST Francis Laniel a écrit :
> Hi.
> 
> Le mardi 28 juin 2022, 15:58:35 CEST Will Deacon a écrit :
> > On Wed, Jun 08, 2022 at 05:24:46PM +0100, Francis Laniel wrote:
> > > This patch enables exeve*() to be traced with syscalls:sys_exit_execve
> > > tracepoint.
> > > Previous to it, by calling forget_syscall(), this tracepoint would not
> > > print its information as syscall is -1.
> > > So, this patch removes call to forget_syscall() and set regs->syscallno
> > > to its previous value.
> > > 
> > > Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> > > ---
> > > 
> > >  arch/arm64/include/asm/processor.h | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/include/asm/processor.h
> > > b/arch/arm64/include/asm/processor.h index 9e58749db21d..86eb0bfe3b38
> > > 100644
> > > --- a/arch/arm64/include/asm/processor.h
> > > +++ b/arch/arm64/include/asm/processor.h
> > > @@ -272,8 +272,9 @@ void tls_preserve_current_state(void);
> > > 
> > >  static inline void start_thread_common(struct pt_regs *regs, unsigned
> > >  long pc) {
> > > 
> > > +	s32 previous_syscall = regs->syscallno;
> > > 
> > >  	memset(regs, 0, sizeof(*regs));
> > > 
> > > -	forget_syscall(regs);
> > > +	regs->syscallno = previous_syscall;
> > 
> > I'm still unsure about this. Even if we preserve the syscall number here,
> > won't all the arguments be reported as 0?
> 
> I am not really sure what you meant about arguments, can you please precise
> between command line arguments (ls -al) and syscall arguments (argp, envp,
> etc.)?
> Indeed, if my understanding is correct syscall arguments are showed by
> sys_enter_* while sys_exit_* only reports the syscall return code.
> 
> Regarding the return code I think the value is correct as it is used in
> syscall_trace_exit() but set in invoke_syscall() after the syscall finishes
> [1, 2].
> The comparison of arm64 and amd64 output also shows no difference:
> # amd64
> ls 435739 [002] 24689.292479:  syscalls:sys_exit_execve: 0x0
>             7fc43732e100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)
> # arm64
> ls   266 [000]    34.708444:  syscalls:sys_exit_execve: 0x0
>                     1140 [unknown] (/usr/lib/aarch64-linux-gnu/ld-2.31.so)
> 
> > I also looked quickly at the 32-bit arch/arm/ code and it looks like the
> > same behaviour exists there (module CONFIG_BINFMT_ELF_FDPIC).
> 

I tested arm32 and it is not affected (even though I did not have 
CONFIG_BINFMT_ELF_FDPIC set).
Here is ftrace output for arm64 without this patch:
            bash-316     [000] .....    72.167342: sys_execve(filename: 
aaaaf9bbcd30, argv: aaaaf9bb54f0, envp: aaaaf9a7d9b0)
Here is the output for arm64 with this patch:
             cat-313     [000] .....   417.926073: sys_execve(filename: 
aaaaee7ce9f0, argv: aaaaee7833a0, envp: aaaaee6a69b0)
             cat-313     [000] .....   417.939619: sys_execve -> 0x0
And here is output for arm32:
             cat-254     [000] .....   127.804128: sys_execve(filename: 5bff18, 
argv: 53bb00, envp: 5543a8)
             cat-254     [000] .....   127.809142: sys_execve -> 0x0
From the above, the arm32 output seems correct even though:
# CONFIG_BINFMT_ELF_FDPIC is not set

After some debugging, I realized that arm32 syscall_get_nr() uses abi_syscall 
to get the syscall number and not a register (I guess abi_syscall was set to 
value of R7 before) [1].
So the fact that regs->uregs are memset'ed to 0 is not a problem.

> 
> Best regards.
> ---
> [1] https://elixir.bootlin.com/linux/v5.18/source/arch/arm64/kernel/
> ptrace.c#L1868
> [2] https://elixir.bootlin.com/linux/v5.18/source/arch/arm64/kernel/
> syscall.c#L57


Best regards.
---
[1] https://elixir.bootlin.com/linux/v5.18.8/source/arch/arm/include/asm/
syscall.h#L22



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread.
  2022-06-30 17:16       ` Francis Laniel
@ 2022-07-01 11:36         ` Will Deacon
  2022-07-01 12:10           ` Francis Laniel
  0 siblings, 1 reply; 10+ messages in thread
From: Will Deacon @ 2022-07-01 11:36 UTC (permalink / raw)
  To: Francis Laniel
  Cc: linux-trace-devel, James Morse, Daniel Kiss, Catalin Marinas,
	Mark Brown, Peter Collingbourne, Kees Cook, Mark Rutland,
	Christophe Leroy, linux-arm-kernel, linux-kernel

On Thu, Jun 30, 2022 at 07:16:44PM +0200, Francis Laniel wrote:
> Hi.
> 
> Le mardi 28 juin 2022, 21:26:32 CEST Francis Laniel a écrit :
> > Hi.
> > 
> > Le mardi 28 juin 2022, 15:58:35 CEST Will Deacon a écrit :
> > > On Wed, Jun 08, 2022 at 05:24:46PM +0100, Francis Laniel wrote:
> > > > This patch enables exeve*() to be traced with syscalls:sys_exit_execve
> > > > tracepoint.
> > > > Previous to it, by calling forget_syscall(), this tracepoint would not
> > > > print its information as syscall is -1.
> > > > So, this patch removes call to forget_syscall() and set regs->syscallno
> > > > to its previous value.
> > > > 
> > > > Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> > > > ---
> > > > 
> > > >  arch/arm64/include/asm/processor.h | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/arm64/include/asm/processor.h
> > > > b/arch/arm64/include/asm/processor.h index 9e58749db21d..86eb0bfe3b38
> > > > 100644
> > > > --- a/arch/arm64/include/asm/processor.h
> > > > +++ b/arch/arm64/include/asm/processor.h
> > > > @@ -272,8 +272,9 @@ void tls_preserve_current_state(void);
> > > > 
> > > >  static inline void start_thread_common(struct pt_regs *regs, unsigned
> > > >  long pc) {
> > > > 
> > > > +	s32 previous_syscall = regs->syscallno;
> > > > 
> > > >  	memset(regs, 0, sizeof(*regs));
> > > > 
> > > > -	forget_syscall(regs);
> > > > +	regs->syscallno = previous_syscall;
> > > 
> > > I'm still unsure about this. Even if we preserve the syscall number here,
> > > won't all the arguments be reported as 0?
> > 
> > I am not really sure what you meant about arguments, can you please precise
> > between command line arguments (ls -al) and syscall arguments (argp, envp,
> > etc.)?
> > Indeed, if my understanding is correct syscall arguments are showed by
> > sys_enter_* while sys_exit_* only reports the syscall return code.
> > 
> > Regarding the return code I think the value is correct as it is used in
> > syscall_trace_exit() but set in invoke_syscall() after the syscall finishes
> > [1, 2].
> > The comparison of arm64 and amd64 output also shows no difference:
> > # amd64
> > ls 435739 [002] 24689.292479:  syscalls:sys_exit_execve: 0x0
> >             7fc43732e100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)
> > # arm64
> > ls   266 [000]    34.708444:  syscalls:sys_exit_execve: 0x0
> >                     1140 [unknown] (/usr/lib/aarch64-linux-gnu/ld-2.31.so)
> > 
> > > I also looked quickly at the 32-bit arch/arm/ code and it looks like the
> > > same behaviour exists there (module CONFIG_BINFMT_ELF_FDPIC).
> > 
> 
> I tested arm32 and it is not affected (even though I did not have 
> CONFIG_BINFMT_ELF_FDPIC set).
> Here is ftrace output for arm64 without this patch:
>             bash-316     [000] .....    72.167342: sys_execve(filename: 
> aaaaf9bbcd30, argv: aaaaf9bb54f0, envp: aaaaf9a7d9b0)
> Here is the output for arm64 with this patch:
>              cat-313     [000] .....   417.926073: sys_execve(filename: 
> aaaaee7ce9f0, argv: aaaaee7833a0, envp: aaaaee6a69b0)
>              cat-313     [000] .....   417.939619: sys_execve -> 0x0
> And here is output for arm32:
>              cat-254     [000] .....   127.804128: sys_execve(filename: 5bff18, 
> argv: 53bb00, envp: 5543a8)
>              cat-254     [000] .....   127.809142: sys_execve -> 0x0
> From the above, the arm32 output seems correct even though:
> # CONFIG_BINFMT_ELF_FDPIC is not set
> 
> After some debugging, I realized that arm32 syscall_get_nr() uses abi_syscall 
> to get the syscall number and not a register (I guess abi_syscall was set to 
> value of R7 before) [1].
> So the fact that regs->uregs are memset'ed to 0 is not a problem.

Thanks for confirming this, I'll go ahead and queue your patch and let's
hope nothing breaks :)

Will

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread.
  2022-07-01 11:36         ` Will Deacon
@ 2022-07-01 12:10           ` Francis Laniel
  0 siblings, 0 replies; 10+ messages in thread
From: Francis Laniel @ 2022-07-01 12:10 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-trace-devel, James Morse, Daniel Kiss, Catalin Marinas,
	Mark Brown, Peter Collingbourne, Kees Cook, Mark Rutland,
	Christophe Leroy, linux-arm-kernel, linux-kernel

Hi.


Le vendredi 1 juillet 2022, 13:36:32 CEST Will Deacon a écrit :
> On Thu, Jun 30, 2022 at 07:16:44PM +0200, Francis Laniel wrote:
> > Hi.
> > 
> > Le mardi 28 juin 2022, 21:26:32 CEST Francis Laniel a écrit :
> > > Hi.
> > > 
> > > Le mardi 28 juin 2022, 15:58:35 CEST Will Deacon a écrit :
> > > > On Wed, Jun 08, 2022 at 05:24:46PM +0100, Francis Laniel wrote:
> > > > > This patch enables exeve*() to be traced with
> > > > > syscalls:sys_exit_execve
> > > > > tracepoint.
> > > > > Previous to it, by calling forget_syscall(), this tracepoint would
> > > > > not
> > > > > print its information as syscall is -1.
> > > > > So, this patch removes call to forget_syscall() and set
> > > > > regs->syscallno
> > > > > to its previous value.
> > > > > 
> > > > > Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
> > > > > ---
> > > > > 
> > > > >  arch/arm64/include/asm/processor.h | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/arch/arm64/include/asm/processor.h
> > > > > b/arch/arm64/include/asm/processor.h index
> > > > > 9e58749db21d..86eb0bfe3b38
> > > > > 100644
> > > > > --- a/arch/arm64/include/asm/processor.h
> > > > > +++ b/arch/arm64/include/asm/processor.h
> > > > > @@ -272,8 +272,9 @@ void tls_preserve_current_state(void);
> > > > > 
> > > > >  static inline void start_thread_common(struct pt_regs *regs,
> > > > >  unsigned
> > > > >  long pc) {
> > > > > 
> > > > > +	s32 previous_syscall = regs->syscallno;
> > > > > 
> > > > >  	memset(regs, 0, sizeof(*regs));
> > > > > 
> > > > > -	forget_syscall(regs);
> > > > > +	regs->syscallno = previous_syscall;
> > > > 
> > > > I'm still unsure about this. Even if we preserve the syscall number
> > > > here,
> > > > won't all the arguments be reported as 0?
> > > 
> > > I am not really sure what you meant about arguments, can you please
> > > precise
> > > between command line arguments (ls -al) and syscall arguments (argp,
> > > envp,
> > > etc.)?
> > > Indeed, if my understanding is correct syscall arguments are showed by
> > > sys_enter_* while sys_exit_* only reports the syscall return code.
> > > 
> > > Regarding the return code I think the value is correct as it is used in
> > > syscall_trace_exit() but set in invoke_syscall() after the syscall
> > > finishes
> > > [1, 2].
> > > The comparison of arm64 and amd64 output also shows no difference:
> > > # amd64
> > > ls 435739 [002] 24689.292479:  syscalls:sys_exit_execve: 0x0
> > > 
> > >             7fc43732e100 _start+0x0
> > >             (/usr/lib/x86_64-linux-gnu/ld-2.31.so)
> > > 
> > > # arm64
> > > ls   266 [000]    34.708444:  syscalls:sys_exit_execve: 0x0
> > > 
> > >                     1140 [unknown]
> > >                     (/usr/lib/aarch64-linux-gnu/ld-2.31.so)
> > > > 
> > > > I also looked quickly at the 32-bit arch/arm/ code and it looks like
> > > > the
> > > > same behaviour exists there (module CONFIG_BINFMT_ELF_FDPIC).
> > 
> > I tested arm32 and it is not affected (even though I did not have
> > CONFIG_BINFMT_ELF_FDPIC set).
> > 
> > Here is ftrace output for arm64 without this patch:
> >             bash-316     [000] .....    72.167342: sys_execve(filename:
> > aaaaf9bbcd30, argv: aaaaf9bb54f0, envp: aaaaf9a7d9b0)
> > 
> > Here is the output for arm64 with this patch:
> >              cat-313     [000] .....   417.926073: sys_execve(filename:
> > aaaaee7ce9f0, argv: aaaaee7833a0, envp: aaaaee6a69b0)
> > 
> >              cat-313     [000] .....   417.939619: sys_execve -> 0x0
> > 
> > And here is output for arm32:
> >              cat-254     [000] .....   127.804128: sys_execve(filename:
> >              5bff18,
> > 
> > argv: 53bb00, envp: 5543a8)
> > 
> >              cat-254     [000] .....   127.809142: sys_execve -> 0x0
> > 
> > From the above, the arm32 output seems correct even though:
> > # CONFIG_BINFMT_ELF_FDPIC is not set
> > 
> > After some debugging, I realized that arm32 syscall_get_nr() uses
> > abi_syscall to get the syscall number and not a register (I guess
> > abi_syscall was set to value of R7 before) [1].
> > So the fact that regs->uregs are memset'ed to 0 is not a problem.
> 
> Thanks for confirming this, I'll go ahead and queue your patch and let's
> hope nothing breaks :)

You are welcome!
If there any problem, feel free to ping and I will try to handle them.
 
> Will


Best regards.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/1] Remove forget_syscall() from start_thread_common()
  2022-06-08 16:24 [PATCH v2 0/1] Remove forget_syscall() from start_thread_common() Francis Laniel
  2022-06-08 16:24 ` [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread Francis Laniel
  2022-06-23 14:09 ` [PATCH v2 0/1] Remove forget_syscall() from start_thread_common() Francis Laniel
@ 2022-07-01 15:41 ` Will Deacon
  2022-07-04  8:55   ` Francis Laniel
  2 siblings, 1 reply; 10+ messages in thread
From: Will Deacon @ 2022-07-01 15:41 UTC (permalink / raw)
  To: linux-trace-devel, Francis Laniel
  Cc: catalin.marinas, kernel-team, Will Deacon, Palmer Dabbelt,
	Peter Collingbourne, Mark Brown, Paul Walmsley, Christophe Leroy,
	linux-arm-kernel, Mark Rutland, Kees Cook, Daniel Kiss,
	linux-riscv, linux-kernel, James Morse, Albert Ou

On Wed, 8 Jun 2022 17:24:45 +0100, Francis Laniel wrote:
> First, I hope you are fine and the same for your relatives.
> 
> With this contribution, I enabled using syscalls:sys_exit_execve and
> syscalls:sys_exit_execveat as tracepoints on arm64.
> Indeed, before this contribution, the above tracepoint would not print their
> information as syscall number was set to -1 by calling forget_syscall().
> 
> [...]

Applied to arm64 (for-next/misc), thanks!

[1/1] arm64: Do not forget syscall when starting a new thread.
      https://git.kernel.org/arm64/c/de6921856f99

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/1] Remove forget_syscall() from start_thread_common()
  2022-07-01 15:41 ` Will Deacon
@ 2022-07-04  8:55   ` Francis Laniel
  0 siblings, 0 replies; 10+ messages in thread
From: Francis Laniel @ 2022-07-04  8:55 UTC (permalink / raw)
  To: linux-trace-devel, Will Deacon
  Cc: catalin.marinas, kernel-team, Will Deacon, Palmer Dabbelt,
	Peter Collingbourne, Mark Brown, Paul Walmsley, Christophe Leroy,
	linux-arm-kernel, Mark Rutland, Kees Cook, Daniel Kiss,
	linux-riscv, linux-kernel, James Morse, Albert Ou

Hi.


Le vendredi 1 juillet 2022, 17:41:28 CEST Will Deacon a écrit :
> On Wed, 8 Jun 2022 17:24:45 +0100, Francis Laniel wrote:
> > First, I hope you are fine and the same for your relatives.
> > 
> > With this contribution, I enabled using syscalls:sys_exit_execve and
> > syscalls:sys_exit_execveat as tracepoints on arm64.
> > Indeed, before this contribution, the above tracepoint would not print
> > their information as syscall number was set to -1 by calling
> > forget_syscall().
> > 
> > [...]
> 
> Applied to arm64 (for-next/misc), thanks!

Thank you for the merge!

> [1/1] arm64: Do not forget syscall when starting a new thread.
>       https://git.kernel.org/arm64/c/de6921856f99
> 
> Cheers,


Best regards.



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-07-04  8:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-08 16:24 [PATCH v2 0/1] Remove forget_syscall() from start_thread_common() Francis Laniel
2022-06-08 16:24 ` [PATCH v2 1/1] arm64: Do not forget syscall when starting a new thread Francis Laniel
2022-06-28 13:58   ` Will Deacon
2022-06-28 19:26     ` Francis Laniel
2022-06-30 17:16       ` Francis Laniel
2022-07-01 11:36         ` Will Deacon
2022-07-01 12:10           ` Francis Laniel
2022-06-23 14:09 ` [PATCH v2 0/1] Remove forget_syscall() from start_thread_common() Francis Laniel
2022-07-01 15:41 ` Will Deacon
2022-07-04  8:55   ` Francis Laniel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).