Re: [RFC/PATCH bpf-next] bpf: Fix d_path test after last fs update

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, Aug 31, 2023 at 03:37:37PM +0800, Hou Tao wrote:
> Hi
> 
> On 8/30/2023 5:35 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.
> 
> To make test_d_path self-test pass, beside attaching to a different
> function (e.g., __fput_sync or filp_flush), we could also use
> close_range() or even dup2() to close the created fd, because these
> syscalls still use filp_close() to close the opened file.

nice, I like the close_range solution ;-) patch below fixes the test

> 
> >
> > I can't see any other solution than to hook filp_flush function and
> > that also means we need to add it to btf_allowlist_d_path list, so
> > it can use the d_path helper.
> >
> > But it's probably not very stable because filp_flush is static so it
> > could be potentially inlined.
> >
> > Also if we'd keep the current filp_close hook and find a way how to 'wait'
> > for it to be called so user space can go with checks, then it looks
> > like d_path might not work properly when the task is no longer around.
> 
> It seems there is no need to wait for it to be called, because
> filp_close() is still called synchronously by some syscall (e.g.,
> close_range or io_uring). So if the bpf program tries to collect many
> close event as possible, it should be attach to both filp_close() and
> __fput_sync(), right ?
> 

right, when we hook to close_range it's still synchronous

thanks,
jirka


---
diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
index 911345c526e6..8a558c980753 100644
--- a/tools/testing/selftests/bpf/prog_tests/d_path.c
+++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
@@ -91,6 +91,7 @@ static int trigger_fstat_events(pid_t pid)
 
 out_close:
 	/* triggers filp_close */
+#define close(fd) close_range(fd, fd, 0)
 	close(pipefd[0]);
 	close(pipefd[1]);
 	close(sockfd);
@@ -98,6 +99,7 @@ static int trigger_fstat_events(pid_t pid)
 	close(devfd);
 	close(localfd);
 	close(indicatorfd);
+#undef close
 	return ret;
 }