2022-04-03 18:39:02

by Song Chen

[permalink] [raw]
Subject: [PATCH] sample: bpf: syscall_tp_kern: add dfd before filename

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 <[email protected]>
---
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;
--
2.25.1


2022-04-05 01:06:32

by Andrii Nakryiko

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

On Thu, Mar 31, 2022 at 6:34 PM Song Chen <[email protected]> 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 <[email protected]>
> ---
> 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
>

2022-04-06 15:14:56

by Song Chen

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

Hi,

在 2022/4/5 06:17, Andrii Nakryiko 写道:
> On Thu, Mar 31, 2022 at 6:34 PM Song Chen <[email protected]> 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 <[email protected]>
>> ---
>> 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
>>
>