Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <1648777246-21352-1-git-send-email-chensong_2000@189.cn>
In-Reply-To: <1648777246-21352-1-git-send-email-chensong_2000@189.cn>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Mon, 4 Apr 2022 15:17:04 -0700
Message-ID: <CAEf4Bzbo=DU_LqJ=sXgawP9-O4VR84jDdhuf9Xto=T3LSsrySA@mail.gmail.com>
Subject: Re: [PATCH] sample: bpf: syscall_tp_kern: add dfd before filename
To:     Song Chen <chensong_2000@189.cn>
Cc:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>, Martin Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        john fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Mar 31, 2022 at 6:34 PM Song Chen <chensong_2000@189.cn> wrote:
>
> When i was writing my eBPF program, i copied some pieces of code from
> syscall_tp, syscall_tp_kern only records how many files are opened, but
> mine needs to print file name.I reused struct syscalls_enter_open_args,
> which is defined as:
>
> struct syscalls_enter_open_args {
>         unsigned long long unused;
>         long syscall_nr;
>         long filename_ptr;
>         long flags;
>         long mode;
> };
>
> I tried to use filename_ptr, but it's not the pointer of filename, flags
> turns out to be the pointer I'm looking for, there might be something
> missed in the struct.
>
> I read the ftrace log, found the missed one is dfd, which is supposed to be
> placed in between syscall_nr and filename_ptr.
>
> Actually syscall_tp has nothing to do with dfd, it can run anyway without
> it, but it's better to have it to make it a better eBPF sample, especially
> to new eBPF programmers, then i fixed it.
>
> Signed-off-by: Song Chen <chensong_2000@189.cn>
> ---
>  samples/bpf/syscall_tp_kern.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/samples/bpf/syscall_tp_kern.c b/samples/bpf/syscall_tp_kern.c
> index 50231c2eff9c..e4ac818aee57 100644
> --- a/samples/bpf/syscall_tp_kern.c
> +++ b/samples/bpf/syscall_tp_kern.c
> @@ -7,6 +7,7 @@
>  struct syscalls_enter_open_args {
>         unsigned long long unused;
>         long syscall_nr;
> +       long dfd_ptr;
>         long filename_ptr;
>         long flags;
>         long mode;

Here's what I see on latest bpf-next:

# cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_open/format
name: sys_enter_open
ID: 613
format:
        field:unsigned short common_type;       offset:0;
size:2; signed:0;
        field:unsigned char common_flags;       offset:2;
size:1; signed:0;
        field:unsigned char common_preempt_count;       offset:3;
 size:1; signed:0;
        field:int common_pid;   offset:4;       size:4; signed:1;

        field:int __syscall_nr; offset:8;       size:4; signed:1;
        field:const char * filename;    offset:16;      size:8; signed:0;
        field:int flags;        offset:24;      size:8; signed:0;
        field:umode_t mode;     offset:32;      size:8; signed:0;

This layout doesn't correspond either to before or after state of
syscalls_enter_open_args. Not sure what's going on, but it doesn't
seem that struct syscalls_enter_open_args is correct anyways.


> --
> 2.25.1
>