[PATCH bpf-next 0/3] bpf: Add bpf_path_getattr()

[PATCH bpf-next 0/3] bpf: Add bpf_path_getattr()

[Anastasios Papagiannis]

Userspace code can populate BPF maps using file identities obtained from
statx(). BPF LSM programs need a way to generate those inode and device
numbers to look up for those entries. Reading inode fields directly can
give different values on overlayfs and misses 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 statx() on 
overlayfs. It does not invoke the filesystem's getattr operation or
provide mount information.

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

The kfunc returns a 64-byte bpf_file_stat. We avoid using struct kstat
directly as this is an internal structure and this can lead to changing
the user API. Inode and device identity follow the filesystem's getattr
operation, and the device number uses stat(2) encoding. Mount IDs,
mount-root attributes and ownership translation follow statx(), including
mount idmapping and translation into the current task's user namespace.

Selftests compare the result with statx() on tmpfs and on overlayfs before
and after copy-up, and exercise verifier restrictions. The series also
updates the relevant documentation.

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                            | 108 +++++++++-
 .../selftests/bpf/prog_tests/path_getattr.c   | 185 ++++++++++++++++++
 .../selftests/bpf/progs/path_getattr.c        |  25 +++
 .../bpf/progs/verifier_path_getattr.c         |  57 ++++++
 5 files changed, 387 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: d80373a8e70d3193151232c3062abbbe367ff5ad
-- 
2.56.0

[PATCH bpf-next 1/3] bpf: Add bpf_path_getattr()

[Anastasios Papagiannis]

Direct inode reads can differ from statx(), especially on overlayfs.

Add a sleepable LSM kfunc to query a trusted path and return a compact
result with statx semantics. Use vfs_getattr_nosec() to avoid calling
security_inode_getattr() again, and validate the request and output buffer.

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

diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 35ccd98a4de6..95d7b0de2453 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -1,23 +1,127 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2024 Google LLC. */
 
+#include <linux/align.h>
 #include <linux/binfmt_misc.h>
 #include <linux/bpf.h>
 #include <linux/bpf_lsm.h>
 #include <linux/btf.h>
 #include <linux/btf_ids.h>
+#include <linux/cred.h>
 #include <linux/dcache.h>
 #include <linux/fs.h>
 #include <linux/fsnotify.h>
 #include <linux/file.h>
+#include <linux/kdev_t.h>
 #include <linux/kernfs.h>
 #include <linux/lsm_hooks.h>
 #include <linux/mm.h>
 #include <linux/net.h>
 #include <linux/xattr.h>
 
+#include "internal.h"
+#include "mount.h"
+
+/**
+ * struct bpf_file_stat - attributes returned by bpf_path_getattr()
+ * @ino: inode number
+ * @dev: device number in huge_encode_dev() format
+ * @mnt_id: legacy mount ID, or unique ID if STATX_MNT_ID_UNIQUE is requested
+ * @size: file size in bytes
+ * @attributes: STATX_ATTR_* flags
+ * @attributes_mask: supported bits in @attributes
+ * @mode: file type and permissions
+ * @uid: owner UID in the current task's user namespace
+ * @gid: owner GID in the current task's user namespace
+ * @result_mask: available STATX_* fields represented in this structure
+ *
+ * Ownership includes mount idmapping. Unmapped IDs use the overflow UID or GID.
+ */
+struct bpf_file_stat {
+	u64 ino;
+	u64 dev;
+	u64 mnt_id;
+	u64 size;
+	u64 attributes;
+	u64 attributes_mask;
+	u32 mode;
+	u32 uid;
+	u32 gid;
+	u32 result_mask;
+};
+
+#define BPF_FILE_STAT_FIELDS (STATX_TYPE | STATX_MODE | STATX_UID | STATX_GID | \
+			      STATX_INO | STATX_SIZE | STATX_MNT_ID | \
+			      STATX_MNT_ID_UNIQUE)
+
 __bpf_kfunc_start_defs();
 
+/**
+ * bpf_path_getattr - query attributes of a trusted path
+ * @path: resolved, trusted path
+ * @request_mask: bits from STATX_BASIC_STATS, STATX_MNT_ID and STATX_MNT_ID_UNIQUE
+ * @query_flags: AT_STATX_SYNC_AS_STAT, AT_STATX_FORCE_SYNC or AT_STATX_DONT_SYNC
+ * @out__uninit: naturally aligned result buffer
+ * @out__sz: must equal sizeof(struct bpf_file_stat)
+ *
+ * Only sleepable LSM programs may call this kfunc. The caller must not hold
+ * locks that conflict with the filesystem's getattr operation.
+ *
+ * Uses vfs_getattr_nosec() to avoid a direct security_inode_getattr() call.
+ * Results follow statx(2), including mount IDs and STATX_ATTR_MOUNT_ROOT.
+ * The mount ID is legacy unless STATX_MNT_ID_UNIQUE is requested. Fields not
+ * requested may also be returned.
+ *
+ * Return: Zero on success or a negative error code. Unsupported mask bits,
+ * query flags, size or alignment return -EINVAL. With a valid size, the output
+ * is initialized on every return; an invalid size leaves it untouched.
+ */
+__bpf_kfunc int bpf_path_getattr(const struct path *path, u32 request_mask,
+				 u32 query_flags, struct bpf_file_stat *out__uninit,
+				 u32 out__sz)
+{
+	struct bpf_file_stat *out = out__uninit;
+	struct kstat stat;
+	int ret;
+
+	if (out__sz != sizeof(*out))
+		return -EINVAL;
+	memset(out, 0, sizeof(*out));
+
+	if (!IS_ALIGNED((unsigned long)out, __alignof__(*out)))
+		return -EINVAL;
+
+	if (request_mask & ~(STATX_BASIC_STATS | STATX_MNT_ID | STATX_MNT_ID_UNIQUE))
+		return -EINVAL;
+	if (query_flags & ~AT_STATX_SYNC_TYPE ||
+	    (query_flags & AT_STATX_SYNC_TYPE) == AT_STATX_SYNC_TYPE)
+		return -EINVAL;
+
+	ret = vfs_getattr_nosec(path, &stat, request_mask, query_flags);
+	if (ret)
+		return ret;
+
+	out->ino = stat.ino;
+	out->dev = huge_encode_dev(stat.dev);
+	if (request_mask & STATX_MNT_ID_UNIQUE) {
+		out->mnt_id = real_mount(path->mnt)->mnt_id_unique;
+		stat.result_mask |= STATX_MNT_ID_UNIQUE;
+	} else {
+		out->mnt_id = real_mount(path->mnt)->mnt_id;
+		stat.result_mask |= STATX_MNT_ID;
+	}
+	out->size = stat.size;
+	out->attributes = stat.attributes & ~STATX_ATTR_CHANGE_MONOTONIC;
+	if (path_mounted(path))
+		out->attributes |= STATX_ATTR_MOUNT_ROOT;
+	out->attributes_mask = stat.attributes_mask | STATX_ATTR_MOUNT_ROOT;
+	out->mode = stat.mode;
+	out->uid = from_kuid_munged(current_user_ns(), stat.uid);
+	out->gid = from_kgid_munged(current_user_ns(), stat.gid);
+	out->result_mask = stat.result_mask & BPF_FILE_STAT_FIELDS;
+	return 0;
+}
+
 /**
  * bpf_get_task_exe_file - get a reference on the exe_file struct file member of
  *                         the mm_struct that is nested within the supplied
@@ -504,6 +608,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 +620,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

[PATCH bpf-next 2/3] docs/bpf: Document bpf_path_getattr()

[Anastasios Papagiannis]

Describe the interface, result fields, supported flags, 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..9e0e391bc007 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 with statx(2) semantics,
+including overlayfs inode and device identity. 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_file_stat bpf_path_getattr
-- 
2.56.0

[PATCH bpf-next 3/3] selftests/bpf: Test bpf_path_getattr()

[Anastasios Papagiannis]

Compare the result with statx() on tmpfs and overlayfs, including copy-up.
Cover verifier restrictions.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
---
 .../selftests/bpf/prog_tests/path_getattr.c   | 185 ++++++++++++++++++
 .../selftests/bpf/progs/path_getattr.c        |  25 +++
 .../bpf/progs/verifier_path_getattr.c         |  57 ++++++
 3 files changed, 267 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..667e42342202
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/path_getattr.c
@@ -0,0 +1,185 @@
+// 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 <sys/sysmacros.h>
+#include <test_progs.h>
+
+/* Userspace definition of the BTF-visible kfunc result. */
+struct bpf_file_stat {
+	__u64 ino;
+	__u64 dev;
+	__u64 mnt_id;
+	__u64 size;
+	__u64 attributes;
+	__u64 attributes_mask;
+	__u32 mode;
+	__u32 uid;
+	__u32 gid;
+	__u32 result_mask;
+};
+
+#include "path_getattr.skel.h"
+#include "verifier_path_getattr.skel.h"
+
+#define STAT_FIELDS (STATX_TYPE | STATX_MODE | STATX_UID | STATX_GID | \
+		     STATX_INO | STATX_SIZE | STATX_MNT_ID | STATX_MNT_ID_UNIQUE)
+
+static void query_stat(struct path_getattr *skel, const char *path)
+{
+	const struct bpf_file_stat *got = &skel->bss->output;
+	struct statx stx;
+	int err;
+
+	memset(&skel->bss->output, 0xff, sizeof(skel->bss->output));
+	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(got->ino, stx.stx_ino, "ino");
+	ASSERT_EQ(got->dev, makedev(stx.stx_dev_major, stx.stx_dev_minor), "dev");
+	ASSERT_EQ(got->mnt_id, stx.stx_mnt_id, "mnt_id");
+	ASSERT_EQ(got->size, stx.stx_size, "size");
+	ASSERT_EQ(got->mode, stx.stx_mode, "mode");
+	ASSERT_EQ(got->uid, stx.stx_uid, "uid");
+	ASSERT_EQ(got->gid, stx.stx_gid, "gid");
+	ASSERT_EQ(got->attributes, stx.stx_attributes, "attributes");
+	ASSERT_EQ(got->attributes_mask, stx.stx_attributes_mask, "attributes_mask");
+	ASSERT_EQ(got->result_mask, stx.stx_mask & STAT_FIELDS, "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];
+	__u64 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;
+	}
+	skel->bss->request_mask = STATX_BASIC_STATS | STATX_MNT_ID;
+	query_stat(skel, merged);
+	overlay_dev = skel->bss->output.dev;
+	query_stat(skel, merged_file);
+	ASSERT_NEQ(skel->bss->output.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->output.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")) {
+		skel->bss->request_mask = STATX_BASIC_STATS | STATX_MNT_ID;
+		query_stat(skel, file);
+		skel->bss->request_mask = STATX_BASIC_STATS | STATX_MNT_ID_UNIQUE;
+		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..03e9bed2a90b
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/path_getattr.c
@@ -0,0 +1,25 @@
+// 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;
+
+struct bpf_file_stat output;
+
+SEC("lsm.s/inode_getattr")
+int BPF_PROG(path_getattr, const struct path *path, int ret)
+{
+	if (ret || (__u32)bpf_get_current_pid_tgid() != monitored_tid || calls)
+		return ret;
+	calls++;
+	getattr_ret = bpf_path_getattr(path, request_mask, 0, &output, sizeof(output));
+	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..5d3d0366f4f0
--- /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 bpf_file_stat output;
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(getattr_embedded_path_stack, struct file *file)
+{
+	struct bpf_file_stat 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

Re: [PATCH bpf-next 1/3] bpf: Add bpf_path_getattr()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2039 bytes --]

> bpf: Add bpf_path_getattr()

TL;DR: Design nit only: bpf_path_getattr() near-copies the mount ID and
mount-root handling from vfs_statx_path(), so the two may drift; suggests a
shared helper in fs/stat.c and avoiding VFS-private headers.

> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 35ccd98a4de6..95d7b0de2453 100644
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c

[ ... ]

> @@ -1,23 +1,127 @@
>  #include <linux/xattr.h>
>  
> +#include "internal.h"
> +#include "mount.h"
> +

[ ... ]

> +	if (request_mask & STATX_MNT_ID_UNIQUE) {
> +		out->mnt_id = real_mount(path->mnt)->mnt_id_unique;
> +		stat.result_mask |= STATX_MNT_ID_UNIQUE;
> +	} else {
> +		out->mnt_id = real_mount(path->mnt)->mnt_id;
> +		stat.result_mask |= STATX_MNT_ID;
> +	}
> +	out->size = stat.size;
> +	out->attributes = stat.attributes & ~STATX_ATTR_CHANGE_MONOTONIC;
> +	if (path_mounted(path))
> +		out->attributes |= STATX_ATTR_MOUNT_ROOT;
> +	out->attributes_mask = stat.attributes_mask | STATX_ATTR_MOUNT_ROOT;

this isn't a bug, but could the mount ID and STATX_ATTR_MOUNT_ROOT handling
be shared with vfs_statx_path() through a small helper in fs/stat.c?

This block looks like a near-verbatim copy of the post-getattr part of
vfs_statx_path(), plus the STATX_ATTR_CHANGE_MONOTONIC masking from
cp_statx().

The kernel-doc says 'Results follow statx(2)', so a later change to
vfs_statx_path() (a new mount attribute, a different mount ID rule) would
have to be mirrored here by hand.

The copy is also why this file now includes the VFS-private headers:

	#include "internal.h"
	#include "mount.h"

just to reach real_mount() and path_mounted().  A helper in fs/stat.c that
applies the mount ID and mount-root fields to a kstat would keep the two
in sync and keep struct mount inside fs/.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40857522501