mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH vfs-7.5.bpf v2 0/3] bpf: Add bpf_path_getattr()
@ 2026-11-02 10:46 Anastasios Papagiannis
  2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 1/3] " Anastasios Papagiannis
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Anastasios Papagiannis @ 2026-11-02 10:46 UTC (permalink / raw)
  To: brauner
  Cc: bpf, linux-kernel, linux-fsdevel, ast, daniel, andrii, eddyz87,
	memxor, viro, jack, tasos.papagiannnis

Userspace code can populate BPF maps using file identities obtained from
stat() and related system calls. BPF LSM programs need matching inode and
device numbers to look up those entries. Reading inode fields directly can
give different values on overlayfs and miss filesystem-specific getattr
behavior.

bpf_real_data_inode() resolves the inode hosting a file's data, whose
identity and metadata can differ from those reported by stat() and related
system calls on overlayfs. It does not invoke the filesystem's getattr
operation.

This series adds bpf_path_getattr() for sleepable LSM programs. It takes a
trusted path and calls vfs_getattr_nosec() to avoid calling back into
security_inode_getattr().

The kfunc returns struct kstat. Inode and device identity follow the
filesystem's getattr operation.

Selftests compare basic fields with statx() on tmpfs and on overlayfs
before and after copy-up, accounting for device encoding, and exercise
verifier restrictions. The series also updates the relevant documentation.

Changes in v2:
- Use a thin vfs_getattr_nosec() wrapper returning struct kstat. (Alexei)
- Remove struct bpf_file_stat. (Alexei)
- Update the documentation and tests.
- Rebase onto vfs-7.5.bpf for merging through the VFS tree. (Daniel)

v1:
https://lore.kernel.org/bpf/20261029095952.44833-1-tasos.papagiannnis@gmail.com/

Anastasios Papagiannis (3):
  bpf: Add bpf_path_getattr()
  docs/bpf: Document bpf_path_getattr()
  selftests/bpf: Test bpf_path_getattr()

 Documentation/bpf/fs_kfuncs.rst               |  13 ++
 fs/bpf_fs_kfuncs.c                            |  38 +++-
 .../selftests/bpf/prog_tests/path_getattr.c   | 162 ++++++++++++++++++
 .../selftests/bpf/progs/path_getattr.c        |  42 +++++
 .../bpf/progs/verifier_path_getattr.c         |  57 ++++++
 5 files changed, 311 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/path_getattr.c
 create mode 100644 tools/testing/selftests/bpf/progs/path_getattr.c
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_path_getattr.c


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.56.0


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

* [PATCH vfs-7.5.bpf v2 1/3] bpf: Add bpf_path_getattr()
  2026-11-02 10:46 [PATCH vfs-7.5.bpf v2 0/3] bpf: Add bpf_path_getattr() Anastasios Papagiannis
@ 2026-11-02 10:46 ` Anastasios Papagiannis
  2026-11-06  8:36   ` Christian Brauner
  2026-11-06 10:38   ` Christian Brauner
  2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 2/3] docs/bpf: Document bpf_path_getattr() Anastasios Papagiannis
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 7+ messages in thread
From: Anastasios Papagiannis @ 2026-11-02 10:46 UTC (permalink / raw)
  To: brauner
  Cc: bpf, linux-kernel, linux-fsdevel, ast, daniel, andrii, eddyz87,
	memxor, viro, jack, tasos.papagiannnis

Direct inode reads can differ from stat() and related system calls,
especially on overlayfs.

Add a sleepable LSM kfunc to query a trusted path and return struct kstat.
Use vfs_getattr_nosec() to avoid calling security_inode_getattr() again.
Limit the copy to the smaller of the buffer and the kernel structure.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
---
 fs/bpf_fs_kfuncs.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 35ccd98a4de6..97afc9659f1a 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -96,6 +96,40 @@ __bpf_kfunc int bpf_path_d_path(const struct path *path, char *buf, size_t buf__
 	return len;
 }
 
+/**
+ * bpf_path_getattr - get the attributes of the supplied path
+ * @path: trusted path to get the attributes of
+ * @request_mask: STATX_* flags of the attributes that the caller wants
+ * @query_flags: AT_STATX_SYNC_AS_STAT, AT_STATX_FORCE_SYNC or AT_STATX_DONT_SYNC
+ * @stat__uninit: buffer to return the attributes in
+ * @stat__sz: size of the supplied buffer
+ *
+ * Ask the filesystem for the attributes of *path*. This is vfs_getattr()
+ * without the call into security_inode_getattr().
+ *
+ * This BPF kfunc may only be called from sleepable BPF LSM programs.
+ *
+ * Copies the smaller of @stat__sz and sizeof(struct kstat). Any remaining
+ * buffer bytes are left untouched. On error, the buffer is unchanged.
+ *
+ * Return: Zero on success or a negative error code.
+ */
+__bpf_kfunc int bpf_path_getattr(const struct path *path, u32 request_mask,
+				 u32 query_flags, struct kstat *stat__uninit,
+				 size_t stat__sz)
+{
+	struct kstat stat;
+	int ret;
+
+	ret = vfs_getattr_nosec(path, &stat, request_mask, query_flags);
+	if (ret)
+		return ret;
+
+	/* struct kstat in the program's vmlinux.h may have a different size. */
+	memcpy(stat__uninit, &stat, min(stat__sz, sizeof(stat)));
+	return 0;
+}
+
 static bool match_security_bpf_prefix(const char *name__str)
 {
 	return !strncmp(name__str, XATTR_NAME_BPF_LSM, XATTR_NAME_BPF_LSM_LEN);
@@ -504,6 +538,7 @@ BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
 BTF_ID_FLAGS(func, bpf_get_task_exe_file, KF_ACQUIRE | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE)
 BTF_ID_FLAGS(func, bpf_path_d_path)
+BTF_ID_FLAGS(func, bpf_path_getattr, KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE)
@@ -515,8 +550,9 @@ BTF_ID_FLAGS(func, bpf_sock_read_xattr, KF_RCU)
 #endif
 BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
 
-/* Side-effecting kfuncs that stay exclusive to LSM programs. */
+/* Kfuncs that stay exclusive to LSM programs. */
 BTF_SET_START(bpf_fs_kfunc_lsm_only_ids)
+BTF_ID(func, bpf_path_getattr)
 BTF_ID(func, bpf_set_dentry_xattr)
 BTF_ID(func, bpf_remove_dentry_xattr)
 BTF_SET_END(bpf_fs_kfunc_lsm_only_ids)
-- 
2.56.0


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

* [PATCH vfs-7.5.bpf v2 2/3] docs/bpf: Document bpf_path_getattr()
  2026-11-02 10:46 [PATCH vfs-7.5.bpf v2 0/3] bpf: Add bpf_path_getattr() Anastasios Papagiannis
  2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 1/3] " Anastasios Papagiannis
@ 2026-11-02 10:46 ` Anastasios Papagiannis
  2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 3/3] selftests/bpf: Test bpf_path_getattr() Anastasios Papagiannis
  2026-11-02 22:49 ` [PATCH vfs-7.5.bpf v2 0/3] bpf: Add bpf_path_getattr() Alexei Starovoitov
  3 siblings, 0 replies; 7+ messages in thread
From: Anastasios Papagiannis @ 2026-11-02 10:46 UTC (permalink / raw)
  To: brauner
  Cc: bpf, linux-kernel, linux-fsdevel, ast, daniel, andrii, eddyz87,
	memxor, viro, jack, tasos.papagiannnis

Describe the VFS result, output buffer, and calling requirements.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
---
 Documentation/bpf/fs_kfuncs.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/bpf/fs_kfuncs.rst b/Documentation/bpf/fs_kfuncs.rst
index 8762c3233a3d..05cc9b657a42 100644
--- a/Documentation/bpf/fs_kfuncs.rst
+++ b/Documentation/bpf/fs_kfuncs.rst
@@ -13,9 +13,22 @@ BPF kfuncs can be used to get these data.
 
  * ``bpf_get_fsverity_digest()``
 
+ * ``bpf_path_getattr()``
+
 To avoid recursions, these kfuncs follow the following rules:
 
 1. These kfuncs are only permitted from BPF LSM function.
 2. These kfuncs should not call into other LSM hooks, i.e. security_*(). For
    example, ``bpf_get_file_xattr()`` does not use ``vfs_getxattr()``, because
    the latter calls LSM hook ``security_inode_getxattr``.
+
+File attributes
+===============
+
+``bpf_path_getattr()`` queries filesystem attributes through
+``vfs_getattr_nosec()`` and returns ``struct kstat``. Use a trusted hook
+argument, such as the path in ``lsm.s/inode_getattr`` or
+``&file->f_path`` in ``lsm.s/file_open``.
+
+.. kernel-doc:: fs/bpf_fs_kfuncs.c
+   :identifiers: bpf_path_getattr
-- 
2.56.0


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

* [PATCH vfs-7.5.bpf v2 3/3] selftests/bpf: Test bpf_path_getattr()
  2026-11-02 10:46 [PATCH vfs-7.5.bpf v2 0/3] bpf: Add bpf_path_getattr() Anastasios Papagiannis
  2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 1/3] " Anastasios Papagiannis
  2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 2/3] docs/bpf: Document bpf_path_getattr() Anastasios Papagiannis
@ 2026-11-02 10:46 ` Anastasios Papagiannis
  2026-11-02 22:49 ` [PATCH vfs-7.5.bpf v2 0/3] bpf: Add bpf_path_getattr() Alexei Starovoitov
  3 siblings, 0 replies; 7+ messages in thread
From: Anastasios Papagiannis @ 2026-11-02 10:46 UTC (permalink / raw)
  To: brauner
  Cc: bpf, linux-kernel, linux-fsdevel, ast, daniel, andrii, eddyz87,
	memxor, viro, jack, tasos.papagiannnis

Compare basic fields with statx() on tmpfs and overlayfs, including
copy-up and device encoding. Cover verifier restrictions.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
---
 .../selftests/bpf/prog_tests/path_getattr.c   | 162 ++++++++++++++++++
 .../selftests/bpf/progs/path_getattr.c        |  42 +++++
 .../bpf/progs/verifier_path_getattr.c         |  57 ++++++
 3 files changed, 261 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/path_getattr.c
 create mode 100644 tools/testing/selftests/bpf/progs/path_getattr.c
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_path_getattr.c

diff --git a/tools/testing/selftests/bpf/prog_tests/path_getattr.c b/tools/testing/selftests/bpf/prog_tests/path_getattr.c
new file mode 100644
index 000000000000..6d8cabfd24c6
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/path_getattr.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+
+#include <linux/stat.h>
+#include <sched.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <test_progs.h>
+
+#include "path_getattr.skel.h"
+#include "verifier_path_getattr.skel.h"
+
+static void query_stat(struct path_getattr *skel, const char *path)
+{
+	struct statx stx;
+	int err;
+
+	skel->bss->request_mask = STATX_BASIC_STATS;
+	skel->bss->calls = 0;
+	skel->bss->monitored_tid = syscall(SYS_gettid);
+	err = statx(AT_FDCWD, path, 0, skel->bss->request_mask, &stx);
+	skel->bss->monitored_tid = 0;
+	if (!ASSERT_OK(err, "statx") ||
+	    !ASSERT_EQ(skel->bss->calls, 1, "hook_calls") ||
+	    !ASSERT_OK(skel->bss->getattr_ret, "bpf_path_getattr"))
+		return;
+	ASSERT_EQ(skel->bss->ino, stx.stx_ino, "ino");
+	/* Kernel dev_t stores the minor number in the low 20 bits. */
+	ASSERT_EQ(skel->bss->dev, (stx.stx_dev_major << 20) | stx.stx_dev_minor, "dev");
+	ASSERT_EQ(skel->bss->size, stx.stx_size, "size");
+	ASSERT_EQ(skel->bss->mode, stx.stx_mode, "mode");
+	ASSERT_EQ(skel->bss->uid, stx.stx_uid, "uid");
+	ASSERT_EQ(skel->bss->gid, stx.stx_gid, "gid");
+	ASSERT_EQ(skel->bss->result_mask & STATX_BASIC_STATS,
+		  stx.stx_mask & STATX_BASIC_STATS, "result_mask");
+}
+
+static int create_file(const char *path)
+{
+	int fd, err;
+
+	fd = open(path, O_CREAT | O_EXCL | O_RDWR | O_CLOEXEC, 0640);
+	if (!ASSERT_GE(fd, 0, "create_file"))
+		return -1;
+	err = ftruncate(fd, 12345);
+	if (ASSERT_OK(err, "truncate_file"))
+		err = fchown(fd, 123, 456);
+	ASSERT_OK(err, "chown_file");
+	close(fd);
+	return err;
+}
+
+static void test_overlay(struct path_getattr *skel, const char *root)
+{
+	char lower[PATH_MAX], upper[PATH_MAX], work[PATH_MAX], merged[PATH_MAX];
+	char lower_file[PATH_MAX], merged_file[PATH_MAX], opts[PATH_MAX * 3 + 64];
+	__u32 overlay_dev;
+	int fd;
+
+	snprintf(lower, sizeof(lower), "%s/lower", root);
+	snprintf(upper, sizeof(upper), "%s/upper", root);
+	snprintf(work, sizeof(work), "%s/work", root);
+	snprintf(merged, sizeof(merged), "%s/merged", root);
+	if (!ASSERT_OK(mkdir(lower, 0755), "mkdir_lower") ||
+	    !ASSERT_OK(mkdir(upper, 0755), "mkdir_upper") ||
+	    !ASSERT_OK(mkdir(work, 0755), "mkdir_work") ||
+	    !ASSERT_OK(mkdir(merged, 0755), "mkdir_merged"))
+		return;
+	snprintf(lower_file, sizeof(lower_file), "%s/lower/file", root);
+	snprintf(merged_file, sizeof(merged_file), "%s/merged/file", root);
+	/*
+	 * Distinct layer filesystems with xino disabled expose a different
+	 * device for regular files than the overlay superblock's device.
+	 */
+	if (!ASSERT_OK(mount("tmpfs", lower, "tmpfs", 0, NULL), "mount_lower"))
+		return;
+	if (create_file(lower_file))
+		goto out_lower;
+	snprintf(opts, sizeof(opts), "lowerdir=%s,upperdir=%s,workdir=%s,xino=off",
+		 lower, upper, work);
+	if (mount("overlay", merged, "overlay", 0, opts)) {
+		if (errno == ENODEV || errno == EOPNOTSUPP) {
+			printf("%s:SKIP: overlayfs unavailable: %s\n",
+			       __func__, strerror(errno));
+			test__skip();
+			goto out_lower;
+		}
+		ASSERT_OK(-errno, "mount_overlay");
+		goto out_lower;
+	}
+	query_stat(skel, merged);
+	overlay_dev = skel->bss->dev;
+	query_stat(skel, merged_file);
+	ASSERT_NEQ(skel->bss->dev, overlay_dev, "lower_layer_dev");
+	/* Opening for write copies a lower-only file into the upper layer. */
+	fd = open(merged_file, O_WRONLY | O_CLOEXEC);
+	if (ASSERT_GE(fd, 0, "copy_up")) {
+		ASSERT_OK(ftruncate(fd, 23456), "truncate_upper");
+		close(fd);
+		query_stat(skel, merged_file);
+		ASSERT_NEQ(skel->bss->dev, overlay_dev, "upper_layer_dev");
+	}
+	umount(merged);
+out_lower:
+	umount(lower);
+}
+
+void test_path_getattr(void)
+{
+	char root[] = "/tmp/bpf_path_getattr.XXXXXX";
+	struct path_getattr *skel;
+	char file[PATH_MAX];
+	int nsfd, cwdfd;
+
+	skel = path_getattr__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+	if (!ASSERT_OK(path_getattr__attach(skel), "attach_skeleton"))
+		goto out_skel;
+	nsfd = open("/proc/self/ns/mnt", O_RDONLY | O_CLOEXEC);
+	if (!ASSERT_GE(nsfd, 0, "save_mountns"))
+		goto out_skel;
+	cwdfd = open(".", O_RDONLY | O_DIRECTORY | O_CLOEXEC);
+	if (!ASSERT_GE(cwdfd, 0, "save_cwd"))
+		goto out_ns;
+	if (!ASSERT_OK(unshare(CLONE_NEWNS), "unshare_mountns"))
+		goto out_cwd;
+	if (!ASSERT_OK(mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL), "private_mounts") ||
+	    !ASSERT_OK_PTR(mkdtemp(root), "mkdtemp"))
+		goto out_restore;
+	if (!ASSERT_OK(mount("tmpfs", root, "tmpfs", 0, NULL), "mount_tmpfs"))
+		goto out_dir;
+	snprintf(file, sizeof(file), "%s/file", root);
+	if (create_file(file))
+		goto out_umount;
+	if (test__start_subtest("stat_parity")) {
+		query_stat(skel, file);
+		query_stat(skel, root);
+	}
+	if (test__start_subtest("overlay"))
+		test_overlay(skel, root);
+out_umount:
+	umount2(root, MNT_DETACH);
+out_dir:
+	rmdir(root);
+out_restore:
+	ASSERT_OK(setns(nsfd, CLONE_NEWNS), "restore_mountns");
+	/* setns() also resets the working directory. */
+	ASSERT_OK(fchdir(cwdfd), "restore_cwd");
+out_cwd:
+	close(cwdfd);
+out_ns:
+	close(nsfd);
+out_skel:
+	path_getattr__destroy(skel);
+}
+
+void test_verifier_path_getattr(void)
+{
+	RUN_TESTS(verifier_path_getattr);
+}
diff --git a/tools/testing/selftests/bpf/progs/path_getattr.c b/tools/testing/selftests/bpf/progs/path_getattr.c
new file mode 100644
index 000000000000..509cf13a0047
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/path_getattr.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_kfuncs.h"
+
+char _license[] SEC("license") = "GPL";
+
+__u32 monitored_tid;
+__u32 request_mask;
+__u32 calls;
+int getattr_ret;
+
+__u64 ino;
+__u32 dev;
+__s64 size;
+__u32 mode;
+__u32 uid;
+__u32 gid;
+__u32 result_mask;
+
+SEC("lsm.s/inode_getattr")
+int BPF_PROG(path_getattr, const struct path *path, int ret)
+{
+	struct kstat stat = {};
+
+	if (ret || (__u32)bpf_get_current_pid_tgid() != monitored_tid || calls)
+		return ret;
+	calls++;
+	getattr_ret = bpf_path_getattr(path, request_mask, 0, &stat, sizeof(stat));
+	if (!getattr_ret) {
+		ino = stat.ino;
+		dev = stat.dev;
+		size = stat.size;
+		mode = stat.mode;
+		uid = stat.uid.val;
+		gid = stat.gid.val;
+		result_mask = stat.result_mask;
+	}
+	return ret;
+}
diff --git a/tools/testing/selftests/bpf/progs/verifier_path_getattr.c b/tools/testing/selftests/bpf/progs/verifier_path_getattr.c
new file mode 100644
index 000000000000..8792f4242727
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_path_getattr.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_kfuncs.h"
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+static struct kstat output;
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(getattr_embedded_path_stack, struct file *file)
+{
+	struct kstat st;
+
+	bpf_path_getattr(&file->f_path, 0, 0, &st, sizeof(st));
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("dereference of modified untrusted_ptr_")
+int BPF_PROG(getattr_untrusted_path)
+{
+	struct task_struct *task = bpf_get_current_task_btf();
+
+	bpf_path_getattr(&task->fs->pwd, 0, 0, &output, sizeof(output));
+	return 0;
+}
+
+SEC("lsm/inode_getattr")
+__failure __msg("program must be sleepable to call sleepable kfunc bpf_path_getattr")
+int BPF_PROG(getattr_non_sleepable, const struct path *path)
+{
+	bpf_path_getattr(path, 0, 0, &output, sizeof(output));
+	return 0;
+}
+
+SEC("fentry/vfs_getattr")
+__failure __msg("calling kernel function bpf_path_getattr is not allowed")
+int BPF_PROG(getattr_fentry, const struct path *path)
+{
+	bpf_path_getattr(path, 0, 0, &output, sizeof(output));
+	return 0;
+}
+
+SEC("lsm.s/inode_getattr")
+__failure __msg("invalid write to stack")
+int BPF_PROG(getattr_short_output, const struct path *path)
+{
+	__u64 small;
+
+	bpf_path_getattr(path, 0, 0, (void *)&small, sizeof(output));
+	return 0;
+}
-- 
2.56.0


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

* Re: [PATCH vfs-7.5.bpf v2 0/3] bpf: Add bpf_path_getattr()
  2026-11-02 10:46 [PATCH vfs-7.5.bpf v2 0/3] bpf: Add bpf_path_getattr() Anastasios Papagiannis
                   ` (2 preceding siblings ...)
  2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 3/3] selftests/bpf: Test bpf_path_getattr() Anastasios Papagiannis
@ 2026-11-02 22:49 ` Alexei Starovoitov
  3 siblings, 0 replies; 7+ messages in thread
From: Alexei Starovoitov @ 2026-11-02 22:49 UTC (permalink / raw)
  To: Anastasios Papagiannis, brauner
  Cc: bpf, linux-kernel, linux-fsdevel, daniel, andrii, eddyz87,
	memxor, viro, jack

On Mon, Nov 02, 2026 at 02:46 AM Anastasios Papagiannis <tasos.papagiannnis@gmail.com> wrote:
> Changes in v2:
> - Use a thin vfs_getattr_nosec() wrapper returning struct kstat. (Alexei)
> - Remove struct bpf_file_stat. (Alexei)
> - Update the documentation and tests.
> - Rebase onto vfs-7.5.bpf for merging through the VFS tree. (Daniel)

For the series:
Acked-by: Alexei Starovoitov <ast@kernel.org>

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

* Re: [PATCH vfs-7.5.bpf v2 1/3] bpf: Add bpf_path_getattr()
  2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 1/3] " Anastasios Papagiannis
@ 2026-11-06  8:36   ` Christian Brauner
  2026-11-06 10:38   ` Christian Brauner
  1 sibling, 0 replies; 7+ messages in thread
From: Christian Brauner @ 2026-11-06  8:36 UTC (permalink / raw)
  To: Anastasios Papagiannis
  Cc: bpf, linux-kernel, linux-fsdevel, ast, daniel, andrii, eddyz87,
	memxor, viro, jack

On Mon, Nov 02, 2026 at 12:46:53PM +0200, Anastasios Papagiannis wrote:
> +	ret = vfs_getattr_nosec(path, &stat, request_mask, query_flags);

Hm, do_statx() filters both masks before they hit the filesystem and we
should do the same here:

	if (request_mask & STATX__RESERVED)
		return -EINVAL;
	if ((query_flags & AT_STATX_SYNC_TYPE) == AT_STATX_SYNC_TYPE)
		return -EINVAL;
	request_mask &= ~STATX_CHANGE_COOKIE;

STATX_CHANGE_COOKIE isn't innocous... It causes generic_fillattr() to
call inode_query_iversion() which sets I_VERSION_QUERIED and forces the
next change to bump i_version.

That's in-kernel only. A bpf program shouldn't be able to set it on
every inode it looks at.

> -/* Side-effecting kfuncs that stay exclusive to LSM programs. */
> +/* Kfuncs that stay exclusive to LSM programs. */
>  BTF_SET_START(bpf_fs_kfunc_lsm_only_ids)
> +BTF_ID(func, bpf_path_getattr)

I don't think this belongs here. The set exists so binfmt_misc
handlers can't modify filesystem state. bpf_path_getattr() doesn't
modify anything and the binfmt_misc match and load programs are
sleepable. So they can use it on bprm->file->f_path just like
bpf_path_d_path() and bpf_get_file_xattr(). I'd drop this.

-- 

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

* Re: [PATCH vfs-7.5.bpf v2 1/3] bpf: Add bpf_path_getattr()
  2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 1/3] " Anastasios Papagiannis
  2026-11-06  8:36   ` Christian Brauner
@ 2026-11-06 10:38   ` Christian Brauner
  1 sibling, 0 replies; 7+ messages in thread
From: Christian Brauner @ 2026-11-06 10:38 UTC (permalink / raw)
  To: Anastasios Papagiannis
  Cc: bpf, linux-kernel, linux-fsdevel, ast, daniel, andrii, eddyz87,
	memxor, viro, jack

On Mon, Nov 02, 2026 at 12:46:53PM +0200, Anastasios Papagiannis wrote:
> +	ret = vfs_getattr_nosec(path, &stat, request_mask, query_flags);

This blocks on the filesystem and it's callable from every sleepable LSM
hook. It's trivial to get a trusted path via bpf_get_task_exe_file().

So the issue I have is that some of those hooks run under locks where
blocking on the filesystem is a deadlock...

For example, a synchronous NFS rpc is transmitted in the caller's
context,

	__rpc_execute()
	-> call_transmit()		/* XPRT_LOCKED, xprt->snd_task = T1 */
	   -> xs_tcp_send_request()
	      -> xprt_sock_sendmsg()
	         -> sock_sendmsg()
	            -> __sock_sendmsg()
	               -> security_socket_sendmsg()

so a program on lsm.s/socket_sendmsg doing

	file = bpf_get_task_exe_file(bpf_get_current_task_btf());
	bpf_path_getattr(&file->f_path, STATX_BASIC_STATS, 0, &st, sizeof(st));

When the attribute cache expires and the file on the same nfs mount you
get:

	nfs_getattr()
	-> __nfs_revalidate_inode()
	   -> rpc_call_sync()		/* T2 */
	      -> xprt_reserve_xprt()	/* XPRT_LOCKED && T2 != snd_task */
	         -> rpc_sleep_on(&xprt->sending)

with T1 never finishing the send. cifs has the same shape with
server->_srv_mutex held across smb_send() and ocfs2 with sc_send_lock.

There are others of the same kind:

- bprm_committing_creds and bprm_committed_creds run with
  exec_update_lock held for write.

  A FUSE_GETATTR on bprm->file deadlocks if the daemon does
  readlink("/proc/<pid>/exe") or reads /proc/<pid>/stat of the
  requester because both take exec_update_lock now.

  I just massaged that codepath and moved close_cloexec_files() out from
  under that lock for exactly this reason.

  Fun fact, request_wait_answer() is uninterruptible once the daemon has
  read the request.

- sb_show_options and sb_statfs via statmount() hold namespace_sem.

- inode_free_security runs from evict under s_umount or from reclaim.

- task_setscheduler via cpuset_can_attach() holds threadgroup_rwsem for write.

- file_receive from seccomp addfd holds notify_lock.

The worst part is that - as I said in the bpf session at LSFMM -
bpf_get_file_xattr() already brings in similar problems.

But it needs an NFSv4.2 server with xattr support and a cache miss.

In contrast, getattr is hit on every attribute cache expiry and every
fuse getattr.

So we need mechanisms in bpf to guard against things like that
architecturally. Ideally it would not just be a filter like in the past.

And really, request_mask and query_flags reach the filesystem
unfiltered and do_statx() does:

	if (mask & STATX__RESERVED)
		return -EINVAL;
	if ((flags & AT_STATX_SYNC_TYPE) == AT_STATX_SYNC_TYPE)
		return -EINVAL;
	mask &= ~STATX_CHANGE_COOKIE;

and we should do the same here.

And STATX_CHANGE_COOKIE does changes to the inode. generic_fillattr()
calls inode_query_iversion() which sets I_VERSION_QUERIED. That forces
i_version bumps. That's kernel-only infra.

> -/* Side-effecting kfuncs that stay exclusive to LSM programs. */
> +/* Kfuncs that stay exclusive to LSM programs. */
>  BTF_SET_START(bpf_fs_kfunc_lsm_only_ids)
> +BTF_ID(func, bpf_path_getattr)

The other thing is that struct kstat uses very different formats for say
device numbers than userspace.

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

end of thread, other threads:[~2026-11-06 10:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02 10:46 [PATCH vfs-7.5.bpf v2 0/3] bpf: Add bpf_path_getattr() Anastasios Papagiannis
2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 1/3] " Anastasios Papagiannis
2026-11-06  8:36   ` Christian Brauner
2026-11-06 10:38   ` Christian Brauner
2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 2/3] docs/bpf: Document bpf_path_getattr() Anastasios Papagiannis
2026-11-02 10:46 ` [PATCH vfs-7.5.bpf v2 3/3] selftests/bpf: Test bpf_path_getattr() Anastasios Papagiannis
2026-11-02 22:49 ` [PATCH vfs-7.5.bpf v2 0/3] bpf: Add bpf_path_getattr() Alexei Starovoitov

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®