Re: [PATCH] samples: bpf: fix tracex2 due to empty sys_write count argument

[Yonghong Song <yhs@fb.com>]

On 12/10/21 3:19 AM, Daniel T. Lee wrote:
> Currently from syscall entry, argument can't be fetched correctly as a
> result of register cleanup.
> 
>      commit 6b8cf5cc9965 ("x86/entry/64/compat: Clear registers for compat syscalls, to reduce speculation attack surface")
> 
> For example in upper commit, registers are cleaned prior to syscall.
> To be more specific, sys_write syscall has count size as a third argument.
> But this can't be fetched from __x64_sys_enter/__s390x_sys_enter due to
> register cleanup. (e.g. [x86] xorl %r8d, %r8d / [s390x] xgr %r7, %r7)

is this the real reason? Did you build 32-bit user space application?
Note that the above commit is for compat syscalls.

> 
> This commit fix this problem by modifying the trace event to ksys_write
> instead of sys_write syscall entry.
> 
>      # Wrong example of 'write()' syscall argument fetching
>      # ./tracex2
>      ...
>      pid 50909 cmd dd uid 0
>             syscall write() stats
>       byte_size       : count     distribution
>         1 -> 1        : 4968837  |************************************* |
> 
>      # Successful example of 'write()' syscall argument fetching
>      # (dd's write bytes at a time defaults to 512)
>      # ./tracex2
>      ...
>      pid 3095 cmd dd uid 0
>             syscall write() stats
>       byte_size       : count     distribution
>      ...
>       256 -> 511      : 0        |                                      |
>       512 -> 1023     : 4968844  |************************************* |
> 
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> ---
>   samples/bpf/tracex2_kern.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
> index 5bc696bac27d..96dff3bea227 100644
> --- a/samples/bpf/tracex2_kern.c
> +++ b/samples/bpf/tracex2_kern.c
> @@ -78,7 +78,7 @@ struct {
>   	__uint(max_entries, 1024);
>   } my_hist_map SEC(".maps");
>   
> -SEC("kprobe/" SYSCALL(sys_write))
> +SEC("kprobe/ksys_write")
>   int bpf_prog3(struct pt_regs *ctx)
>   {
>   	long write_size = PT_REGS_PARM3(ctx);

I think the real reason of the failure is due to SYSCALL_WRAPPER.
Please take a look at test_probe_write_user_kern.c.

The issue with ksys_write() is that it can easily be inlined. For 
example, the source code,

ssize_t ksys_write(unsigned int fd, const char __user *buf, size_t count)
{
	...
}

SYSCALL_DEFINE3(write, unsigned int, fd, const char __user *, buf,
                 size_t, count)
{
         return ksys_write(fd, buf, count);
}

In my 5.12 kernel, I have
ffffffff81053b00 <__x64_sys_write>:
ffffffff81053b00: 0f 1f 44 00 00        nopl    (%rax,%rax)
ffffffff81053b05: 41 57                 pushq   %r15
ffffffff81053b07: 41 56                 pushq   %r14
ffffffff81053b09: 41 55                 pushq   %r13
ffffffff81053b0b: 41 54                 pushq   %r12
ffffffff81053b0d: 53                    pushq   %rbx
ffffffff81053b0e: 48 83 ec 10           subq    $16, %rsp
ffffffff81053b12: 65 48 8b 04 25 28 00 00 00    movq    %gs:40, %rax
ffffffff81053b1b: 48 89 44 24 08        movq    %rax, 8(%rsp)
ffffffff81053b20: 8b 47 70              movl    112(%rdi), %eax
ffffffff81053b23: 4c 8b 7f 60           movq    96(%rdi), %r15
ffffffff81053b27: 4c 8b 67 68           movq    104(%rdi), %r12
ffffffff81053b2b: 89 c7                 movl    %eax, %edi
ffffffff81053b2d: e8 6e a3 00 00        callq   0xffffffff8105dea0 
<__fdget_pos>
...

The ksys_write() is inlined.