On Fri, Sep 01, 2023 at 04:09:31PM -0700, Song Liu wrote:
> On Thu, Aug 31, 2023 at 8:21 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
> >
> > On 8/31/23 4:11 PM, Jiri Olsa wrote:
> > > Recent commit [1] broken d_path test, because now filp_close is not called
> > > directly from sys_close, but eventually later when the file is finally
> > > released.
> > >
> > > As suggested by Hou Tao we don't need to re-hook the bpf program, but just
> > > instead we can use sys_close_range to trigger filp_close synchronously.
> > >
> > > [1] 021a160abf62 ("fs: use __fput_sync in close(2)")
> > > Suggested-by: Hou Tao <houtao@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> >
> > That did the trick, thanks everyone, applied!
>
> I guess I am a bit late. But how about we use something like the following?
> I like this one better because it tests bpf_d_path() from retval at fexit.
right, that would have been an option as well
>
> Thanks,
> Song
>
>
>
>
> diff --git i/kernel/trace/bpf_trace.c w/kernel/trace/bpf_trace.c
> index a7264b2c17ad..fe91836cedcd 100644
> --- i/kernel/trace/bpf_trace.c
> +++ w/kernel/trace/bpf_trace.c
> @@ -941,6 +941,7 @@ BTF_ID(func, vfs_fallocate)
> BTF_ID(func, dentry_open)
> BTF_ID(func, vfs_getattr)
> BTF_ID(func, filp_close)
> +BTF_ID(func, close_fd_get_file)
I liked using the close_range syscall because we did not need to
add new allowed function.. however close_fd_get_file looks safe
enough so I wouldn't mind changing that if you insist ;-)
jirka
> BTF_SET_END(btf_allowlist_d_path)
>
> static bool bpf_d_path_allowed(const struct bpf_prog *prog)
> diff --git i/tools/testing/selftests/bpf/progs/test_d_path.c
> w/tools/testing/selftests/bpf/progs/test_d_path.c
> index 84e1f883f97b..c880cfc95737 100644
> --- i/tools/testing/selftests/bpf/progs/test_d_path.c
> +++ w/tools/testing/selftests/bpf/progs/test_d_path.c
> @@ -40,8 +40,8 @@ int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
> return 0;
> }
>
> -SEC("fentry/filp_close")
> -int BPF_PROG(prog_close, struct file *file, void *id)
> +SEC("fexit/close_fd_get_file")
> +int BPF_PROG(close_fd_get_file, int fd, struct file *file /* retval */)
> {
> pid_t pid = bpf_get_current_pid_tgid() >> 32;
> __u32 cnt = cnt_close;
