linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 bpf-next 0/5] bpf path iterator
@ 2025-06-11 22:02 Song Liu
  2025-06-11 22:02 ` [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent() Song Liu
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Song Liu @ 2025-06-11 22:02 UTC (permalink / raw)
  To: bpf, linux-fsdevel, linux-kernel, linux-security-module
  Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, m, neil, Song Liu

In security use cases, it is common to apply rules to VFS subtrees.
However, filtering files in a subtree is not straightforward [1].

One solution to this problem is to start from a path and walk up the VFS
tree (towards the root). Among in-tree LSMs, Landlock uses this solution.

BPF LSM solutions, such like Tetragon [2], also use similar approaches.
However, due to lack of proper helper/kfunc support, BPF LSM solutions
usually do the path walk with probe read, which is racy.

This patchset introduces a new helper path_walk_parent, which walks
path to its VFS parent. The helper is used in Landlock.

A new BPF iterator, path iterator, is introduced to do the path walking.
The BPF path iterator uses the new path_walk_parent help to walk the VFS
tree.

Changes v3 => v4:
1. Rewrite path_walk_parent based on suggestion from Neil Brown.
2. When path_walk_parent() returns false, it call path_put on @path and
   then zeros @path.

v3: https://lore.kernel.org/bpf/20250606213015.255134-1-song@kernel.org/

Changes v2 => v3:
1. Fix an issue with path_walk_parent.
2. Move bpf path iterator to fs/bpf_fs_kfuncs.c
3. Optimize bpf path iterator (less memory).
4. Add more selftests.
5. Add more comments.

v2: https://lore.kernel.org/bpf/20250603065920.3404510-1-song@kernel.org/

Changes v1 => v2:
1. Rename path_parent => path_walk_parent.
2. Remove path_connected check in path_walk_parent.
3. Fix is_access_to_paths_allowed().
4. Remove mode for path iterator, add a flag instead.

v1: https://lore.kernel.org/bpf/20250528222623.1373000-1-song@kernel.org/

[1] https://lpc.events/event/18/contributions/1940/
[2] https://github.com/cilium/tetragon/

Song Liu (5):
  namei: Introduce new helper function path_walk_parent()
  landlock: Use path_walk_parent()
  bpf: Introduce path iterator
  selftests/bpf: Add tests for bpf path iterator
  selftests/bpf: Path walk test

 fs/bpf_fs_kfuncs.c                            |  72 +++++++++
 fs/namei.c                                    |  99 ++++++++++--
 include/linux/namei.h                         |   2 +
 kernel/bpf/verifier.c                         |   5 +
 security/landlock/fs.c                        |  28 +---
 .../testing/selftests/bpf/bpf_experimental.h  |   6 +
 .../selftests/bpf/prog_tests/path_iter.c      | 111 ++++++++++++++
 tools/testing/selftests/bpf/progs/path_iter.c | 145 ++++++++++++++++++
 tools/testing/selftests/bpf/progs/path_walk.c |  59 +++++++
 9 files changed, 493 insertions(+), 34 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/path_iter.c
 create mode 100644 tools/testing/selftests/bpf/progs/path_iter.c
 create mode 100644 tools/testing/selftests/bpf/progs/path_walk.c

--
2.47.1

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

* [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent()
  2025-06-11 22:02 [PATCH v4 bpf-next 0/5] bpf path iterator Song Liu
@ 2025-06-11 22:02 ` Song Liu
  2025-06-13  0:10   ` NeilBrown
  2025-06-14 18:36   ` Tingmao Wang
  2025-06-11 22:02 ` [PATCH v4 bpf-next 2/5] landlock: Use path_walk_parent() Song Liu
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Song Liu @ 2025-06-11 22:02 UTC (permalink / raw)
  To: bpf, linux-fsdevel, linux-kernel, linux-security-module
  Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, m, neil, Song Liu

This helper walks an input path to its parent. Logic are added to handle
walking across mount tree.

This will be used by landlock, and BPF LSM.

Suggested-by: Neil Brown <neil@brown.name>
Signed-off-by: Song Liu <song@kernel.org>
---
 fs/namei.c            | 99 +++++++++++++++++++++++++++++++++++++------
 include/linux/namei.h |  2 +
 2 files changed, 87 insertions(+), 14 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 4bb889fc980b..bc65361c5d13 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2048,36 +2048,107 @@ static struct dentry *follow_dotdot_rcu(struct nameidata *nd)
 	return nd->path.dentry;
 }
 
-static struct dentry *follow_dotdot(struct nameidata *nd)
+/**
+ * __path_walk_parent - Find the parent of the given struct path
+ * @path  - The struct path to start from
+ * @root  - A struct path which serves as a boundary not to be crosses.
+ *        - If @root is zero'ed, walk all the way to global root.
+ * @flags - Some LOOKUP_ flags.
+ *
+ * Find and return the dentry for the parent of the given path
+ * (mount/dentry). If the given path is the root of a mounted tree, it
+ * is first updated to the mount point on which that tree is mounted.
+ *
+ * If %LOOKUP_NO_XDEV is given, then *after* the path is updated to a new
+ * mount, the error EXDEV is returned.
+ *
+ * If no parent can be found, either because the tree is not mounted or
+ * because the @path matches the @root, then @path->dentry is returned
+ * unless @flags contains %LOOKUP_BENEATH, in which case -EXDEV is returned.
+ *
+ * Returns: either an ERR_PTR() or the chosen parent which will have had
+ * the refcount incremented.
+ */
+static struct dentry *__path_walk_parent(struct path *path, const struct path *root, int flags)
 {
 	struct dentry *parent;
 
-	if (path_equal(&nd->path, &nd->root))
+	if (path_equal(path, root))
 		goto in_root;
-	if (unlikely(nd->path.dentry == nd->path.mnt->mnt_root)) {
-		struct path path;
+	if (unlikely(path->dentry == path->mnt->mnt_root)) {
+		struct path new_path;
 
-		if (!choose_mountpoint(real_mount(nd->path.mnt),
-				       &nd->root, &path))
+		if (!choose_mountpoint(real_mount(path->mnt),
+				       root, &new_path))
 			goto in_root;
-		path_put(&nd->path);
-		nd->path = path;
-		nd->inode = path.dentry->d_inode;
-		if (unlikely(nd->flags & LOOKUP_NO_XDEV))
+		path_put(path);
+		*path = new_path;
+		if (unlikely(flags & LOOKUP_NO_XDEV))
 			return ERR_PTR(-EXDEV);
 	}
 	/* rare case of legitimate dget_parent()... */
-	parent = dget_parent(nd->path.dentry);
-	if (unlikely(!path_connected(nd->path.mnt, parent))) {
+	parent = dget_parent(path->dentry);
+	if (unlikely(!path_connected(path->mnt, parent))) {
 		dput(parent);
 		return ERR_PTR(-ENOENT);
 	}
 	return parent;
 
 in_root:
-	if (unlikely(nd->flags & LOOKUP_BENEATH))
+	if (unlikely(flags & LOOKUP_BENEATH))
 		return ERR_PTR(-EXDEV);
-	return dget(nd->path.dentry);
+	return dget(path->dentry);
+}
+
+/**
+ * path_walk_parent - Walk to the parent of path
+ * @path: input and output path.
+ * @root: root of the path walk, do not go beyond this root. If @root is
+ *        zero'ed, walk all the way to real root.
+ *
+ * Given a path, find the parent path. Replace @path with the parent path.
+ * If we were already at the real root or a disconnected root, @path is
+ * released and zero'ed.
+ *
+ * Returns:
+ *  true  - if @path is updated to its parent.
+ *  false - if @path is already the root (real root or @root).
+ */
+bool path_walk_parent(struct path *path, const struct path *root)
+{
+	struct dentry *parent;
+
+	parent = __path_walk_parent(path, root, LOOKUP_BENEATH);
+
+	if (IS_ERR(parent))
+		goto false_out;
+
+	if (parent == path->dentry) {
+		dput(parent);
+		goto false_out;
+	}
+	dput(path->dentry);
+	path->dentry = parent;
+	return true;
+
+false_out:
+	path_put(path);
+	memset(path, 0, sizeof(*path));
+	return false;
+}
+
+static struct dentry *follow_dotdot(struct nameidata *nd)
+{
+	struct dentry *parent = __path_walk_parent(&nd->path, &nd->root, nd->flags);
+
+	if (IS_ERR(parent))
+		return parent;
+	if (unlikely(!path_connected(nd->path.mnt, parent))) {
+		dput(parent);
+		return ERR_PTR(-ENOENT);
+	}
+	nd->inode = nd->path.dentry->d_inode;
+	return parent;
 }
 
 static const char *handle_dots(struct nameidata *nd, int type)
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 5d085428e471..cba5373ecf86 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -85,6 +85,8 @@ extern int follow_down_one(struct path *);
 extern int follow_down(struct path *path, unsigned int flags);
 extern int follow_up(struct path *);
 
+bool path_walk_parent(struct path *path, const struct path *root);
+
 extern struct dentry *lock_rename(struct dentry *, struct dentry *);
 extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
 extern void unlock_rename(struct dentry *, struct dentry *);
-- 
2.47.1


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

* [PATCH v4 bpf-next 2/5] landlock: Use path_walk_parent()
  2025-06-11 22:02 [PATCH v4 bpf-next 0/5] bpf path iterator Song Liu
  2025-06-11 22:02 ` [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent() Song Liu
@ 2025-06-11 22:02 ` Song Liu
  2025-06-11 22:02 ` [PATCH v4 bpf-next 3/5] bpf: Introduce path iterator Song Liu
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Song Liu @ 2025-06-11 22:02 UTC (permalink / raw)
  To: bpf, linux-fsdevel, linux-kernel, linux-security-module
  Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, m, neil, Song Liu

Use path_walk_parent() to walk a path up to its parent.

No functional changes intended.

Signed-off-by: Song Liu <song@kernel.org>
---
 security/landlock/fs.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 6fee7c20f64d..63232199ce23 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -837,8 +837,8 @@ static bool is_access_to_paths_allowed(
 	 * restriction.
 	 */
 	while (true) {
-		struct dentry *parent_dentry;
 		const struct landlock_rule *rule;
+		struct path root = {};
 
 		/*
 		 * If at least all accesses allowed on the destination are
@@ -895,35 +895,23 @@ static bool is_access_to_paths_allowed(
 		/* Stops when a rule from each layer grants access. */
 		if (allowed_parent1 && allowed_parent2)
 			break;
-jump_up:
-		if (walker_path.dentry == walker_path.mnt->mnt_root) {
-			if (follow_up(&walker_path)) {
-				/* Ignores hidden mount points. */
-				goto jump_up;
-			} else {
-				/*
-				 * Stops at the real root.  Denies access
-				 * because not all layers have granted access.
-				 */
-				break;
-			}
-		}
-		if (unlikely(IS_ROOT(walker_path.dentry))) {
+
+		if (unlikely(IS_ROOT(walker_path.dentry)) &&
+		    (walker_path.mnt->mnt_flags & MNT_INTERNAL)) {
 			/*
 			 * Stops at disconnected root directories.  Only allows
 			 * access to internal filesystems (e.g. nsfs, which is
 			 * reachable through /proc/<pid>/ns/<namespace>).
 			 */
-			if (walker_path.mnt->mnt_flags & MNT_INTERNAL) {
 			allowed_parent1 = true;
 			allowed_parent2 = true;
-			}
 			break;
 		}
-		parent_dentry = dget_parent(walker_path.dentry);
-		dput(walker_path.dentry);
-		walker_path.dentry = parent_dentry;
+		if (!path_walk_parent(&walker_path, &root))
+			break;
 	}
+
+	if (walker_path.dentry)
 		path_put(&walker_path);
 
 	if (!allowed_parent1) {
-- 
2.47.1


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

* [PATCH v4 bpf-next 3/5] bpf: Introduce path iterator
  2025-06-11 22:02 [PATCH v4 bpf-next 0/5] bpf path iterator Song Liu
  2025-06-11 22:02 ` [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent() Song Liu
  2025-06-11 22:02 ` [PATCH v4 bpf-next 2/5] landlock: Use path_walk_parent() Song Liu
@ 2025-06-11 22:02 ` Song Liu
  2025-06-12 23:11   ` Andrii Nakryiko
  2025-06-11 22:02 ` [PATCH v4 bpf-next 4/5] selftests/bpf: Add tests for bpf " Song Liu
  2025-06-11 22:02 ` [PATCH v4 bpf-next 5/5] selftests/bpf: Path walk test Song Liu
  4 siblings, 1 reply; 11+ messages in thread
From: Song Liu @ 2025-06-11 22:02 UTC (permalink / raw)
  To: bpf, linux-fsdevel, linux-kernel, linux-security-module
  Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, m, neil, Song Liu

Introduce a path iterator, which walks a struct path toward the root.
This path iterator is based on path_walk_parent. A fixed zero'ed root
is passed to path_walk_parent(). Therefore, unless the user terminates
it earlier, the iterator will terminate at the real root.

Signed-off-by: Song Liu <song@kernel.org>
---
 fs/bpf_fs_kfuncs.c    | 72 +++++++++++++++++++++++++++++++++++++++++++
 kernel/bpf/verifier.c |  5 +++
 2 files changed, 77 insertions(+)

diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 08412532db1b..1e7e94738c2b 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -10,6 +10,7 @@
 #include <linux/fsnotify.h>
 #include <linux/file.h>
 #include <linux/mm.h>
+#include <linux/namei.h>
 #include <linux/xattr.h>
 
 __bpf_kfunc_start_defs();
@@ -324,6 +325,74 @@ __bpf_kfunc int bpf_remove_dentry_xattr(struct dentry *dentry, const char *name_
 
 __bpf_kfunc_end_defs();
 
+/* open-coded path iterator */
+struct bpf_iter_path {
+	__u64 __opaque[2];
+} __aligned(8);
+
+struct bpf_iter_path_kern {
+	struct path path;
+} __aligned(8);
+
+__bpf_kfunc_start_defs();
+
+__bpf_kfunc int bpf_iter_path_new(struct bpf_iter_path *it,
+				  struct path *start,
+				  __u64 flags)
+{
+	struct bpf_iter_path_kern *kit = (void *)it;
+
+	BUILD_BUG_ON(sizeof(*kit) > sizeof(*it));
+	BUILD_BUG_ON(__alignof__(*kit) != __alignof__(*it));
+
+	if (flags) {
+		/*
+		 * _destroy() is still called when _new() fails. Zero
+		 * kit->path so that it be passed to path_put() safely.
+		 * Note: path_put() is no-op for zero'ed path.
+		 */
+		memset(&kit->path, 0, sizeof(struct path));
+		return -EINVAL;
+	}
+
+	kit->path = *start;
+	path_get(&kit->path);
+
+	return 0;
+}
+
+__bpf_kfunc struct path *bpf_iter_path_next(struct bpf_iter_path *it)
+{
+	struct bpf_iter_path_kern *kit = (void *)it;
+	struct path root = {};
+
+	/*
+	 * "root" is zero'ed. Therefore, unless the loop is explicitly
+	 * terminated, bpf_iter_path_next() will continue looping until
+	 * we've reached the global root of the VFS.
+	 *
+	 * If a root of walk is needed, the user can check "path" against
+	 * that root on each iteration.
+	 */
+	if (!path_walk_parent(&kit->path, &root))
+		return NULL;
+
+	return &kit->path;
+}
+
+__bpf_kfunc void bpf_iter_path_destroy(struct bpf_iter_path *it)
+{
+	struct bpf_iter_path_kern *kit = (void *)it;
+
+	/*
+	 * kit->path might be zero'ed, but this is OK because path_put()
+	 * is no-op for zero'ed struct path
+	 */
+	path_put(&kit->path);
+}
+
+__bpf_kfunc_end_defs();
+
 BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
 BTF_ID_FLAGS(func, bpf_get_task_exe_file,
 	     KF_ACQUIRE | KF_TRUSTED_ARGS | KF_RET_NULL)
@@ -333,6 +402,9 @@ BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_iter_path_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_iter_path_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_iter_path_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
 BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
 
 static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b1f797616f20..9b5ac7c02867 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7049,6 +7049,10 @@ BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct socket) {
 	struct sock *sk;
 };
 
+BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct path) {
+	struct dentry *dentry;
+};
+
 static bool type_is_rcu(struct bpf_verifier_env *env,
 			struct bpf_reg_state *reg,
 			const char *field_name, u32 btf_id)
@@ -7089,6 +7093,7 @@ static bool type_is_trusted_or_null(struct bpf_verifier_env *env,
 				    const char *field_name, u32 btf_id)
 {
 	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct socket));
+	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct path));
 
 	return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id,
 					  "__safe_trusted_or_null");
-- 
2.47.1


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

* [PATCH v4 bpf-next 4/5] selftests/bpf: Add tests for bpf path iterator
  2025-06-11 22:02 [PATCH v4 bpf-next 0/5] bpf path iterator Song Liu
                   ` (2 preceding siblings ...)
  2025-06-11 22:02 ` [PATCH v4 bpf-next 3/5] bpf: Introduce path iterator Song Liu
@ 2025-06-11 22:02 ` Song Liu
  2025-06-11 22:02 ` [PATCH v4 bpf-next 5/5] selftests/bpf: Path walk test Song Liu
  4 siblings, 0 replies; 11+ messages in thread
From: Song Liu @ 2025-06-11 22:02 UTC (permalink / raw)
  To: bpf, linux-fsdevel, linux-kernel, linux-security-module
  Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, m, neil, Song Liu

Add tests for bpf path iterator, including test cases similar to real
workload (call bpf_path_d_path and bpf_get_dentry_xattr), and test cases
where the verifier rejects invalid use of the iterator.

Signed-off-by: Song Liu <song@kernel.org>
---
 .../testing/selftests/bpf/bpf_experimental.h  |   6 +
 .../selftests/bpf/prog_tests/path_iter.c      |  12 ++
 tools/testing/selftests/bpf/progs/path_iter.c | 145 ++++++++++++++++++
 3 files changed, 163 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/path_iter.c
 create mode 100644 tools/testing/selftests/bpf/progs/path_iter.c

diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 5e512a1d09d1..cbb759b473df 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -596,4 +596,10 @@ extern int bpf_iter_dmabuf_new(struct bpf_iter_dmabuf *it) __weak __ksym;
 extern struct dma_buf *bpf_iter_dmabuf_next(struct bpf_iter_dmabuf *it) __weak __ksym;
 extern void bpf_iter_dmabuf_destroy(struct bpf_iter_dmabuf *it) __weak __ksym;
 
+struct bpf_iter_path;
+extern int bpf_iter_path_new(struct bpf_iter_path *it, struct path *start,
+			     __u64 flags) __weak __ksym;
+extern struct path *bpf_iter_path_next(struct bpf_iter_path *it) __weak __ksym;
+extern void bpf_iter_path_destroy(struct bpf_iter_path *it) __weak __ksym;
+
 #endif
diff --git a/tools/testing/selftests/bpf/prog_tests/path_iter.c b/tools/testing/selftests/bpf/prog_tests/path_iter.c
new file mode 100644
index 000000000000..3c99c24fbd96
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/path_iter.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include <test_progs.h>
+#include <bpf/libbpf.h>
+#include <bpf/btf.h>
+#include "path_iter.skel.h"
+
+void test_path_iter(void)
+{
+	RUN_TESTS(path_iter);
+}
diff --git a/tools/testing/selftests/bpf/progs/path_iter.c b/tools/testing/selftests/bpf/progs/path_iter.c
new file mode 100644
index 000000000000..74d0f4e19ffa
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/path_iter.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+char _license[] SEC("license") = "GPL";
+
+char path_name[256];
+char xattr_val[64];
+
+static __always_inline void access_path_dentry(struct path *p)
+{
+	struct bpf_dynptr ptr;
+	struct dentry *dentry;
+
+	if (!p)
+		return;
+
+	bpf_dynptr_from_mem(xattr_val, sizeof(xattr_val), 0, &ptr);
+	bpf_path_d_path(p, path_name, sizeof(path_name));
+
+	dentry = p->dentry;
+	if (dentry)
+		bpf_get_dentry_xattr(dentry, "user.xattr", &ptr);
+}
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(open_code, struct file *f)
+{
+	struct bpf_iter_path path_it;
+	struct path *p;
+	int ret;
+
+	ret = bpf_iter_path_new(&path_it, &f->f_path, 0);
+	if (ret) {
+		bpf_iter_path_destroy(&path_it);
+		return 0;
+	}
+
+	p = bpf_iter_path_next(&path_it);
+	access_path_dentry(p);
+	bpf_iter_path_destroy(&path_it);
+
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(for_each, struct file *f)
+{
+	struct path *p;
+
+	bpf_for_each(path, p, &f->f_path, 0)
+		access_path_dentry(p);
+
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("Unreleased reference")
+int BPF_PROG(missing_destroy, struct file *f)
+{
+	struct bpf_iter_path path_it;
+
+	bpf_iter_path_new(&path_it, &f->f_path, 0);
+
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("expected an initialized iter_path")
+int BPF_PROG(missing_new, struct file *f)
+{
+	struct bpf_iter_path path_it;
+
+	bpf_iter_path_destroy(&path_it);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("expected uninitialized iter_path")
+int BPF_PROG(new_twice, struct file *f)
+{
+	struct bpf_iter_path path_it;
+
+	bpf_iter_path_new(&path_it, &f->f_path, 0);
+	bpf_iter_path_new(&path_it, &f->f_path, 0);
+	bpf_iter_path_destroy(&path_it);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("expected an initialized iter_path")
+int BPF_PROG(destroy_twice, struct file *f)
+{
+	struct bpf_iter_path path_it;
+
+	bpf_iter_path_new(&path_it, &f->f_path, 0);
+	bpf_iter_path_destroy(&path_it);
+	bpf_iter_path_destroy(&path_it);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(reuse_path_iter, struct file *f)
+{
+	struct bpf_iter_path path_it;
+
+	bpf_iter_path_new(&path_it, &f->f_path, 0);
+	bpf_iter_path_destroy(&path_it);
+	bpf_iter_path_new(&path_it, &f->f_path, 0);
+	bpf_iter_path_destroy(&path_it);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("invalid read from stack off")
+int BPF_PROG(invalid_read_path_iter, struct file *f)
+{
+	struct bpf_iter_path path_it;
+	struct bpf_iter_path path_it_2;
+
+
+	bpf_iter_path_new(&path_it, &f->f_path, 0);
+	path_it_2 = path_it;
+	bpf_iter_path_destroy(&path_it_2);
+	return 0;
+}
+
+SEC("lsm.s/sb_alloc_security")
+__failure __msg("must be referenced or trusted")
+int BPF_PROG(untrusted_path, struct super_block *sb)
+{
+	struct bpf_iter_path path_it;
+
+	bpf_iter_path_new(&path_it, &sb->s_bdev_file->f_path, 0);
+	bpf_iter_path_destroy(&path_it);
+	return 0;
+}
-- 
2.47.1


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

* [PATCH v4 bpf-next 5/5] selftests/bpf: Path walk test
  2025-06-11 22:02 [PATCH v4 bpf-next 0/5] bpf path iterator Song Liu
                   ` (3 preceding siblings ...)
  2025-06-11 22:02 ` [PATCH v4 bpf-next 4/5] selftests/bpf: Add tests for bpf " Song Liu
@ 2025-06-11 22:02 ` Song Liu
  4 siblings, 0 replies; 11+ messages in thread
From: Song Liu @ 2025-06-11 22:02 UTC (permalink / raw)
  To: bpf, linux-fsdevel, linux-kernel, linux-security-module
  Cc: kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, m, neil, Song Liu

Add an end-to-end test with path_iter on security hook file_open.

A test file is created in folder /tmp/test_progs_path_iter/folder. On
file_open, walk file->f_path up to its parent and grand parent, and test
bpf_get_dentry_xattr and bpf_path_d_path on the folders.

Signed-off-by: Song Liu <song@kernel.org>
---
 .../selftests/bpf/prog_tests/path_iter.c      | 99 +++++++++++++++++++
 tools/testing/selftests/bpf/progs/path_walk.c | 59 +++++++++++
 2 files changed, 158 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/path_walk.c

diff --git a/tools/testing/selftests/bpf/prog_tests/path_iter.c b/tools/testing/selftests/bpf/prog_tests/path_iter.c
index 3c99c24fbd96..b9772026fbf7 100644
--- a/tools/testing/selftests/bpf/prog_tests/path_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/path_iter.c
@@ -2,11 +2,110 @@
 /* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
 
 #include <test_progs.h>
+#include <fcntl.h>
 #include <bpf/libbpf.h>
 #include <bpf/btf.h>
+#include <sys/stat.h>
+#include <sys/xattr.h>
+
 #include "path_iter.skel.h"
+#include "path_walk.skel.h"
+
+static const char grand_parent_path[] = "/tmp/test_progs_path_iter";
+static const char parent_path[] = "/tmp/test_progs_path_iter/folder";
+static const char file_path[] = "/tmp/test_progs_path_iter/folder/file";
+static const char xattr_name[] = "user.bpf.selftests";
+static const char xattr_value[] = "selftest_path_iter";
+
+static void cleanup_files(void)
+{
+	remove(file_path);
+	rmdir(parent_path);
+	rmdir(grand_parent_path);
+}
+
+static int setup_files_and_xattrs(void)
+{
+	int ret = -1;
+
+	/* create test folders */
+	if (mkdir(grand_parent_path, 0755))
+		goto error;
+	if (mkdir(parent_path, 0755))
+		goto error;
+
+	/* setxattr for test folders */
+	ret = setxattr(grand_parent_path, xattr_name,
+		       xattr_value, sizeof(xattr_value), 0);
+	if (ret < 0) {
+		/* return errno, so that we can handle EOPNOTSUPP in the caller */
+		ret = errno;
+		goto error;
+	}
+	ret = setxattr(parent_path, xattr_name,
+		       xattr_value, sizeof(xattr_value), 0);
+	if (ret < 0) {
+		/* return errno, so that we can handle EOPNOTSUPP in the caller */
+		ret = errno;
+		goto error;
+	}
+
+	return 0;
+error:
+	cleanup_files();
+	return ret;
+}
+
+static void test_path_walk(void)
+{
+	struct path_walk *skel = NULL;
+	int file_fd;
+	int err;
+
+	err = setup_files_and_xattrs();
+	if (err == EOPNOTSUPP) {
+		printf("%s:SKIP:local fs doesn't support xattr (%d)\n"
+		       "To run this test, make sure /tmp filesystem supports xattr.\n",
+		       __func__, errno);
+		test__skip();
+		return;
+	}
+
+	if (!ASSERT_OK(err, "setup_file"))
+		return;
+
+	skel = path_walk__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "path_walk__open_and_load"))
+		goto cleanup;
+
+	skel->bss->monitored_pid = getpid();
+	if (!ASSERT_OK(path_walk__attach(skel), "path_walk__attach"))
+		goto cleanup;
+
+	file_fd = open(file_path, O_CREAT);
+	if (!ASSERT_OK_FD(file_fd, "open_file"))
+		goto cleanup;
+	close(file_fd);
+
+	ASSERT_OK(strncmp(skel->bss->parent_xattr_buf, xattr_value, strlen(xattr_value)),
+		  "parent_xattr");
+	ASSERT_OK(strncmp(skel->bss->grand_parent_xattr_buf, xattr_value, strlen(xattr_value)),
+		  "grand_parent_xattr");
+
+	ASSERT_OK(strncmp(skel->bss->parent_path_buf, parent_path, strlen(parent_path)),
+		  "parent_d_path");
+	ASSERT_OK(strncmp(skel->bss->grand_parent_path_buf, grand_parent_path,
+			  strlen(grand_parent_path)),
+		  "grand_parent_d_path");
+
+cleanup:
+	path_walk__destroy(skel);
+	cleanup_files();
+}
 
 void test_path_iter(void)
 {
 	RUN_TESTS(path_iter);
+	if (test__start_subtest("path_walk_example"))
+		test_path_walk();
 }
diff --git a/tools/testing/selftests/bpf/progs/path_walk.c b/tools/testing/selftests/bpf/progs/path_walk.c
new file mode 100644
index 000000000000..1e1ae82b47a2
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/path_walk.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include "vmlinux.h"
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
+#include "bpf_kfuncs.h"
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+__u32 monitored_pid;
+
+#define BUF_SIZE 1024
+char parent_path_buf[BUF_SIZE] = {};
+char parent_xattr_buf[BUF_SIZE] = {};
+char grand_parent_path_buf[BUF_SIZE] = {};
+char grand_parent_xattr_buf[BUF_SIZE] = {};
+
+static __always_inline void d_path_and_read_xattr(struct path *p, char *path, char *xattr)
+{
+	struct bpf_dynptr ptr;
+	struct dentry *dentry;
+
+	if (!p)
+		return;
+	bpf_path_d_path(p, path, BUF_SIZE);
+	bpf_dynptr_from_mem(xattr, BUF_SIZE, 0, &ptr);
+	dentry = p->dentry;
+	if (dentry)
+		bpf_get_dentry_xattr(dentry, "user.bpf.selftests", &ptr);
+}
+
+SEC("lsm.s/file_open")
+int BPF_PROG(test_file_open, struct file *f)
+{
+	__u32 pid = bpf_get_current_pid_tgid() >> 32;
+	struct bpf_iter_path path_it;
+	struct path *p;
+
+	if (pid != monitored_pid)
+		return 0;
+
+	bpf_iter_path_new(&path_it, &f->f_path, 0);
+
+	/* Get d_path and xattr for the parent directory */
+	p = bpf_iter_path_next(&path_it);
+	d_path_and_read_xattr(p, parent_path_buf, parent_xattr_buf);
+
+	/* Get d_path and xattr for the grand parent directory */
+	p = bpf_iter_path_next(&path_it);
+	d_path_and_read_xattr(p, grand_parent_path_buf, grand_parent_xattr_buf);
+
+	bpf_iter_path_destroy(&path_it);
+
+	return 0;
+}
-- 
2.47.1


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

* Re: [PATCH v4 bpf-next 3/5] bpf: Introduce path iterator
  2025-06-11 22:02 ` [PATCH v4 bpf-next 3/5] bpf: Introduce path iterator Song Liu
@ 2025-06-12 23:11   ` Andrii Nakryiko
  0 siblings, 0 replies; 11+ messages in thread
From: Andrii Nakryiko @ 2025-06-12 23:11 UTC (permalink / raw)
  To: Song Liu
  Cc: bpf, linux-fsdevel, linux-kernel, linux-security-module,
	kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, m, neil

On Wed, Jun 11, 2025 at 3:02 PM Song Liu <song@kernel.org> wrote:
>
> Introduce a path iterator, which walks a struct path toward the root.
> This path iterator is based on path_walk_parent. A fixed zero'ed root
> is passed to path_walk_parent(). Therefore, unless the user terminates
> it earlier, the iterator will terminate at the real root.
>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
>  fs/bpf_fs_kfuncs.c    | 72 +++++++++++++++++++++++++++++++++++++++++++
>  kernel/bpf/verifier.c |  5 +++
>  2 files changed, 77 insertions(+)
>

LGTM from BPF and open-coded contract POV.

Acked-by: Andrii Nakryiko <andrii@kernel.org>

[...]

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

* Re: [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent()
  2025-06-11 22:02 ` [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent() Song Liu
@ 2025-06-13  0:10   ` NeilBrown
  2025-06-13 22:27     ` Song Liu
  2025-06-14 18:36   ` Tingmao Wang
  1 sibling, 1 reply; 11+ messages in thread
From: NeilBrown @ 2025-06-13  0:10 UTC (permalink / raw)
  To: Song Liu
  Cc: bpf, linux-fsdevel, linux-kernel, linux-security-module,
	kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, m, Song Liu

On Thu, 12 Jun 2025, Song Liu wrote:
> This helper walks an input path to its parent. Logic are added to handle
> walking across mount tree.
> 
> This will be used by landlock, and BPF LSM.
> 
> Suggested-by: Neil Brown <neil@brown.name>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
>  fs/namei.c            | 99 +++++++++++++++++++++++++++++++++++++------
>  include/linux/namei.h |  2 +
>  2 files changed, 87 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 4bb889fc980b..bc65361c5d13 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2048,36 +2048,107 @@ static struct dentry *follow_dotdot_rcu(struct nameidata *nd)
>  	return nd->path.dentry;
>  }
>  
> -static struct dentry *follow_dotdot(struct nameidata *nd)
> +/**
> + * __path_walk_parent - Find the parent of the given struct path
> + * @path  - The struct path to start from
> + * @root  - A struct path which serves as a boundary not to be crosses.
> + *        - If @root is zero'ed, walk all the way to global root.
> + * @flags - Some LOOKUP_ flags.
> + *
> + * Find and return the dentry for the parent of the given path
> + * (mount/dentry). If the given path is the root of a mounted tree, it
> + * is first updated to the mount point on which that tree is mounted.
> + *
> + * If %LOOKUP_NO_XDEV is given, then *after* the path is updated to a new
> + * mount, the error EXDEV is returned.
> + *
> + * If no parent can be found, either because the tree is not mounted or
> + * because the @path matches the @root, then @path->dentry is returned
> + * unless @flags contains %LOOKUP_BENEATH, in which case -EXDEV is returned.
> + *
> + * Returns: either an ERR_PTR() or the chosen parent which will have had
> + * the refcount incremented.
> + */
> +static struct dentry *__path_walk_parent(struct path *path, const struct path *root, int flags)
>  {
>  	struct dentry *parent;
>  
> -	if (path_equal(&nd->path, &nd->root))
> +	if (path_equal(path, root))
>  		goto in_root;
> -	if (unlikely(nd->path.dentry == nd->path.mnt->mnt_root)) {
> -		struct path path;
> +	if (unlikely(path->dentry == path->mnt->mnt_root)) {
> +		struct path new_path;
>  
> -		if (!choose_mountpoint(real_mount(nd->path.mnt),
> -				       &nd->root, &path))
> +		if (!choose_mountpoint(real_mount(path->mnt),
> +				       root, &new_path))
>  			goto in_root;
> -		path_put(&nd->path);
> -		nd->path = path;
> -		nd->inode = path.dentry->d_inode;
> -		if (unlikely(nd->flags & LOOKUP_NO_XDEV))
> +		path_put(path);
> +		*path = new_path;
> +		if (unlikely(flags & LOOKUP_NO_XDEV))
>  			return ERR_PTR(-EXDEV);
>  	}
>  	/* rare case of legitimate dget_parent()... */
> -	parent = dget_parent(nd->path.dentry);
> -	if (unlikely(!path_connected(nd->path.mnt, parent))) {
> +	parent = dget_parent(path->dentry);
> +	if (unlikely(!path_connected(path->mnt, parent))) {
>  		dput(parent);
>  		return ERR_PTR(-ENOENT);
>  	}
>  	return parent;
>  
>  in_root:
> -	if (unlikely(nd->flags & LOOKUP_BENEATH))
> +	if (unlikely(flags & LOOKUP_BENEATH))
>  		return ERR_PTR(-EXDEV);
> -	return dget(nd->path.dentry);
> +	return dget(path->dentry);
> +}
> +
> +/**
> + * path_walk_parent - Walk to the parent of path
> + * @path: input and output path.
> + * @root: root of the path walk, do not go beyond this root. If @root is
> + *        zero'ed, walk all the way to real root.
> + *
> + * Given a path, find the parent path. Replace @path with the parent path.
> + * If we were already at the real root or a disconnected root, @path is
> + * released and zero'ed.
> + *
> + * Returns:
> + *  true  - if @path is updated to its parent.
> + *  false - if @path is already the root (real root or @root).
> + */
> +bool path_walk_parent(struct path *path, const struct path *root)
> +{
> +	struct dentry *parent;
> +
> +	parent = __path_walk_parent(path, root, LOOKUP_BENEATH);
> +
> +	if (IS_ERR(parent))
> +		goto false_out;
> +
> +	if (parent == path->dentry) {
> +		dput(parent);
> +		goto false_out;
> +	}
> +	dput(path->dentry);
> +	path->dentry = parent;
> +	return true;
> +
> +false_out:
> +	path_put(path);
> +	memset(path, 0, sizeof(*path));
> +	return false;
> +}

I think the public function should return 0 on success and -error on
failure.  That is a well established pattern.  I also think you
shouldn't assume that all callers will want the same flags.

And it isn't clear to me why you want to path_put() on failure.

I wonder if there might be other potential users in the kernel.
If so we should consider how well the interface meets their needs.

autofs, devpts, nfsd, landlock all call follow_up...
maybe they should be using the new interface...
nfsd is the most likely to benefit - particularly nfsd_lookup_parent().

Just a thought..

NeilBrown



> +
> +static struct dentry *follow_dotdot(struct nameidata *nd)
> +{
> +	struct dentry *parent = __path_walk_parent(&nd->path, &nd->root, nd->flags);
> +
> +	if (IS_ERR(parent))
> +		return parent;
> +	if (unlikely(!path_connected(nd->path.mnt, parent))) {
> +		dput(parent);
> +		return ERR_PTR(-ENOENT);
> +	}
> +	nd->inode = nd->path.dentry->d_inode;
> +	return parent;
>  }
>  
>  static const char *handle_dots(struct nameidata *nd, int type)
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 5d085428e471..cba5373ecf86 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -85,6 +85,8 @@ extern int follow_down_one(struct path *);
>  extern int follow_down(struct path *path, unsigned int flags);
>  extern int follow_up(struct path *);
>  
> +bool path_walk_parent(struct path *path, const struct path *root);
> +
>  extern struct dentry *lock_rename(struct dentry *, struct dentry *);
>  extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
>  extern void unlock_rename(struct dentry *, struct dentry *);
> -- 
> 2.47.1
> 
> 


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

* Re: [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent()
  2025-06-13  0:10   ` NeilBrown
@ 2025-06-13 22:27     ` Song Liu
  0 siblings, 0 replies; 11+ messages in thread
From: Song Liu @ 2025-06-13 22:27 UTC (permalink / raw)
  To: NeilBrown
  Cc: bpf, linux-fsdevel, linux-kernel, linux-security-module,
	kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, m

On Thu, Jun 12, 2025 at 5:11 PM NeilBrown <neil@brown.name> wrote:
[...]
> > +
> > +false_out:
> > +     path_put(path);
> > +     memset(path, 0, sizeof(*path));
> > +     return false;
> > +}
>
> I think the public function should return 0 on success and -error on
> failure.  That is a well established pattern.

Yeah, I think we can use this pattern.

> I also think you
> shouldn't assume that all callers will want the same flags.

__path_walk_parent() only handles two LOOKUP_ flags, so
it is a bit weird to allow all the flags. But if folks think this is a
good idea, I don't have strong objections to taking various flags.

>
> And it isn't clear to me why you want to path_put() on failure.

In earlier versions, we would keep "path" unchanged when the
walk stopped. However, this is not the case in this version
(choose_mountpoint() => in_root => return -EXDEV). So I
decided to just release it, so that we will not leak a path that
the walk should not get to.

>
> I wonder if there might be other potential users in the kernel.
> If so we should consider how well the interface meets their needs.
>
> autofs, devpts, nfsd, landlock all call follow_up...
> maybe they should be using the new interface...
> nfsd is the most likely to benefit - particularly nfsd_lookup_parent().

AFAICT, autofs and devpts can just use follow_up().
For nfsd, nfsd_lookup_parent() and nfsd4_encode_pathname4() can
use path_walk_parent. And 2/5 covers landlock.

I think we can update nfsd in a follow up patch, just to keep this set
simpler.

Thanks,
Song

> Just a thought..

[...]

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

* Re: [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent()
  2025-06-11 22:02 ` [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent() Song Liu
  2025-06-13  0:10   ` NeilBrown
@ 2025-06-14 18:36   ` Tingmao Wang
  2025-06-17  5:08     ` Song Liu
  1 sibling, 1 reply; 11+ messages in thread
From: Tingmao Wang @ 2025-06-14 18:36 UTC (permalink / raw)
  To: Song Liu
  Cc: bpf, linux-fsdevel, linux-kernel, linux-security-module,
	kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, neil

On 6/11/25 23:02, Song Liu wrote:
> This helper walks an input path to its parent. Logic are added to handle
> walking across mount tree.
> 
> This will be used by landlock, and BPF LSM.
> 
> Suggested-by: Neil Brown <neil@brown.name>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
>  fs/namei.c            | 99 +++++++++++++++++++++++++++++++++++++------
>  include/linux/namei.h |  2 +
>  2 files changed, 87 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 4bb889fc980b..bc65361c5d13 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2048,36 +2048,107 @@ static struct dentry *follow_dotdot_rcu(struct nameidata *nd)
>  	return nd->path.dentry;
>  }
>  
> -static struct dentry *follow_dotdot(struct nameidata *nd)
> +/**
> + * __path_walk_parent - Find the parent of the given struct path
> + * @path  - The struct path to start from
> + * @root  - A struct path which serves as a boundary not to be crosses.
> + *        - If @root is zero'ed, walk all the way to global root.
> + * @flags - Some LOOKUP_ flags.
> + *
> + * Find and return the dentry for the parent of the given path
> + * (mount/dentry). If the given path is the root of a mounted tree, it
> + * is first updated to the mount point on which that tree is mounted.
> + *
> + * If %LOOKUP_NO_XDEV is given, then *after* the path is updated to a new
> + * mount, the error EXDEV is returned.
> + *
> + * If no parent can be found, either because the tree is not mounted or
> + * because the @path matches the @root, then @path->dentry is returned
> + * unless @flags contains %LOOKUP_BENEATH, in which case -EXDEV is returned.
> + *
> + * Returns: either an ERR_PTR() or the chosen parent which will have had
> + * the refcount incremented.
> + */
> +static struct dentry *__path_walk_parent(struct path *path, const struct path *root, int flags)
>  {
>  	struct dentry *parent;
>  
> -	if (path_equal(&nd->path, &nd->root))
> +	if (path_equal(path, root))
>  		goto in_root;
> -	if (unlikely(nd->path.dentry == nd->path.mnt->mnt_root)) {
> -		struct path path;
> +	if (unlikely(path->dentry == path->mnt->mnt_root)) {
> +		struct path new_path;
>  
> -		if (!choose_mountpoint(real_mount(nd->path.mnt),
> -				       &nd->root, &path))
> +		if (!choose_mountpoint(real_mount(path->mnt),
> +				       root, &new_path))
>  			goto in_root;
> -		path_put(&nd->path);
> -		nd->path = path;
> -		nd->inode = path.dentry->d_inode;
> -		if (unlikely(nd->flags & LOOKUP_NO_XDEV))
> +		path_put(path);
> +		*path = new_path;
> +		if (unlikely(flags & LOOKUP_NO_XDEV))
>  			return ERR_PTR(-EXDEV);
>  	}
>  	/* rare case of legitimate dget_parent()... */
> -	parent = dget_parent(nd->path.dentry);
> -	if (unlikely(!path_connected(nd->path.mnt, parent))) {
> +	parent = dget_parent(path->dentry);
> +	if (unlikely(!path_connected(path->mnt, parent))) {

This is checking path_connected here but also in follow_dotdot,
path_connected is checked again. Is this check meant to be here?  It will
also change the landlock behaviour right?

(For some reason patch 2 rejects when I tried to apply it on v6.16-rc1, so
I haven't actually tested this patch to see if this is really an issue)

>  		dput(parent);
>  		return ERR_PTR(-ENOENT);
>  	}
>  	return parent;
>  
>  in_root:
> -	if (unlikely(nd->flags & LOOKUP_BENEATH))
> +	if (unlikely(flags & LOOKUP_BENEATH))
>  		return ERR_PTR(-EXDEV);
> -	return dget(nd->path.dentry);
> +	return dget(path->dentry);
> +}
> +
> +/**
> + * path_walk_parent - Walk to the parent of path
> + * @path: input and output path.
> + * @root: root of the path walk, do not go beyond this root. If @root is
> + *        zero'ed, walk all the way to real root.
> + *
> + * Given a path, find the parent path. Replace @path with the parent path.
> + * If we were already at the real root or a disconnected root, @path is
> + * released and zero'ed.
> + *
> + * Returns:
> + *  true  - if @path is updated to its parent.
> + *  false - if @path is already the root (real root or @root).
> + */
> +bool path_walk_parent(struct path *path, const struct path *root)
> +{
> +	struct dentry *parent;
> +
> +	parent = __path_walk_parent(path, root, LOOKUP_BENEATH);
> +
> +	if (IS_ERR(parent))
> +		goto false_out;
> +
> +	if (parent == path->dentry) {
> +		dput(parent);
> +		goto false_out;
> +	}
> +	dput(path->dentry);
> +	path->dentry = parent;
> +	return true;
> +
> +false_out:
> +	path_put(path);
> +	memset(path, 0, sizeof(*path));
> +	return false;
> +}
> +
> +static struct dentry *follow_dotdot(struct nameidata *nd)
> +{
> +	struct dentry *parent = __path_walk_parent(&nd->path, &nd->root, nd->flags);
> +
> +	if (IS_ERR(parent))
> +		return parent;
> +	if (unlikely(!path_connected(nd->path.mnt, parent))) {
> +		dput(parent);
> +		return ERR_PTR(-ENOENT);
> +	}
> +	nd->inode = nd->path.dentry->d_inode;
> +	return parent;
>  }
>  
>  static const char *handle_dots(struct nameidata *nd, int type)
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 5d085428e471..cba5373ecf86 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -85,6 +85,8 @@ extern int follow_down_one(struct path *);
>  extern int follow_down(struct path *path, unsigned int flags);
>  extern int follow_up(struct path *);
>  
> +bool path_walk_parent(struct path *path, const struct path *root);
> +
>  extern struct dentry *lock_rename(struct dentry *, struct dentry *);
>  extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
>  extern void unlock_rename(struct dentry *, struct dentry *);


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

* Re: [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent()
  2025-06-14 18:36   ` Tingmao Wang
@ 2025-06-17  5:08     ` Song Liu
  0 siblings, 0 replies; 11+ messages in thread
From: Song Liu @ 2025-06-17  5:08 UTC (permalink / raw)
  To: Tingmao Wang
  Cc: bpf, linux-fsdevel, linux-kernel, linux-security-module,
	kernel-team, andrii, eddyz87, ast, daniel, martin.lau, viro,
	brauner, jack, kpsingh, mattbobrowski, amir73il, repnop, jlayton,
	josef, mic, gnoack, neil

On Sat, Jun 14, 2025 at 11:36 AM Tingmao Wang <m@maowtm.org> wrote:
>
> On 6/11/25 23:02, Song Liu wrote:
> > This helper walks an input path to its parent. Logic are added to handle
> > walking across mount tree.
> >
> > This will be used by landlock, and BPF LSM.
> >
> > Suggested-by: Neil Brown <neil@brown.name>
> > Signed-off-by: Song Liu <song@kernel.org>
> > ---
> >  fs/namei.c            | 99 +++++++++++++++++++++++++++++++++++++------
> >  include/linux/namei.h |  2 +
> >  2 files changed, 87 insertions(+), 14 deletions(-)
> >
> > diff --git a/fs/namei.c b/fs/namei.c
> > index 4bb889fc980b..bc65361c5d13 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -2048,36 +2048,107 @@ static struct dentry *follow_dotdot_rcu(struct nameidata *nd)
> >       return nd->path.dentry;
> >  }
> >
> > -static struct dentry *follow_dotdot(struct nameidata *nd)
> > +/**
> > + * __path_walk_parent - Find the parent of the given struct path
> > + * @path  - The struct path to start from
> > + * @root  - A struct path which serves as a boundary not to be crosses.
> > + *        - If @root is zero'ed, walk all the way to global root.
> > + * @flags - Some LOOKUP_ flags.
> > + *
> > + * Find and return the dentry for the parent of the given path
> > + * (mount/dentry). If the given path is the root of a mounted tree, it
> > + * is first updated to the mount point on which that tree is mounted.
> > + *
> > + * If %LOOKUP_NO_XDEV is given, then *after* the path is updated to a new
> > + * mount, the error EXDEV is returned.
> > + *
> > + * If no parent can be found, either because the tree is not mounted or
> > + * because the @path matches the @root, then @path->dentry is returned
> > + * unless @flags contains %LOOKUP_BENEATH, in which case -EXDEV is returned.
> > + *
> > + * Returns: either an ERR_PTR() or the chosen parent which will have had
> > + * the refcount incremented.
> > + */
> > +static struct dentry *__path_walk_parent(struct path *path, const struct path *root, int flags)
> >  {
> >       struct dentry *parent;
> >
> > -     if (path_equal(&nd->path, &nd->root))
> > +     if (path_equal(path, root))
> >               goto in_root;
> > -     if (unlikely(nd->path.dentry == nd->path.mnt->mnt_root)) {
> > -             struct path path;
> > +     if (unlikely(path->dentry == path->mnt->mnt_root)) {
> > +             struct path new_path;
> >
> > -             if (!choose_mountpoint(real_mount(nd->path.mnt),
> > -                                    &nd->root, &path))
> > +             if (!choose_mountpoint(real_mount(path->mnt),
> > +                                    root, &new_path))
> >                       goto in_root;
> > -             path_put(&nd->path);
> > -             nd->path = path;
> > -             nd->inode = path.dentry->d_inode;
> > -             if (unlikely(nd->flags & LOOKUP_NO_XDEV))
> > +             path_put(path);
> > +             *path = new_path;
> > +             if (unlikely(flags & LOOKUP_NO_XDEV))
> >                       return ERR_PTR(-EXDEV);
> >       }
> >       /* rare case of legitimate dget_parent()... */
> > -     parent = dget_parent(nd->path.dentry);
> > -     if (unlikely(!path_connected(nd->path.mnt, parent))) {
> > +     parent = dget_parent(path->dentry);
> > +     if (unlikely(!path_connected(path->mnt, parent))) {
>
> This is checking path_connected here but also in follow_dotdot,
> path_connected is checked again. Is this check meant to be here?  It will
> also change the landlock behaviour right?

Good catch! Removed the check in v5.

>
> (For some reason patch 2 rejects when I tried to apply it on v6.16-rc1, so
> I haven't actually tested this patch to see if this is really an issue)
>
> >               dput(parent);
> >               return ERR_PTR(-ENOENT);
> >       }
> >       return parent;
> >
> >  in_root:
> > -     if (unlikely(nd->flags & LOOKUP_BENEATH))
> > +     if (unlikely(flags & LOOKUP_BENEATH))
> >               return ERR_PTR(-EXDEV);
> > -     return dget(nd->path.dentry);
> > +     return dget(path->dentry);
> > +}
> > +
> > +/**
> > + * path_walk_parent - Walk to the parent of path
> > + * @path: input and output path.
> > + * @root: root of the path walk, do not go beyond this root. If @root is
> > + *        zero'ed, walk all the way to real root.
> > + *
> > + * Given a path, find the parent path. Replace @path with the parent path.
> > + * If we were already at the real root or a disconnected root, @path is
> > + * released and zero'ed.
> > + *
> > + * Returns:
> > + *  true  - if @path is updated to its parent.
> > + *  false - if @path is already the root (real root or @root).
> > + */
> > +bool path_walk_parent(struct path *path, const struct path *root)
> > +{
> > +     struct dentry *parent;
> > +
> > +     parent = __path_walk_parent(path, root, LOOKUP_BENEATH);
> > +
> > +     if (IS_ERR(parent))
> > +             goto false_out;
> > +
> > +     if (parent == path->dentry) {
> > +             dput(parent);
> > +             goto false_out;
> > +     }
> > +     dput(path->dentry);
> > +     path->dentry = parent;
> > +     return true;
> > +
> > +false_out:
> > +     path_put(path);
> > +     memset(path, 0, sizeof(*path));
> > +     return false;
> > +}
> > +
> > +static struct dentry *follow_dotdot(struct nameidata *nd)
> > +{
> > +     struct dentry *parent = __path_walk_parent(&nd->path, &nd->root, nd->flags);
> > +
> > +     if (IS_ERR(parent))
> > +             return parent;
> > +     if (unlikely(!path_connected(nd->path.mnt, parent))) {
> > +             dput(parent);
> > +             return ERR_PTR(-ENOENT);
> > +     }
> > +     nd->inode = nd->path.dentry->d_inode;
> > +     return parent;
> >  }
> >
> >  static const char *handle_dots(struct nameidata *nd, int type)
> > diff --git a/include/linux/namei.h b/include/linux/namei.h
> > index 5d085428e471..cba5373ecf86 100644
> > --- a/include/linux/namei.h
> > +++ b/include/linux/namei.h
> > @@ -85,6 +85,8 @@ extern int follow_down_one(struct path *);
> >  extern int follow_down(struct path *path, unsigned int flags);
> >  extern int follow_up(struct path *);
> >
> > +bool path_walk_parent(struct path *path, const struct path *root);
> > +
> >  extern struct dentry *lock_rename(struct dentry *, struct dentry *);
> >  extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
> >  extern void unlock_rename(struct dentry *, struct dentry *);
>

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

end of thread, other threads:[~2025-06-17  5:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11 22:02 [PATCH v4 bpf-next 0/5] bpf path iterator Song Liu
2025-06-11 22:02 ` [PATCH v4 bpf-next 1/5] namei: Introduce new helper function path_walk_parent() Song Liu
2025-06-13  0:10   ` NeilBrown
2025-06-13 22:27     ` Song Liu
2025-06-14 18:36   ` Tingmao Wang
2025-06-17  5:08     ` Song Liu
2025-06-11 22:02 ` [PATCH v4 bpf-next 2/5] landlock: Use path_walk_parent() Song Liu
2025-06-11 22:02 ` [PATCH v4 bpf-next 3/5] bpf: Introduce path iterator Song Liu
2025-06-12 23:11   ` Andrii Nakryiko
2025-06-11 22:02 ` [PATCH v4 bpf-next 4/5] selftests/bpf: Add tests for bpf " Song Liu
2025-06-11 22:02 ` [PATCH v4 bpf-next 5/5] selftests/bpf: Path walk test Song Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).