All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH vfs v2] bpf: Allow bpf_d_path() from filp_close_sync()
@ 2026-09-25 21:03 Ihor Solodrai
  2026-09-25 21:17 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Ihor Solodrai @ 2026-09-25 21:03 UTC (permalink / raw)
  To: Christian Brauner, Alexei Starovoitov, Andrii Nakryiko,
	Daniel Borkmann, Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: Alexander Viro, Jan Kara, NeilBrown, Jiri Olsa, Mark Brown,
	linux-fsdevel, bpf, linux-kernel, kernel-team

The d_path selftest closes its descriptors with close_range() so that
its fentry program on filp_close() runs. The vfs commit
c1ed65ea167b ("fs: make close_range() synchronous") switched
close_range() to filp_close_sync(), so the program still attaches but
is never called:

  test_d_path_basic:FAIL:close trampoline for filp_close was not called

With that series, close(), close_range() and the closes on exec and
exit all go through filp_close_sync(). Few paths still reach
filp_close(), dup2() being one of them, so a program on filp_close()
no longer sees most file closes.

Allow bpf_d_path() from filp_close_sync(), but only from fentry.
filp_close_sync() drops the last file reference inline, so by the time
an fexit program runs, file->f_path may already be freed, with no RCU
delay for NORCU dentries such as pipes and sockets.

Attach the selftest program to filp_close_sync() and keep triggering
it with close_range().

Add a test rejecting bpf_d_path() from fexit/filp_close_sync.

Assisted-by: LLM
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

---

v1->v2:
  * Allow bpf_d_path() from filp_close_sync() only for fentry (bpf-ci-bot)
  * Add a verifier test rejecting bpf_d_path() from fexit/filp_close_sync
v1: https://lore.kernel.org/bpf/20260924204226.190315-1-ihor.solodrai@linux.dev/

The issue was found by BPF CI when running on linux-next:
https://github.com/kernel-patches/bpf/actions/runs/35923662147/job/107398237366
---
 kernel/trace/bpf_trace.c                      | 16 ++++++++++++--
 .../testing/selftests/bpf/prog_tests/d_path.c |  7 ++++---
 .../testing/selftests/bpf/progs/test_d_path.c |  2 +-
 .../selftests/bpf/progs/verifier_d_path.c     | 21 +++++++++++++++++++
 4 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 29260951aa87..c3b869646bae 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -974,6 +974,10 @@ BTF_ID(func, vfs_getattr)
 BTF_ID(func, filp_close)
 BTF_SET_END(btf_allowlist_d_path)
 
+BTF_SET_START(btf_allowlist_d_path_fentry)
+BTF_ID(func, filp_close_sync)
+BTF_SET_END(btf_allowlist_d_path_fentry)
+
 static bool bpf_d_path_allowed(const struct bpf_prog *prog)
 {
 	if (prog->type == BPF_PROG_TYPE_TRACING &&
@@ -983,8 +987,16 @@ static bool bpf_d_path_allowed(const struct bpf_prog *prog)
 	if (prog->type == BPF_PROG_TYPE_LSM)
 		return bpf_lsm_is_sleepable_hook(prog->aux->attach_btf_id);
 
-	return btf_id_set_contains(&btf_allowlist_d_path,
-				   prog->aux->attach_btf_id);
+	if (btf_id_set_contains(&btf_allowlist_d_path,
+				prog->aux->attach_btf_id))
+		return true;
+
+	if (btf_id_set_contains(&btf_allowlist_d_path_fentry,
+				prog->aux->attach_btf_id))
+		return prog->expected_attach_type == BPF_TRACE_FENTRY ||
+		       prog->expected_attach_type == BPF_TRACE_FENTRY_MULTI;
+
+	return false;
 }
 
 BTF_ID_LIST_SINGLE(bpf_d_path_btf_ids, struct, path)
diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
index 1a2a2f1abf03..7665e1d28a64 100644
--- a/tools/testing/selftests/bpf/prog_tests/d_path.c
+++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
@@ -109,8 +109,9 @@ static int trigger_fstat_events(pid_t pid)
 	fstat(indicatorfd, &fileStat);
 
 out_close:
-	/* sys_close no longer triggers filp_close, but we can
-	 * call sys_close_range instead which still does
+	/*
+	 * filp_close_sync() may be inlined into close(2), so use
+	 * close_range(2), which calls it directly.
 	 */
 	syscall_close(pipefd[0]);
 	syscall_close(pipefd[1]);
@@ -165,7 +166,7 @@ static void test_d_path_basic(void)
 
 	if (CHECK(!bss->called_close,
 		  "close",
-		  "trampoline for filp_close was not called\n"))
+		  "trampoline for filp_close_sync was not called\n"))
 		goto cleanup;
 
 	for (int i = 0; i < MAX_FILES; i++) {
diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c
index 561b2f861808..edb5494a47f7 100644
--- a/tools/testing/selftests/bpf/progs/test_d_path.c
+++ b/tools/testing/selftests/bpf/progs/test_d_path.c
@@ -41,7 +41,7 @@ int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
 	return 0;
 }
 
-SEC("fentry/filp_close")
+SEC("fentry/filp_close_sync")
 int BPF_PROG(prog_close, struct file *file, void *id)
 {
 	pid_t pid = bpf_get_current_pid_tgid() >> 32;
diff --git a/tools/testing/selftests/bpf/progs/verifier_d_path.c b/tools/testing/selftests/bpf/progs/verifier_d_path.c
index 87e51a215558..7ab9b6b1d589 100644
--- a/tools/testing/selftests/bpf/progs/verifier_d_path.c
+++ b/tools/testing/selftests/bpf/progs/verifier_d_path.c
@@ -45,4 +45,25 @@ __naked void d_path_reject(void)
 	: __clobber_all);
 }
 
+SEC("fexit/filp_close_sync")
+__description("d_path fexit reject")
+__failure __msg("helper call is not allowed in probe")
+__naked void d_path_fexit_reject(void)
+{
+	asm volatile ("					\
+	r1 = *(u64 *)(r1 + 0);				\
+	r2 = r10;					\
+	r2 += -8;					\
+	r6 = 0;						\
+	*(u64*)(r2 + 0) = r6;				\
+	r3 = 8 ll;					\
+	call %[bpf_d_path];				\
+	r0 = 0;						\
+	exit;						\
+	"
+	:
+	: __imm(bpf_d_path)
+	: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";

base-commit: 6b97d8b942960f55e821df4f618438e3da125887
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH vfs v2] bpf: Allow bpf_d_path() from filp_close_sync()
  2026-09-25 21:03 [PATCH vfs v2] bpf: Allow bpf_d_path() from filp_close_sync() Ihor Solodrai
@ 2026-09-25 21:17 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-09-25 21:17 UTC (permalink / raw)
  To: Ihor Solodrai; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] bpf: dead code in bpf_d_path_allowed for BPF_TRACE_FENTRY_MULTI
- [Low] bpf: build failure due to missing filp_close_sync BTF ID
- [Low] selftests/bpf: use of deprecated CHECK() macro when updating existing test
--

commit 83314c61560bbf42baf321997dbfd6e5b66818d7
Author: Ihor Solodrai <ihor.solodrai@linux.dev>
bpf: Allow bpf_d_path() from filp_close_sync()

This commit allows bpf_d_path() to be called from the filp_close_sync()
fentry program. It updates the allowlist and related selftests to reflect
the target function change introduced by recent VFS updates.

Link: https://lore.kernel.org/bpf/20260924204226.190315-1-ihor.solodrai@linux.dev/

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 29260951aa871..c3b869646baeb 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -974,6 +974,10 @@ BTF_ID(func, vfs_getattr)
>  BTF_ID(func, filp_close)
>  BTF_SET_END(btf_allowlist_d_path)
>  
> +BTF_SET_START(btf_allowlist_d_path_fentry)
> +BTF_ID(func, filp_close_sync)

[Severity: Low]
Is this function available in the current tree? The commit message mentions
VFS commit c1ed65ea167b, but without it being merged, resolve_btfids will
abort the kernel build because it cannot find the target function
filp_close_sync.

> +BTF_SET_END(btf_allowlist_d_path_fentry)

[ ... ]

> -	return btf_id_set_contains(&btf_allowlist_d_path,
> -				   prog->aux->attach_btf_id);
> +	if (btf_id_set_contains(&btf_allowlist_d_path,
> +				prog->aux->attach_btf_id))
> +		return true;
> +
> +	if (btf_id_set_contains(&btf_allowlist_d_path_fentry,
> +				prog->aux->attach_btf_id))
> +		return prog->expected_attach_type == BPF_TRACE_FENTRY ||
> +		       prog->expected_attach_type == BPF_TRACE_FENTRY_MULTI;

[Severity: Low]
Could this check for BPF_TRACE_FENTRY_MULTI be unreachable? For multi-attach
programs, prog->aux->attach_btf_id is always initialized to a dummy ID
(bpf_multi_func_btf_id[0]) at program load time. Because this dummy ID is not
present in the btf_allowlist_d_path_fentry set, the btf_id_set_contains()
check evaluates to false before reaching the expected_attach_type condition.

> +
> +	return false;
>  }

[ ... ]

> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
> index 1a2a2f1abf033..7665e1d28a641 100644
> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> @@ -165,7 +166,7 @@ static void test_d_path_basic(void)
>  
>  	if (CHECK(!bss->called_close,
>  		  "close",
> -		  "trampoline for filp_close was not called\n"))
> +		  "trampoline for filp_close_sync was not called\n"))

[Severity: Low]
Since this test is being updated, should this deprecated CHECK() macro be
migrated to a modern ASSERT_*() macro? The BPF subsystem selftest standards
mandate using modern assertion macros when updating existing tests.

>  		goto cleanup;

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c
> index 561b2f861808e..edb5494a47f74 100644
> --- a/tools/testing/selftests/bpf/progs/test_d_path.c
> +++ b/tools/testing/selftests/bpf/progs/test_d_path.c
> @@ -41,7 +41,7 @@ int BPF_PROG(prog_stat, struct path *path, struct kstat *stat,
>  	return 0;
>  }
>  
> -SEC("fentry/filp_close")
> +SEC("fentry/filp_close_sync")

[Severity: Low]
Similar to the BTF ID issue above, will this test fail to load because
filp_close_sync does not exist in the current kernel tree?

>  int BPF_PROG(prog_close, struct file *file, void *id)
>  {
>  	pid_t pid = bpf_get_current_pid_tgid() >> 32;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925210347.1364191-1-ihor.solodrai@linux.dev?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-25 21:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-25 21:03 [PATCH vfs v2] bpf: Allow bpf_d_path() from filp_close_sync() Ihor Solodrai
2026-09-25 21:17 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.