mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH vfs v3] bpf: Allow bpf_d_path() from filp_close_sync()
@ 2026-10-21 20:13 Ihor Solodrai
  2026-10-23 16:50 ` Ihor Solodrai
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ihor Solodrai @ 2026-10-21 20:13 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.

Commit 056069eb2b66 ("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
Fixes: 056069eb2b66 ("fs: make close_range() synchronous")
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

---

v2->v3:
  * Drop the unreachable BPF_TRACE_FENTRY_MULTI check (Sashiko)
  * Add a Fixes: tag
v2: https://lore.kernel.org/bpf/20260925210347.1364191-1-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                      | 15 +++++++++++--
 .../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, 39 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 29260951aa87..4b2f944bf67f 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,15 @@ 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;
+
+	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: a804c5742bff224d4cb4d25dd383554d598be32b
-- 
2.56.0


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

* Re: [PATCH vfs v3] bpf: Allow bpf_d_path() from filp_close_sync()
  2026-10-21 20:13 [PATCH vfs v3] bpf: Allow bpf_d_path() from filp_close_sync() Ihor Solodrai
@ 2026-10-23 16:50 ` Ihor Solodrai
  2026-10-29 20:51 ` Ihor Solodrai
  2026-10-30 15:10 ` Christian Brauner
  2 siblings, 0 replies; 4+ messages in thread
From: Ihor Solodrai @ 2026-10-23 16:50 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

On 10/21/26 1:13 PM, Ihor Solodrai wrote:
> The d_path selftest closes its descriptors with close_range() so that
> its fentry program on filp_close() runs.
> 
> Commit 056069eb2b66 ("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
> Fixes: 056069eb2b66 ("fs: make close_range() synchronous")
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

Christian,

Just in case you pay attention to BPF CI signals, they are red for
this patch because commit 056069eb2b66 is not in bpf-next yet.

Thanks.

> [...]

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

* Re: [PATCH vfs v3] bpf: Allow bpf_d_path() from filp_close_sync()
  2026-10-21 20:13 [PATCH vfs v3] bpf: Allow bpf_d_path() from filp_close_sync() Ihor Solodrai
  2026-10-23 16:50 ` Ihor Solodrai
@ 2026-10-29 20:51 ` Ihor Solodrai
  2026-10-30 15:10 ` Christian Brauner
  2 siblings, 0 replies; 4+ messages in thread
From: Ihor Solodrai @ 2026-10-29 20:51 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

On 2026-10-21 1:13 p.m., Ihor Solodrai wrote:
> The d_path selftest closes its descriptors with close_range() so that
> its fentry program on filp_close() runs.
> 
> Commit 056069eb2b66 ("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
> Fixes: 056069eb2b66 ("fs: make close_range() synchronous")
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> 
> [...]

Bumping this, in case it fall off the radar.

bpf-next has the failure right now, we apply the fix on CI.


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

* Re: [PATCH vfs v3] bpf: Allow bpf_d_path() from filp_close_sync()
  2026-10-21 20:13 [PATCH vfs v3] bpf: Allow bpf_d_path() from filp_close_sync() Ihor Solodrai
  2026-10-23 16:50 ` Ihor Solodrai
  2026-10-29 20:51 ` Ihor Solodrai
@ 2026-10-30 15:10 ` Christian Brauner
  2 siblings, 0 replies; 4+ messages in thread
From: Christian Brauner @ 2026-10-30 15:10 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Ihor Solodrai
  Cc: Alexander Viro, Jan Kara, NeilBrown, Jiri Olsa, Mark Brown,
	linux-fsdevel, bpf, linux-kernel, kernel-team

On Wed, 21 Oct 2026 13:13:59 -0700, Ihor Solodrai wrote:
> The d_path selftest closes its descriptors with close_range() so that
> its fentry program on filp_close() runs.
> 
> Commit 056069eb2b66 ("fs: make close_range() synchronous") switched
> close_range() to filp_close_sync(), so the program still attaches but
> is never called:
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] bpf: Allow bpf_d_path() from filp_close_sync()
      https://git.kernel.org/vfs/vfs/c/3a00ecc89069


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

end of thread, other threads:[~2026-10-30 15:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-21 20:13 [PATCH vfs v3] bpf: Allow bpf_d_path() from filp_close_sync() Ihor Solodrai
2026-10-23 16:50 ` Ihor Solodrai
2026-10-29 20:51 ` Ihor Solodrai
2026-10-30 15:10 ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®