Re: [PATCH] sample: bpf: syscall_tp_kern: add dfd before filename

[Song Chen]

Hi,

在 2022/4/5 06:17, Andrii Nakryiko 写道:
> On Thu, Mar 31, 2022 at 6:34 PM Song Chen <chensong_2000@xxxxxx> 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@xxxxxx>
> > ---
> >   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.

sys_enter_open is not enabled in my system somehow and i haven't figured 
out why, then i used sys_enter_openat, whose format is:

name: sys_enter_openat
ID: 647
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:int dfd;	offset:16;	size:8;	signed:0;
	field:const char * filename;	offset:24;	size:8;	signed:0;
	field:int flags;	offset:32;	size:8;	signed:0;
	field:umode_t mode;	offset:40;	size:8;	signed:0;

print fmt: "dfd: 0x%08lx, filename: 0x%08lx, flags: 0x%08lx, mode: 
0x%08lx", ((unsigned long)(REC->dfd)), ((unsigned long)(REC->filename)), 
((unsigned long)(REC->flags)), ((unsigned long)(REC->mode))

I think in this case syscalls_enter_open_args is not applicable for 
sys_enter_openat, how about we introduce a new struct specific for 
sys_enter_openat with dfd in it?

/Song

> > --
> > 2.25.1