[RFC PATCH bpf-next 00/12] fs: unified VFS ancestor walk for Landlock and BPF

[RFC PATCH bpf-next 00/12] fs: unified VFS ancestor walk for Landlock and BPF

[Justin Suess]

Howdy folks,

This series adds a VFS API for walking a path's ancestors that covers
both path-walk modes (refwalk and rcu-walk, plus the hybrid escalation
between them) through a unified interface, converts Landlock to it, and
exposes it to BPF LSM programs.  It picks up Song Liu's bpf path
iterator series [1] where the discussion left it, and tries to be the
synthesis of what came out of that thread: a combination of the
callback and open-coded approaches proposed there.

It's a bigger diff, because it tries to cover the whole three-cheese
enchilada of VFS path walk: refwalk, rcu and hybrid. I figure doing all
modes at once is best, even if it hurts now, as bolting on an RCU api as
an afterthought would make for a miserable experience for in-kernel and
BPF users alike dealing with breakage / migration.

I'm RFC'ing this for now, as I'm still not 100% confident on the design
choices, especially on some of the weirder corners of VFS (disconnected
directories, MNT_DOOMED).

Much credit to Song Liu for the inspiration for the initial design.

Background
==========

Landlock evaluates filesystem access by walking from a path to the real
root with an open-coded dget_parent()/follow_up() loop.  This works,
but the semantics are difficult to get right. Al noted that follow_up()
ignores the mount's disappearance, so a concurrent umount can leave
that walk operating on an unaccounted mount [2].  BPF LSMs (Tetragon et
al.) want the same walk and today approximate it with racy probe reads.
Song's series added a step-up helper and a referenced-only BPF
iterator.

This attempts to bring some of the lessons from Landlock's VFS walk to
VFS core code, and exposes the same API to BPF. Other callers can be
converted later.

From what I understand from the feedback, Christian rejected merging a
referenced-mode API now and bolting rcu-walk on later, and asked for
one unified API serving both use-cases, with the callback design Neil
sketched as the accepted shape for in-kernel callers [3].  Mickaël
additionally required that disconnected directories be well-defined
before anything lands, and suggested the layering used here:
"the best approach would be to have a VFS API with a callback,
and a BPF helper (leveraging this VFS API) with an iterator state" [4].

What this series does
=====================

It's a path walking API for VFS, with BPF and Landlock consumers.

The core is a struct vfs_ancestor_walk driven by vfs_walk_start() /
vfs_walk_next() / vfs_walk_end(), sharing its step-to-parent
cores with follow_dotdot() and follow_dotdot_rcu().  The engine
owns every invariant: references in refwalk mode, d_seq/mount_lock
validation in rcu mode, mount crossings via choose_mountpoint(),
which is what fixes the Landlock race.  Both modes live behind the
same calls; a VFS_WALK_RCU flag picks the mode and nothing outside
namei.c sees seqcounts or nameidata.

On top of the engine sit two front-ends matched to their consumers.
In-kernel callers get the callback design, which must not sleep.

  int vfs_walk_ancestors(const struct path *path,
			 int (*cb)(const struct path *ancestor,
				   unsigned int pos_flags, void *data),
			 void *data, unsigned int flags);

@cb is invoked on @path, then on each ancestor up to the real root,
and returns VFS_WALK_CONTINUE, VFS_WALK_STOP or a negative errno;
@flags takes VFS_WALK_RCU when the callback copes with unreferenced
positions.  Landlock's walk converts to this, preserving its evaluation
order exactly (including the disconnected-directory semantics of its
recent fixes) plus the mount_lock-validated crossings.

The engine itself is declared in fs/internal.h:

  void vfs_walk_start(struct vfs_ancestor_walk *aw,
		      const struct path *path, unsigned int flags);
  int  vfs_walk_next(struct vfs_ancestor_walk *aw);
  void vfs_walk_end(struct vfs_ancestor_walk *aw);
  bool vfs_walk_handover(struct vfs_ancestor_walk *to,
			 struct vfs_ancestor_walk *from);

So the one consumer that cannot be a callback (BPF) can drive it one
step per call, from fs/bpf_fs_kfuncs.c. Here are the (numerous) kfuncs,
many of which are thin wrappers around the engine:

  /* referenced, sleepable: each position comes acquired */
  int  bpf_iter_path_ancestors_new(struct bpf_iter_path_ancestors *it,
				   struct path *path, u64 flags);
  struct path *
       bpf_iter_path_ancestors_next(struct bpf_iter_path_ancestors *it);
  void bpf_iter_path_ancestors_destroy(struct bpf_iter_path_ancestors *it);
  u32  bpf_path_ancestors_pos_flags(struct bpf_iter_path_ancestors *it);
  void bpf_path_put(struct path *path);

  /* lockless: positions are borrowed, valid until the next step */
  int  bpf_iter_path_ancestors_rcu_new(struct bpf_iter_path_ancestors_rcu *it,
				       struct path *path, u64 flags);
  struct path *
       bpf_iter_path_ancestors_rcu_next(struct bpf_iter_path_ancestors_rcu *it);
  void bpf_iter_path_ancestors_rcu_destroy(struct bpf_iter_path_ancestors_rcu *it);
  u32  bpf_path_ancestors_rcu_pos_flags(struct bpf_iter_path_ancestors_rcu *it);

  /* escalation for hybrid walk. Mirroring unlazy_walk(): acquire the
   * lockless walk's current position straight into a referenced iterator
   */
  int  bpf_path_ancestors_legitimize(struct bpf_iter_path_ancestors *it,
				     struct bpf_iter_path_ancestors_rcu *rcu_it);

The lockless constructor is KF_RCU_PROTECTED, so the verifier forces
the whole iteration into one RCU read-side critical section, within
which everything sleepable is already rejected. Each next() is still
namei.c code, so the walk invariants stay in the VFS even though the
loop is in the program.  A hybrid walk runs lockless until it needs
sleepable work, then it escalates. Here's what hybrid walk looks like
concretely:

	bpf_rcu_read_lock();
	bpf_iter_path_ancestors_rcu_new(&rit, dir, 0);
	while ((pos = bpf_iter_path_ancestors_rcu_next(&rit))) {
		/* borrowed position: evaluate, but no sleeping allowed */
		if (foobar(pos))
			break;
	}
  /* converts the iterator from rcu -> refwalk */
	bpf_path_ancestors_legitimize(&it, &rit);
	bpf_iter_path_ancestors_rcu_destroy(&rit);
	bpf_rcu_read_unlock();

	/* sleepable kfuncs are legal again. */
	while ((pos = bpf_iter_path_ancestors_next(&it))) {
		bpf_path_d_path(pos, buf, sizeof(buf));
    /* refwalk gives you path references that you gotta free to appease
     * the verifier
     */
		bpf_path_put(pos);
	}
	bpf_iter_path_ancestors_destroy(&it);

Nothing is allocated during the escalation, so it cannot fail for want
of memory inside the critical section, and the sleepable work lands
after bpf_rcu_read_unlock() by construction: the resuming next() is a
sleepable kfunc, an ordering the verifier enforces.

A lockless walk that loses a race does not restart transparently: it
dies with -ECHILD (BPF: BPF_PATH_ANCESTORS_RETRY), the caller discards
what it accumulated and retries in referenced mode, so one lockless
attempt bounds the retries.  This is simpler to reason about than the
restart-signal contract discussed in the thread.

Disconnected directories are exposed first-class rather than papered over:
positions whose dentry is a disconnected root are flagged
VFS_WALK_POS_DISCONNECTED (plus VFS_WALK_POS_MOUNTPOINT for the
mountpoint a crossing landed on, which the old Landlock loop never
visited), and continuing over one resumes at the root of its mount.
The mechanism lives in the walker; the MNT_INTERNAL allow-and-stop
policy stays in Landlock.

Supporting pieces: mnt_undo_legitimize() gives a failed
__legitimize_mnt() an undo callable inside the RCU read-side critical
section, deferring a final mntput to delayed_mntput().

Secondly, for the bpf_path_ancestors_legitimize, a patch is added to allow
kfuncs to accept an "__uninit"-suffixed iterator argument so a generic
kfunc (the handover) can initialize an iterator from another one, since
KF_ITER_NEW allows only one constructor per type; and an acquiring
KF_ITER_NEXT's drained branch now releases the reference its acquire
bookkeeping created, which no in-tree iterator needed before.

Patches 1 and 4 are carried from Song's series; patch 3 is derived from
his Landlock conversion and carries the Fixes: tag for the follow_up()
race, per Mickaël's request on v5.

Open questions
==============

- Whether an open-coded iterator is acceptable to the VFS as the BPF
  front-end, given the engine and invariants stay in namei.c and the
  verifier provides the discipline a kernel-owned loop would.  The
  callback shape was the thread's endorsed design for in-kernel
  callers; I believe this split is what Mickaël proposed in [4], and
  the alternative (BPF programs as the callback) costs considerably
  more verifier machinery for a worse programming model.

- Whether VFS_WALK_POS_MOUNTPOINT should exist at all.  It is there so
  Landlock's rule evaluation stays bit-for-bit what it was before the
  conversion; if matching rules on crossed-onto disconnected
  mountpoints is acceptable as a behavior change, the flag disappears.

- The retry-on-ECHILD contract versus a transparent restart signal.

[1] https://lore.kernel.org/bpf/20250617061116.3681325-1-song@kernel.org/
[2] https://lore.kernel.org/r/20250529231018.GP2023217@ZenIV
[3] https://lore.kernel.org/all/20250707-netto-campieren-501525a7d10a@brauner/
[4] https://lore.kernel.org/all/20250704.quio1ceil4Xi@digikod.net/

Justin Suess (11):
  namei: add vfs_walk_ancestors()
  landlock: convert ancestor walk to vfs_walk_ancestors()
  bpf: mark struct path trusted
  namei: make vfs_walk_ancestors() stepwise
  bpf: add a path ancestor iterator
  selftests/bpf: exercise the path ancestor iterator
  fs: add mnt_undo_legitimize()
  namei: add an rcu-walk mode to the ancestor walk
  bpf: support "__uninit" iterator arguments in generic kfuncs
  bpf: add a lockless path ancestor iterator
  selftests/bpf: exercise the lockless path ancestor iterator

Song Liu (1):
  namei: introduce __path_walk_parent()

 fs/bpf_fs_kfuncs.c                            | 248 ++++++++++++
 fs/internal.h                                 |  22 +
 fs/mount.h                                    |   1 +
 fs/namei.c                                    | 377 ++++++++++++++++--
 fs/namespace.c                                |  63 ++-
 include/linux/namei.h                         |  17 +
 kernel/bpf/verifier.c                         |  49 ++-
 security/landlock/fs.c                        | 265 ++++++------
 .../selftests/bpf/prog_tests/path_ancestors.c |  77 ++++
 .../selftests/bpf/progs/path_ancestors.c      | 134 +++++++
 10 files changed, 1076 insertions(+), 177 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/path_ancestors.c
 create mode 100644 tools/testing/selftests/bpf/progs/path_ancestors.c


base-commit: 99dc1ba542420db6b8df209744f55cc52466ad91
-- 
2.55.0

[RFC PATCH bpf-next 01/12] namei: introduce __path_walk_parent()

[Justin Suess]

From: Song Liu <song@kernel.org>

Factor the step-to-parent logic out of follow_dotdot() into
__path_walk_parent(), which walks a plain struct path to its parent,
crossing mount trees via choose_mountpoint().

This will be used by a VFS ancestor walker serving landlock and BPF.

Suggested-by: Neil Brown <neil@brown.name>
Signed-off-by: Song Liu <song@kernel.org>
[justin: rebased, kept the helper static to namei.c, dropped the
 path_walk_parent() wrapper and the namei.h export]
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 fs/namei.c | 62 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 20a6534ea3ef..808fb4bed7c4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2192,36 +2192,62 @@ static struct dentry *follow_dotdot_rcu(struct nameidata *nd)
 	return nd->path.dentry;
 }
 
-static struct dentry *follow_dotdot(struct nameidata *nd)
+/**
+ * __path_walk_parent - step towards the parent of the given struct path
+ * @path: position to step up from; if it is the root of a mounted tree, it
+ *        is first updated to the mount point that tree is mounted on
+ * @root: boundary not to be crossed; if zero'ed, walk all the way to the
+ *        global root
+ * @flags: %LOOKUP_NO_XDEV returns -EXDEV once @path was updated to a new
+ *         mount; %LOOKUP_BENEATH returns -EXDEV at @root or at the root of
+ *         an unmounted tree, where @path->dentry itself is returned
+ *         otherwise
+ *
+ * Returns: the parent dentry with its refcount incremented, or an ERR_PTR().
+ */
+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);
+	return dget_parent(path->dentry);
+
+in_root:
+	if (unlikely(flags & LOOKUP_BENEATH))
+		return ERR_PTR(-EXDEV);
+	return dget(path->dentry);
+}
+
+static struct dentry *follow_dotdot(struct nameidata *nd)
+{
+	struct dentry *parent;
+
+	if (path_equal(&nd->path, &nd->root)) {
+		/* nd->root is never path_connected()-checked. */
+		if (unlikely(nd->flags & LOOKUP_BENEATH))
+			return ERR_PTR(-EXDEV);
+		return dget(nd->path.dentry);
+	}
+	parent = __path_walk_parent(&nd->path, &nd->root, nd->flags);
+	nd->inode = nd->path.dentry->d_inode;
+	if (IS_ERR(parent))
+		return parent;
 	if (unlikely(!path_connected(nd->path.mnt, parent))) {
 		dput(parent);
 		return ERR_PTR(-ENOENT);
 	}
 	return parent;
-
-in_root:
-	if (unlikely(nd->flags & LOOKUP_BENEATH))
-		return ERR_PTR(-EXDEV);
-	return dget(nd->path.dentry);
 }
 
 static const char *handle_dots(struct nameidata *nd, enum last_type type)
-- 
2.55.0

[RFC PATCH bpf-next 02/12] namei: add vfs_walk_ancestors()

[Justin Suess]

Add a callback-based walk over a path and its ancestors, built on
__path_walk_parent(): the kernel owns the loop and the callback must
not sleep, so an rcu-walk engine can be added later without changing
the API.

Disconnected root dentries are reported to the callback instead of
terminating the walk; continuing over one resumes at the root of its
mount.  A disconnected mountpoint landed on by a mount crossing - the
one position this walk visits that a dget_parent()/follow_up() loop
never does - is additionally flagged VFS_WALK_POS_MOUNTPOINT, so
security callers can reproduce their pre-conversion evaluation
sequence exactly.

Suggested-by: NeilBrown <neil@brown.name>
Suggested-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 fs/namei.c            | 76 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/namei.h | 14 ++++++++
 2 files changed, 90 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index 808fb4bed7c4..2e6ea19714b2 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2229,6 +2229,82 @@ static struct dentry *__path_walk_parent(struct path *path, const struct path *r
 	return dget(path->dentry);
 }
 
+/**
+ * vfs_walk_ancestors - invoke a callback on a path and each of its ancestors
+ * @path: path to walk up from; the caller's path is never modified
+ * @cb: callback invoked on @path, then on each ancestor up to the real
+ *      root, crossing mount boundaries.  @cb must not sleep and returns
+ *      %VFS_WALK_CONTINUE, %VFS_WALK_STOP or a negative errno to abort the
+ *      walk.  @ancestor is only valid during the invocation; @cb must take
+ *      its own references to keep a position.
+ *      A position whose dentry is a disconnected root is flagged with
+ *      %VFS_WALK_POS_DISCONNECTED (plus %VFS_WALK_POS_MOUNTPOINT when it
+ *      is a mountpoint a mount crossing landed on rather than a parent);
+ *      if @cb continues over it, the walk resumes at the root of that
+ *      position's mount.
+ * @data: opaque argument passed to @cb
+ * @flags: %VFS_WALK_* flags; none defined yet, pass 0
+ *
+ * Returns: 0 once the real root was reached, 1 if @cb stopped the walk, or
+ * the negative errno @cb aborted with.
+ */
+int vfs_walk_ancestors(const struct path *path,
+		       int (*cb)(const struct path *ancestor,
+				 unsigned int pos_flags, void *data),
+		       void *data, unsigned int flags)
+{
+	const struct path root = {};
+	struct path walk = *path;
+	unsigned int pos_flags = 0;
+	int ret;
+
+	path_get(&walk);
+	if (unlikely(IS_ROOT(walk.dentry) &&
+		     walk.dentry != walk.mnt->mnt_root))
+		pos_flags = VFS_WALK_POS_DISCONNECTED;
+	for (;;) {
+		struct dentry *parent;
+
+		ret = cb(&walk, pos_flags, data);
+		if (ret < 0)
+			break;
+		if (ret == VFS_WALK_STOP) {
+			ret = 1;
+			break;
+		}
+
+		if (unlikely(pos_flags & VFS_WALK_POS_DISCONNECTED)) {
+			dput(walk.dentry);
+			walk.dentry = dget(walk.mnt->mnt_root);
+			pos_flags = 0;
+			continue;
+		}
+		parent = __path_walk_parent(&walk, &root, LOOKUP_BENEATH);
+		if (IS_ERR(parent)) {
+			/* The real root. */
+			ret = 0;
+			break;
+		}
+		/*
+		 * A mount crossing can step onto a disconnected root, whose
+		 * parent is itself: only then is the mountpoint itself
+		 * visited, flagged, next iteration.
+		 */
+		if (unlikely(parent == walk.dentry))
+			pos_flags = VFS_WALK_POS_DISCONNECTED |
+				    VFS_WALK_POS_MOUNTPOINT;
+		else if (unlikely(IS_ROOT(parent) &&
+				  parent != walk.mnt->mnt_root))
+			pos_flags = VFS_WALK_POS_DISCONNECTED;
+		else
+			pos_flags = 0;
+		dput(walk.dentry);
+		walk.dentry = parent;
+	}
+	path_put(&walk);
+	return ret;
+}
+
 static struct dentry *follow_dotdot(struct nameidata *nd)
 {
 	struct dentry *parent;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 86d657b24fc6..3e198de7a0d3 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -162,6 +162,20 @@ extern int follow_down_one(struct path *);
 extern int follow_down(struct path *path, unsigned int flags);
 extern int follow_up(struct path *);
 
+/* per-position flags passed to the vfs_walk_ancestors() callback */
+#define VFS_WALK_POS_DISCONNECTED	BIT(0)
+/* the position is a mountpoint landed on by a mount crossing */
+#define VFS_WALK_POS_MOUNTPOINT		BIT(1)
+
+/* vfs_walk_ancestors() callback verdicts; negative values abort the walk */
+#define VFS_WALK_STOP		0
+#define VFS_WALK_CONTINUE	1
+
+int vfs_walk_ancestors(const struct path *path,
+		       int (*cb)(const struct path *ancestor,
+				 unsigned int pos_flags, void *data),
+		       void *data, unsigned int flags);
+
 int start_renaming(struct renamedata *rd, int lookup_flags,
 		   struct qstr *old_last, struct qstr *new_last);
 int start_renaming_dentry(struct renamedata *rd, int lookup_flags,
-- 
2.55.0

[RFC PATCH bpf-next 03/12] landlock: convert ancestor walk to vfs_walk_ancestors()

[Justin Suess]

Replace the open-coded jump_up/follow_up loop of
is_access_to_paths_allowed() with one vfs_walk_ancestors() call, moving
the loop state into a struct passed to the callback.

follow_up() ignores the mount's disappearance, so a concurrent umount
could make the walk operate on an unaccounted mount.  The walker steps
with choose_mountpoint(), which revalidates against mount_lock.  The
disconnected-directory handling is otherwise preserved: MNT_INTERNAL
roots allow and stop, other disconnected roots resume at their mount
root.  Rule evaluation is preserved exactly: disconnected roots that
were positions of the old loop (the walk's start, the parent of a
disconnected subtree) keep being matched against rules, while the
disconnected mountpoints the walker newly visits on a mount crossing
are skipped via VFS_WALK_POS_MOUNTPOINT, as the old loop never
consulted them.

Co-developed-by: Song Liu <song@kernel.org>
Signed-off-by: Song Liu <song@kernel.org>
Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Closes: https://lore.kernel.org/r/20250529231018.GP2023217@ZenIV
Fixes: cb2c7d1a1776 ("landlock: Support filesystem access-control")
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 security/landlock/fs.c | 265 +++++++++++++++++++++--------------------
 1 file changed, 133 insertions(+), 132 deletions(-)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index cab43892ec2f..369e2e9c83ad 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -751,6 +751,107 @@ static void test_is_eacces_with_write(struct kunit *const test)
 #undef IE_TRUE
 #undef IE_FALSE
 
+struct landlock_walk_state {
+	const struct landlock_domain *domain;
+	struct layer_masks *layer_masks_parent1, *layer_masks_parent2;
+	const struct layer_masks *layer_masks_child1, *layer_masks_child2;
+	access_mask_t access_request_parent1, access_request_parent2;
+	access_mask_t access_masked_parent1, access_masked_parent2;
+	bool child1_is_directory, child2_is_directory;
+	bool allowed_parent1, allowed_parent2, is_dom_check;
+};
+
+static int check_access_path_walk(const struct path *const ancestor,
+				  const unsigned int pos_flags,
+				  void *const data)
+{
+	struct landlock_walk_state *const state = data;
+	struct landlock_id id = {
+		.type = LANDLOCK_KEY_INODE,
+	};
+
+	if (unlikely(pos_flags & VFS_WALK_POS_DISCONNECTED)) {
+		if (likely(ancestor->mnt->mnt_flags & MNT_INTERNAL)) {
+			/*
+			 * Stops and allows access when reaching disconnected
+			 * root directories that are part of internal
+			 * filesystems (e.g. nsfs, which is reachable through
+			 * /proc/<pid>/ns/<namespace>).
+			 */
+			state->allowed_parent1 = true;
+			state->allowed_parent2 = true;
+			return VFS_WALK_STOP;
+		}
+		/*
+		 * The old loop never visited the mountpoints a mount
+		 * crossing lands on: don't match them against rules.  Other
+		 * disconnected roots keep being matched below, as before.
+		 */
+		if (pos_flags & VFS_WALK_POS_MOUNTPOINT)
+			return VFS_WALK_CONTINUE;
+	}
+
+	/*
+	 * If at least all accesses allowed on the destination are already
+	 * allowed on the source, respectively if there is at least as much as
+	 * restrictions on the destination than on the source, then we can
+	 * safely refer files from the source to the destination without
+	 * risking a privilege escalation.  This also applies in the case of
+	 * RENAME_EXCHANGE, which implies checks on both direction.  This is
+	 * crucial for standalone multilayered security policies.  Furthermore,
+	 * this helps avoid policy writers to shoot themselves in the foot.
+	 */
+	if (unlikely(state->is_dom_check &&
+		     no_more_access(state->layer_masks_parent1,
+				    state->layer_masks_child1,
+				    state->child1_is_directory,
+				    state->layer_masks_parent2,
+				    state->layer_masks_child2,
+				    state->child2_is_directory))) {
+		/*
+		 * Now, downgrades the remaining checks from domain handled
+		 * accesses to requested accesses.
+		 */
+		state->is_dom_check = false;
+		state->access_masked_parent1 = state->access_request_parent1;
+		state->access_masked_parent2 = state->access_request_parent2;
+
+		state->allowed_parent1 =
+			state->allowed_parent1 ||
+			scope_to_request(state->access_masked_parent1,
+					 state->layer_masks_parent1);
+		state->allowed_parent2 =
+			state->allowed_parent2 ||
+			scope_to_request(state->access_masked_parent2,
+					 state->layer_masks_parent2);
+
+		/* Stops when all accesses are granted. */
+		if (state->allowed_parent1 && state->allowed_parent2)
+			return VFS_WALK_STOP;
+	}
+
+	if (get_inode_id(ancestor->dentry, &id)) {
+		state->allowed_parent1 =
+			state->allowed_parent1 ||
+			unmask_layers_fs(state->domain, id,
+					 state->access_masked_parent1,
+					 state->layer_masks_parent1,
+					 ancestor->dentry);
+		state->allowed_parent2 =
+			state->allowed_parent2 ||
+			unmask_layers_fs(state->domain, id,
+					 state->access_masked_parent2,
+					 state->layer_masks_parent2,
+					 ancestor->dentry);
+	}
+
+	/* Stops when a rule from each layer grants access. */
+	if (state->allowed_parent1 && state->allowed_parent2)
+		return VFS_WALK_STOP;
+
+	return VFS_WALK_CONTINUE;
+}
+
 /**
  * is_access_to_paths_allowed - Check accesses for requests with a common path
  *
@@ -803,16 +904,16 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain,
 			   struct landlock_request *const log_request_parent2,
 			   struct dentry *const dentry_child2)
 {
-	bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
-	     child1_is_directory = true, child2_is_directory = true;
-	struct path walker_path;
-	struct landlock_id id = {
-		.type = LANDLOCK_KEY_INODE,
-	};
-	access_mask_t access_masked_parent1, access_masked_parent2;
 	struct layer_masks _layer_masks_child1, _layer_masks_child2;
-	struct layer_masks *layer_masks_child1 = NULL,
-			   *layer_masks_child2 = NULL;
+	struct landlock_walk_state state = {
+		.domain = domain,
+		.layer_masks_parent1 = layer_masks_parent1,
+		.layer_masks_parent2 = layer_masks_parent2,
+		.access_request_parent1 = access_request_parent1,
+		.access_request_parent2 = access_request_parent2,
+		.child1_is_directory = true,
+		.child2_is_directory = true,
+	};
 
 	if (!access_request_parent1 && !access_request_parent2)
 		return true;
@@ -826,37 +927,39 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain,
 	if (WARN_ON_ONCE(!layer_masks_parent1))
 		return false;
 
-	allowed_parent1 = is_layer_masks_allowed(layer_masks_parent1);
+	state.allowed_parent1 = is_layer_masks_allowed(layer_masks_parent1);
 
 	if (unlikely(layer_masks_parent2)) {
 		if (WARN_ON_ONCE(!dentry_child1))
 			return false;
 
-		allowed_parent2 = is_layer_masks_allowed(layer_masks_parent2);
+		state.allowed_parent2 =
+			is_layer_masks_allowed(layer_masks_parent2);
 
 		/*
 		 * For a double request, first check for potential privilege
 		 * escalation by looking at domain handled accesses (which are
 		 * a superset of the meaningful requested accesses).
 		 */
-		access_masked_parent1 = access_masked_parent2 =
+		state.access_masked_parent1 =
 			landlock_union_access_masks(domain).fs;
-		is_dom_check = true;
+		state.access_masked_parent2 = state.access_masked_parent1;
+		state.is_dom_check = true;
 	} else {
 		if (WARN_ON_ONCE(dentry_child1 || dentry_child2))
 			return false;
 		/* For a simple request, only check for requested accesses. */
-		access_masked_parent1 = access_request_parent1;
-		access_masked_parent2 = access_request_parent2;
+		state.access_masked_parent1 = access_request_parent1;
+		state.access_masked_parent2 = access_request_parent2;
 		/*
 		 * Simple requests have no parent2 to check, so parent2 is
 		 * trivially allowed.  This must be set explicitly because the
-		 * get_inode_id() gate in the pathwalk loop may prevent
+		 * get_inode_id() gate in the walk callback may prevent
 		 * landlock_unmask_layers() from being called (which would
 		 * otherwise return true for NULL masks as a side effect).
 		 */
-		allowed_parent2 = true;
-		is_dom_check = false;
+		state.allowed_parent2 = true;
+		state.is_dom_check = false;
 	}
 
 	if (unlikely(dentry_child1)) {
@@ -872,8 +975,8 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain,
 		if (handled && get_inode_id(dentry_child1, &id))
 			unmask_layers_fs(domain, id, handled,
 					 &_layer_masks_child1, dentry_child1);
-		layer_masks_child1 = &_layer_masks_child1;
-		child1_is_directory = d_is_dir(dentry_child1);
+		state.layer_masks_child1 = &_layer_masks_child1;
+		state.child1_is_directory = d_is_dir(dentry_child1);
 	}
 	if (unlikely(dentry_child2)) {
 		struct landlock_id id = {
@@ -888,118 +991,16 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain,
 		if (handled && get_inode_id(dentry_child2, &id))
 			unmask_layers_fs(domain, id, handled,
 					 &_layer_masks_child2, dentry_child2);
-		layer_masks_child2 = &_layer_masks_child2;
-		child2_is_directory = d_is_dir(dentry_child2);
+		state.layer_masks_child2 = &_layer_masks_child2;
+		state.child2_is_directory = d_is_dir(dentry_child2);
 	}
 
-	walker_path = *path;
-	path_get(&walker_path);
 	/*
 	 * We need to walk through all the hierarchy to not miss any relevant
-	 * restriction.
+	 * restriction.  Reaching the real root without a grant from each
+	 * layer denies access.
 	 */
-	while (true) {
-		/*
-		 * If at least all accesses allowed on the destination are
-		 * already allowed on the source, respectively if there is at
-		 * least as much as restrictions on the destination than on the
-		 * source, then we can safely refer files from the source to
-		 * the destination without risking a privilege escalation.
-		 * This also applies in the case of RENAME_EXCHANGE, which
-		 * implies checks on both direction.  This is crucial for
-		 * standalone multilayered security policies.  Furthermore,
-		 * this helps avoid policy writers to shoot themselves in the
-		 * foot.
-		 */
-		if (unlikely(is_dom_check &&
-			     no_more_access(
-				     layer_masks_parent1, layer_masks_child1,
-				     child1_is_directory, layer_masks_parent2,
-				     layer_masks_child2,
-				     child2_is_directory))) {
-			/*
-			 * Now, downgrades the remaining checks from domain
-			 * handled accesses to requested accesses.
-			 */
-			is_dom_check = false;
-			access_masked_parent1 = access_request_parent1;
-			access_masked_parent2 = access_request_parent2;
-
-			allowed_parent1 =
-				allowed_parent1 ||
-				scope_to_request(access_masked_parent1,
-						 layer_masks_parent1);
-			allowed_parent2 =
-				allowed_parent2 ||
-				scope_to_request(access_masked_parent2,
-						 layer_masks_parent2);
-
-			/* Stops when all accesses are granted. */
-			if (allowed_parent1 && allowed_parent2)
-				break;
-		}
-
-		if (get_inode_id(walker_path.dentry, &id)) {
-			allowed_parent1 =
-				allowed_parent1 ||
-				unmask_layers_fs(domain, id,
-						 access_masked_parent1,
-						 layer_masks_parent1,
-						 walker_path.dentry);
-			allowed_parent2 =
-				allowed_parent2 ||
-				unmask_layers_fs(domain, id,
-						 access_masked_parent2,
-						 layer_masks_parent2,
-						 walker_path.dentry);
-		}
-
-		/* 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 (likely(walker_path.mnt->mnt_flags & MNT_INTERNAL)) {
-				/*
-				 * Stops and allows access when reaching disconnected root
-				 * directories that are part of internal filesystems (e.g. nsfs,
-				 * which is reachable through /proc/<pid>/ns/<namespace>).
-				 */
-				allowed_parent1 = true;
-				allowed_parent2 = true;
-				break;
-			}
-
-			/*
-			 * We reached a disconnected root directory from a bind mount.
-			 * Let's continue the walk with the mount point we missed.
-			 */
-			dput(walker_path.dentry);
-			walker_path.dentry = walker_path.mnt->mnt_root;
-			dget(walker_path.dentry);
-		} else {
-			struct dentry *const parent_dentry =
-				dget_parent(walker_path.dentry);
-
-			dput(walker_path.dentry);
-			walker_path.dentry = parent_dentry;
-		}
-	}
-	path_put(&walker_path);
+	vfs_walk_ancestors(path, check_access_path_walk, &state, 0);
 
 	/*
 	 * Check CONFIG_SECURITY_LANDLOCK_LOG to enable elision of
@@ -1007,24 +1008,24 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain,
 	 * dead code elimination.
 	 */
 #ifdef CONFIG_SECURITY_LANDLOCK_LOG
-	if (!allowed_parent1 && log_request_parent1) {
+	if (!state.allowed_parent1 && log_request_parent1) {
 		log_request_parent1->type = LANDLOCK_REQUEST_FS_ACCESS;
 		log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH;
 		log_request_parent1->audit.u.path = *path;
-		log_request_parent1->access = access_masked_parent1;
+		log_request_parent1->access = state.access_masked_parent1;
 		log_request_parent1->layer_masks = layer_masks_parent1;
 	}
 
-	if (!allowed_parent2 && log_request_parent2) {
+	if (!state.allowed_parent2 && log_request_parent2) {
 		log_request_parent2->type = LANDLOCK_REQUEST_FS_ACCESS;
 		log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
 		log_request_parent2->audit.u.path = *path;
-		log_request_parent2->access = access_masked_parent2;
+		log_request_parent2->access = state.access_masked_parent2;
 		log_request_parent2->layer_masks = layer_masks_parent2;
 	}
 #endif /* CONFIG_SECURITY_LANDLOCK_LOG */
 
-	return allowed_parent1 && allowed_parent2;
+	return state.allowed_parent1 && state.allowed_parent2;
 }
 
 static int current_check_access_path(const struct path *const path,
-- 
2.55.0

[RFC PATCH bpf-next 04/12] bpf: mark struct path trusted

[Justin Suess]

Walking a trusted struct path yields a trusted dentry - a live path
never carries a NULL dentry - so BPF programs can use it with kfuncs
that require trusted arguments.

Signed-off-by: Song Liu <song@kernel.org>
[justin: carried only the verifier.c hunk of the original patch,
 trusted instead of trusted-or-null]
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 kernel/bpf/verifier.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fd3c0206bd67..24ec4b037de7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6300,6 +6300,10 @@ BTF_TYPE_SAFE_TRUSTED(struct scx_sub_detach_args) {
 	struct sched_ext_ops *ops;
 };
 
+BTF_TYPE_SAFE_TRUSTED(struct path) {
+	struct dentry *dentry;
+};
+
 BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct dentry) {
 	struct inode *d_inode;
 };
@@ -6352,6 +6356,7 @@ static bool type_is_trusted(struct bpf_verifier_env *env,
 	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct scx_cpu_release_args));
 	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct scx_sub_attach_args));
 	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct scx_sub_detach_args));
+	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct path));
 
 	return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id, "__safe_trusted");
 }
-- 
2.55.0

[RFC PATCH bpf-next 05/12] namei: make vfs_walk_ancestors() stepwise

[Justin Suess]

Restructure the walk into a stepwise engine - vfs_walk_start(),
vfs_walk_next(), vfs_walk_end() - private to fs/, so an iterating
consumer (BPF) can drive it with every step and invariant staying in
namei.c.  vfs_walk_ancestors() becomes a loop over it, preserving the
callback contract.

The position's flags move into the walk state, where the next step
needs them anyway: stepping off a disconnected position resumes at the
root of its mount.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 fs/internal.h |  15 ++++++
 fs/namei.c    | 141 ++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 117 insertions(+), 39 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index c658c8a5ebd5..3ce2220ff57f 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -71,6 +71,21 @@ struct dentry *start_dirop(struct dentry *parent, struct qstr *name,
 			   unsigned int lookup_flags);
 int lookup_noperm_common(struct qstr *qname, struct dentry *base);
 
+/*
+ * The stepwise engine under vfs_walk_ancestors(); fs-internal so iterating
+ * consumers (BPF) can drive it, with the walk invariants staying in namei.c.
+ */
+struct vfs_ancestor_walk {
+	struct path pos;
+	unsigned int pos_flags;	/* VFS_WALK_POS_* describing pos */
+	unsigned int flags;	/* VFS_WALK_* */
+};
+
+void vfs_walk_start(struct vfs_ancestor_walk *aw, const struct path *path,
+		    unsigned int flags);
+int vfs_walk_next(struct vfs_ancestor_walk *aw);
+void vfs_walk_end(struct vfs_ancestor_walk *aw);
+
 void __init filename_init(void);
 
 /*
diff --git a/fs/namei.c b/fs/namei.c
index 2e6ea19714b2..73f25152d917 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2229,6 +2229,97 @@ static struct dentry *__path_walk_parent(struct path *path, const struct path *r
 	return dget(path->dentry);
 }
 
+static unsigned int vfs_walk_pos_flags(const struct vfsmount *mnt,
+				       const struct dentry *dentry)
+{
+	if (unlikely(IS_ROOT(dentry) && dentry != mnt->mnt_root))
+		return VFS_WALK_POS_DISCONNECTED;
+	return 0;
+}
+
+/* No boundary: ancestor walks go up to the real root. */
+static const struct path vfs_walk_no_root;
+
+/* Internal walk flag: the starting position has been yielded. */
+#define VFS_WALK_STARTED	BIT(31)
+
+/**
+ * vfs_walk_start - begin a stepwise ancestor walk
+ * @aw: walk state, valid until vfs_walk_end()
+ * @path: position to walk up from; never modified
+ * @flags: %VFS_WALK_* flags; none defined yet, pass 0
+ */
+void vfs_walk_start(struct vfs_ancestor_walk *aw, const struct path *path,
+		    unsigned int flags)
+{
+	aw->pos = *path;
+	aw->flags = flags;
+	aw->pos_flags = 0;
+	path_get(&aw->pos);
+}
+
+static int vfs_walk_step_ref(struct vfs_ancestor_walk *aw)
+{
+	struct dentry *parent;
+
+	if (unlikely(aw->pos_flags & VFS_WALK_POS_DISCONNECTED)) {
+		/* Resume at the root of the disconnected position's mount. */
+		dput(aw->pos.dentry);
+		aw->pos.dentry = dget(aw->pos.mnt->mnt_root);
+		aw->pos_flags = 0;
+		return 0;
+	}
+	parent = __path_walk_parent(&aw->pos, &vfs_walk_no_root,
+				    LOOKUP_BENEATH);
+	if (IS_ERR(parent))
+		/* The real root. */
+		return 1;
+	/*
+	 * A mount crossing can step onto a disconnected root, whose parent
+	 * is itself: only then is the mountpoint itself yielded, flagged.
+	 */
+	aw->pos_flags = parent == aw->pos.dentry ?
+		VFS_WALK_POS_DISCONNECTED | VFS_WALK_POS_MOUNTPOINT :
+		vfs_walk_pos_flags(aw->pos.mnt, parent);
+	dput(aw->pos.dentry);
+	aw->pos.dentry = parent;
+	return 0;
+}
+
+/**
+ * vfs_walk_next - yield the walk's next position in @aw->pos
+ * @aw: the walk
+ *
+ * The first call yields the starting position itself.  @aw->pos_flags
+ * carries the position's %VFS_WALK_POS_* flags, with the semantics
+ * described at vfs_walk_ancestors().
+ *
+ * Returns: 0 with @aw->pos valid, 1 once the walk has passed the real
+ * root.
+ */
+int vfs_walk_next(struct vfs_ancestor_walk *aw)
+{
+	if (aw->flags & VFS_WALK_STARTED) {
+		int err = vfs_walk_step_ref(aw);
+
+		if (err)
+			return err;
+	} else {
+		aw->flags |= VFS_WALK_STARTED;
+		aw->pos_flags = vfs_walk_pos_flags(aw->pos.mnt, aw->pos.dentry);
+	}
+	return 0;
+}
+
+/**
+ * vfs_walk_end - finish a stepwise ancestor walk
+ * @aw: the walk; its positions, including @aw->pos, become invalid
+ */
+void vfs_walk_end(struct vfs_ancestor_walk *aw)
+{
+	path_put(&aw->pos);
+}
+
 /**
  * vfs_walk_ancestors - invoke a callback on a path and each of its ancestors
  * @path: path to walk up from; the caller's path is never modified
@@ -2253,55 +2344,27 @@ int vfs_walk_ancestors(const struct path *path,
 				 unsigned int pos_flags, void *data),
 		       void *data, unsigned int flags)
 {
-	const struct path root = {};
-	struct path walk = *path;
-	unsigned int pos_flags = 0;
+	struct vfs_ancestor_walk aw;
 	int ret;
 
-	path_get(&walk);
-	if (unlikely(IS_ROOT(walk.dentry) &&
-		     walk.dentry != walk.mnt->mnt_root))
-		pos_flags = VFS_WALK_POS_DISCONNECTED;
+	vfs_walk_start(&aw, path, flags);
 	for (;;) {
-		struct dentry *parent;
-
-		ret = cb(&walk, pos_flags, data);
+		ret = vfs_walk_next(&aw);
+		if (ret) {
+			if (ret == 1)
+				/* The real root was reached. */
+				ret = 0;
+			break;
+		}
+		ret = cb(&aw.pos, aw.pos_flags, data);
 		if (ret < 0)
 			break;
 		if (ret == VFS_WALK_STOP) {
 			ret = 1;
 			break;
 		}
-
-		if (unlikely(pos_flags & VFS_WALK_POS_DISCONNECTED)) {
-			dput(walk.dentry);
-			walk.dentry = dget(walk.mnt->mnt_root);
-			pos_flags = 0;
-			continue;
-		}
-		parent = __path_walk_parent(&walk, &root, LOOKUP_BENEATH);
-		if (IS_ERR(parent)) {
-			/* The real root. */
-			ret = 0;
-			break;
-		}
-		/*
-		 * A mount crossing can step onto a disconnected root, whose
-		 * parent is itself: only then is the mountpoint itself
-		 * visited, flagged, next iteration.
-		 */
-		if (unlikely(parent == walk.dentry))
-			pos_flags = VFS_WALK_POS_DISCONNECTED |
-				    VFS_WALK_POS_MOUNTPOINT;
-		else if (unlikely(IS_ROOT(parent) &&
-				  parent != walk.mnt->mnt_root))
-			pos_flags = VFS_WALK_POS_DISCONNECTED;
-		else
-			pos_flags = 0;
-		dput(walk.dentry);
-		walk.dentry = parent;
 	}
-	path_put(&walk);
+	vfs_walk_end(&aw);
 	return ret;
 }
 
-- 
2.55.0

[RFC PATCH bpf-next 06/12] bpf: add a path ancestor iterator

[Justin Suess]

Let BPF programs evaluate a path's ancestry with an open-coded iterator
over the stepwise vfs_walk_ancestors() engine.  The iteration holds a
reference on its current position, which is what lets a program sleep
between positions - a dput() of the last reference may - so the kfuncs
are KF_SLEEPABLE and the iterator is available to sleepable programs
only.  That covers the LSM hooks a path-based policy attaches to:
file_open, file_permission, the path_* hooks, mmap_file and bprm_* are
all in the sleepable allowlist.

bpf_iter_path_ancestors_next() hands each position to the program as an
acquired reference of its own, to release with bpf_path_put().  The
walk's own reference moves on with the walk and is dropped at the next
step, so it cannot be what keeps a position alive: a program that saves
a position's dentry, or hands one to a sleepable kfunc, needs it to
outlive the step it came from.  struct path is a value type with nothing
a BPF reference could be taken on, so an acquired position is a copy of
its own; the allocation is a sleepable GFP_KERNEL one, and a failure
ends the iteration with %BPF_PATH_ANCESTORS_NOMEM rather than silently
truncating the ancestry.

An acquiring KF_ITER_NEXT needs one thing from the verifier: the state
that assumes the drained, NULL-returning branch must not keep the
reference the acquire bookkeeping created for the assumed non-NULL
return.  Open-coded iterators reach that branch through
process_iter_next_call() rather than through mark_ptr_or_null_regs(),
which is where a plain KF_ACQUIRE | KF_RET_NULL kfunc releases it.

The per-position VFS flags are not derivable from the position alone -
whether a disconnected root is a mountpoint a crossing landed on is
walk state - so they are read with bpf_path_ancestors_pos_flags(),
which a program needs to reproduce Landlock's evaluation of
disconnected positions.

The iterator state is deliberately larger than this walk mode needs:
its size is part of the contract with programs, which size their stack
slot from it, so growing it later would reject programs built against
the smaller one.  The walk mode is a parameter of the shared engine for
the same reason - so that a mode added later is not an ABI change.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 fs/bpf_fs_kfuncs.c    | 147 ++++++++++++++++++++++++++++++++++++++++++
 kernel/bpf/verifier.c |   7 ++
 2 files changed, 154 insertions(+)

diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 08f0847c4970..265cb414a08a 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -13,7 +13,11 @@
 #include <linux/kernfs.h>
 #include <linux/lsm_hooks.h>
 #include <linux/mm.h>
+#include <linux/namei.h>
 #include <linux/net.h>
+#include <linux/slab.h>
+
+#include "internal.h"
 #include <linux/xattr.h>
 
 __bpf_kfunc_start_defs();
@@ -500,6 +504,143 @@ __bpf_kfunc struct inode *bpf_real_data_inode(struct file *file)
 
 __bpf_kfunc_end_defs();
 
+enum bpf_path_ancestors_flag {
+	/* bpf_path_ancestors_pos_flags() bits */
+	BPF_PATH_ANCESTORS_DISCONNECTED	= (1 << 0),
+	/* the position is a mountpoint a mount crossing landed on */
+	BPF_PATH_ANCESTORS_MOUNTPOINT	= (1 << 1),
+	/* the iteration ended on a failed allocation, not at the root */
+	BPF_PATH_ANCESTORS_NOMEM	= (1 << 2),
+};
+
+/*
+ * Walks over a path's ancestors.
+ *
+ * bpf_iter_path_ancestors runs with references.  Its kfuncs are
+ * sleepable, so the iteration may sleep between positions.
+ *
+ * Each position is handed to the program as an acquired reference of its
+ * own, to release with bpf_path_put().  The walk's own reference moves on
+ * with the walk, so a position that outlives the step it came from - a
+ * dentry saved for later, or the path a sleepable kfunc is still working
+ * on - has to be kept alive by the program's reference rather than by the
+ * iterator's.
+ */
+struct bpf_iter_path_ancestors {
+	__u64 __opaque[5];
+} __aligned(8);
+
+struct bpf_path_ancestors_kern {
+	struct vfs_ancestor_walk aw;
+	int step;	/* last vfs_walk_next() result, or -ENOMEM */
+} __aligned(8);
+
+static int bpf_path_ancestors_new(struct bpf_path_ancestors_kern *kit,
+				  struct path *path, u64 flags,
+				  unsigned int walk_flags)
+{
+	BUILD_BUG_ON(sizeof(struct bpf_path_ancestors_kern) >
+		     sizeof(struct bpf_iter_path_ancestors));
+	BUILD_BUG_ON(__alignof__(struct bpf_path_ancestors_kern) !=
+		     __alignof__(struct bpf_iter_path_ancestors));
+
+	if (flags) {
+		/* A zeroed walk makes destroying the iterator a no-op. */
+		memset(kit, 0, sizeof(*kit));
+		kit->step = 1;
+		return -EINVAL;
+	}
+	kit->step = 0;
+	vfs_walk_start(&kit->aw, path, walk_flags);
+	return 0;
+}
+
+/* The walk's own view of the next position, which the step after it ends. */
+static struct path *bpf_path_ancestors_step(struct bpf_path_ancestors_kern *kit)
+{
+	if (kit->step)
+		return NULL;
+	kit->step = vfs_walk_next(&kit->aw);
+	return kit->step ? NULL : &kit->aw.pos;
+}
+
+static u32 bpf_path_ancestors_flags(const struct bpf_path_ancestors_kern *kit)
+{
+	u32 flags = 0;
+
+	if (kit->step == -ENOMEM)
+		return BPF_PATH_ANCESTORS_NOMEM;
+	if (!kit->step) {
+		if (kit->aw.pos_flags & VFS_WALK_POS_DISCONNECTED)
+			flags |= BPF_PATH_ANCESTORS_DISCONNECTED;
+		if (kit->aw.pos_flags & VFS_WALK_POS_MOUNTPOINT)
+			flags |= BPF_PATH_ANCESTORS_MOUNTPOINT;
+	}
+	return flags;
+}
+
+__bpf_kfunc_start_defs();
+
+__bpf_kfunc int bpf_iter_path_ancestors_new(struct bpf_iter_path_ancestors *it,
+					    struct path *path, u64 flags)
+{
+	return bpf_path_ancestors_new((void *)it, path, flags, 0);
+}
+
+/**
+ * bpf_iter_path_ancestors_next - acquire the walk's next position
+ * @it: the iterator
+ *
+ * Return: the next position with a reference held, to release with
+ * bpf_path_put(), or NULL once the walk has passed the real root - or on
+ * an allocation failure, which ends the iteration and is reported as
+ * %BPF_PATH_ANCESTORS_NOMEM by bpf_path_ancestors_pos_flags().
+ */
+__bpf_kfunc struct path *
+bpf_iter_path_ancestors_next(struct bpf_iter_path_ancestors *it)
+{
+	struct bpf_path_ancestors_kern *kit = (void *)it;
+	struct path *pos = bpf_path_ancestors_step(kit);
+	struct path *held;
+
+	if (!pos)
+		return NULL;
+	/*
+	 * The position must outlive the walk's own view of it, so it gets a
+	 * reference and a struct path of its own to live in: struct path is
+	 * a value type, with nothing a BPF reference could be taken on
+	 * otherwise.  Sleepable, so no atomic allocation.
+	 */
+	held = kmalloc_obj(*held);
+	if (!held) {
+		kit->step = -ENOMEM;
+		return NULL;
+	}
+	*held = *pos;
+	path_get(held);
+	return held;
+}
+
+__bpf_kfunc void
+bpf_iter_path_ancestors_destroy(struct bpf_iter_path_ancestors *it)
+{
+	vfs_walk_end(&((struct bpf_path_ancestors_kern *)it)->aw);
+}
+
+__bpf_kfunc u32
+bpf_path_ancestors_pos_flags(struct bpf_iter_path_ancestors *it__iter)
+{
+	return bpf_path_ancestors_flags((void *)it__iter);
+}
+
+__bpf_kfunc void bpf_path_put(struct path *path)
+{
+	path_put(path);
+	kfree(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_RET_NULL)
 BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE)
@@ -513,6 +654,12 @@ BTF_ID_FLAGS(func, bpf_inode_init_xattr)
 #ifdef CONFIG_NET
 BTF_ID_FLAGS(func, bpf_sock_read_xattr, KF_RCU)
 #endif
+BTF_ID_FLAGS(func, bpf_iter_path_ancestors_new, KF_ITER_NEW | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_iter_path_ancestors_next,
+	     KF_ITER_NEXT | KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_iter_path_ancestors_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_path_ancestors_pos_flags)
+BTF_ID_FLAGS(func, bpf_path_put, KF_RELEASE | KF_SLEEPABLE)
 BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
 
 /* Side-effecting kfuncs that stay exclusive to LSM programs. */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 24ec4b037de7..066c4b838b85 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8648,6 +8648,13 @@ static int process_iter_next_call(struct bpf_verifier_env *env, int insn_idx,
 	/* switch to DRAINED state, but keep the depth unchanged */
 	/* mark current iter state as drained and assume returned NULL */
 	cur_iter->iter.state = BPF_ITER_STATE_DRAINED;
+	/*
+	 * An acquiring iter_next() hands out nothing once drained: the
+	 * acquired reference exists only in the forked active state, not on
+	 * this NULL-returning branch.
+	 */
+	if (meta->kfunc_flags & KF_ACQUIRE)
+		WARN_ON_ONCE(release_reference_nomark(env, cur_fr->regs[BPF_REG_0].id));
 	__mark_reg_const_zero(env, &cur_fr->regs[BPF_REG_0]);
 
 	return 0;
-- 
2.55.0

[RFC PATCH bpf-next 07/12] selftests/bpf: exercise the path ancestor iterator

[Justin Suess]

Walk the ancestry of a mkdir's parent directory from a sleepable LSM
program and check that the iteration reaches the root: the temporary
directory, its parent and / are visited, and no position comes back
flagged.

Resolve the second position's pathname from the acquired position after
the step that yielded it has been taken, so that the acquired reference
is what the sleepable kfunc runs on rather than the walk's.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 .../selftests/bpf/prog_tests/path_ancestors.c | 48 ++++++++++++++++++
 .../selftests/bpf/progs/path_ancestors.c      | 49 +++++++++++++++++++
 2 files changed, 97 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/path_ancestors.c
 create mode 100644 tools/testing/selftests/bpf/progs/path_ancestors.c

diff --git a/tools/testing/selftests/bpf/prog_tests/path_ancestors.c b/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
new file mode 100644
index 000000000000..2de79673a13b
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Justin Suess */
+
+#include <sys/stat.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <test_progs.h>
+#include "path_ancestors.skel.h"
+
+void test_path_ancestors(void)
+{
+	char base[] = "/tmp/path_ancestors_XXXXXX";
+	struct path_ancestors *skel = NULL;
+	char suba[280], subb[280];
+
+	if (!ASSERT_OK_PTR(mkdtemp(base), "mkdtemp"))
+		return;
+	snprintf(suba, sizeof(suba), "%s/a", base);
+	snprintf(subb, sizeof(subb), "%s/a/b", base);
+	if (!ASSERT_OK(mkdir(suba, 0755), "mkdir_a"))
+		goto out_rm;
+
+	skel = path_ancestors__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		goto out_rm;
+	skel->bss->monitored_pid = getpid();
+	if (!ASSERT_OK(path_ancestors__attach(skel), "attach"))
+		goto out;
+
+	/* mkdir b: the hook sees dir == suba, whose ancestry is walked. */
+	if (!ASSERT_OK(mkdir(subb, 0755), "mkdir_b"))
+		goto out;
+
+	/* suba, base, /tmp, / at least. */
+	ASSERT_GE(skel->bss->ref_count, 3, "ref_count");
+	ASSERT_EQ(skel->bss->ref_flags, 0, "ref_flags");
+
+	/* The acquired second position, used after its step was taken. */
+	ASSERT_STREQ(skel->bss->second_path, base, "second_path");
+	ASSERT_EQ(skel->bss->second_len, strlen(base) + 1, "second_len");
+
+out:
+	path_ancestors__destroy(skel);
+out_rm:
+	rmdir(subb);
+	rmdir(suba);
+	rmdir(base);
+}
diff --git a/tools/testing/selftests/bpf/progs/path_ancestors.c b/tools/testing/selftests/bpf/progs/path_ancestors.c
new file mode 100644
index 000000000000..af6b777e8bec
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/path_ancestors.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Justin Suess */
+
+#include "vmlinux.h"
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_kfuncs.h"
+
+char _license[] SEC("license") = "GPL";
+
+__u32 monitored_pid;
+
+int ref_count;		/* positions seen by the pure referenced walk */
+int ref_flags;		/* pos flags seen by the referenced walk */
+int second_len;		/* d_path length of the walk's second position */
+char second_path[256];
+
+static bool monitored(void)
+{
+	return (bpf_get_current_pid_tgid() >> 32) == monitored_pid;
+}
+
+SEC("lsm.s/path_mkdir")
+int BPF_PROG(walk_modes, const struct path *dir, struct dentry *dentry,
+	     umode_t mode)
+{
+	struct bpf_iter_path_ancestors it;
+	struct path *pos;
+
+	if (!monitored())
+		return 0;
+
+	/*
+	 * Referenced walk: every position comes acquired, so it stays valid
+	 * for sleepable work and past the step that yielded it.
+	 */
+	bpf_iter_path_ancestors_new(&it, (struct path *)dir, 0);
+	while ((pos = bpf_iter_path_ancestors_next(&it))) {
+		ref_count++;
+		ref_flags |= bpf_path_ancestors_pos_flags(&it);
+		if (ref_count == 2)
+			second_len = bpf_path_d_path(pos, second_path,
+						     sizeof(second_path));
+		bpf_path_put(pos);
+	}
+	bpf_iter_path_ancestors_destroy(&it);
+	return 0;
+}
-- 
2.55.0

[RFC PATCH bpf-next 08/12] fs: add mnt_undo_legitimize()

[Justin Suess]

A failed __legitimize_mnt() can oblige its caller to mntput() the
mount, which may be the final put and so may sleep: legitimize_mnt()
leaves the RCU read-side critical section it ran under to do it.  A
caller that cannot leave its critical section - a BPF program driving
a lockless walk, which only the verifier ends - needs that put to be
callable from inside it.

Add mnt_undo_legitimize(), which drops the count and, when the count
turns out to be the mount's last, keeps it and hands the mount to
delayed_mntput() for the sleepable final put.  delayed_mntput() then
tells the two queue populations apart by MNT_DOOMED rather than by an
unlocked mnt_get_count() read, which a concurrent failed
__legitimize_mnt() could transiently inflate.  legitimize_mnt()
switches to it as well, retiring its
rcu_read_unlock/mntput/rcu_read_lock dance.

Unlike mntput(), mnt_expiry_mark is left alone: a walker that failed
to legitimize a mount never used it.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 fs/mount.h     |  1 +
 fs/namespace.c | 63 ++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/fs/mount.h b/fs/mount.h
index 94fcc306d21e..e714040f8e8f 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -140,6 +140,7 @@ static inline int is_mounted(struct vfsmount *mnt)
 extern struct mount *__lookup_mnt(struct vfsmount *, struct dentry *);
 
 extern int __legitimize_mnt(struct vfsmount *, unsigned);
+void mnt_undo_legitimize(struct mount *mnt);
 
 static inline bool __path_is_mountpoint(const struct path *path)
 {
diff --git a/fs/namespace.c b/fs/namespace.c
index 580877e46b1a..920b8a85f9fa 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -767,11 +767,8 @@ static bool legitimize_mnt(struct vfsmount *bastard, unsigned seq)
 	int res = __legitimize_mnt(bastard, seq);
 	if (likely(!res))
 		return true;
-	if (unlikely(res < 0)) {
-		rcu_read_unlock();
-		mntput(bastard);
-		rcu_read_lock();
-	}
+	if (unlikely(res < 0))
+		mnt_undo_legitimize(real_mount(bastard));
 	return false;
 }
 
@@ -1330,8 +1327,19 @@ static void delayed_mntput(struct work_struct *unused)
 	struct llist_node *node = llist_del_all(&delayed_mntput_list);
 	struct mount *m, *t;
 
-	llist_for_each_entry_safe(m, t, node, mnt_llist)
-		cleanup_mnt(m);
+	llist_for_each_entry_safe(m, t, node, mnt_llist) {
+		/*
+		 * MNT_DOOMED is only ever set on the mounts
+		 * mntput_no_expire_slowpath() queues here, never on the
+		 * kept-count ones mnt_undo_legitimize() queues.  Route on it
+		 * rather than on mnt_get_count(), which a concurrent failed
+		 * __legitimize_mnt() can transiently inflate.
+		 */
+		if (m->mnt.mnt_flags & MNT_DOOMED)
+			cleanup_mnt(m);
+		else
+			mntput(&m->mnt);	/* kept by mnt_undo_legitimize() */
+	}
 }
 static DECLARE_DELAYED_WORK(delayed_mntput_work, delayed_mntput);
 
@@ -1423,6 +1431,47 @@ void mntput(struct vfsmount *mnt)
 }
 EXPORT_SYMBOL(mntput);
 
+/**
+ * mnt_undo_legitimize - drop a count __legitimize_mnt() asked us to put
+ * @mnt: the mount __legitimize_mnt() failed on
+ *
+ * Like the mntput() that a failed __legitimize_mnt() normally obliges,
+ * but callable from the RCU read-side critical section the legitimization
+ * ran under: when the count turns out to be the mount's last, it is kept
+ * and the mount handed to delayed_mntput() for the sleepable final put.
+ * Unlike mntput(), mnt_expiry_mark is left alone: a walker that failed
+ * to legitimize the mount never used it.
+ */
+void mnt_undo_legitimize(struct mount *mnt)
+{
+	if (likely(READ_ONCE(mnt->mnt_ns))) {
+		/* Not the final count, as in mntput_no_expire(). */
+		mnt_add_count(mnt, -1);
+		return;
+	}
+	lock_mount_hash();
+	/*
+	 * As in mntput_no_expire(): make sure that if a concurrent
+	 * __legitimize_mnt() has not seen us grab mount_lock, we'll see
+	 * its refcount increment here.
+	 */
+	smp_mb();
+	if (likely(mnt_get_count(mnt) > 1)) {
+		mnt_add_count(mnt, -1);
+		unlock_mount_hash();
+		return;
+	}
+	unlock_mount_hash();
+	/*
+	 * Ours is the last count: nothing else can reach the mount any
+	 * more, which makes its mnt_llist ours to use.  delayed_mntput()
+	 * tells our kept-count mount from the doomed ones by MNT_DOOMED.
+	 */
+	WARN_ON_ONCE(mnt->mnt.mnt_flags & MNT_DOOMED);
+	if (llist_add(&mnt->mnt_llist, &delayed_mntput_list))
+		schedule_delayed_work(&delayed_mntput_work, 1);
+}
+
 struct vfsmount *mntget(struct vfsmount *mnt)
 {
 	if (mnt)
-- 
2.55.0

[RFC PATCH bpf-next 09/12] namei: add an rcu-walk mode to the ancestor walk

[Justin Suess]

Let the stepwise engine walk lockless, entered with %VFS_WALK_RCU under
the caller's rcu_read_lock: stepping shares follow_dotdot_rcu()'s core,
factored into __path_walk_parent_rcu(), with per-step d_seq validation,
choose_mountpoint_rcu() for crossings and mount_lock revalidation
before concluding at the real root.  Any lost race surfaces as -ECHILD
and the caller retries in the referenced mode.

vfs_walk_handover() continues a lockless walk with references, so that
a caller can do at a position what no lockless walk can.  It mirrors
unlazy_walk(): the position is legitimized and becomes the starting
position of a second, reference-based walk that owns the references
acquired on it.

Both halves of that are ordered so that nothing has to be put: a failed
legitimization leaves no partial references, and the one case where
__legitimize_mnt() obliges a sleepable mntput() goes to
mnt_undo_legitimize().  Handing the reference straight over rather than
copying it out and dropping a second one is what keeps the whole
operation callable inside the caller's RCU read-side critical section,
where even a path_put() that provably cannot sleep is still a dput()
and so still a might_sleep().

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 fs/internal.h         |   7 ++
 fs/namei.c            | 196 ++++++++++++++++++++++++++++++++++++------
 include/linux/namei.h |   3 +
 3 files changed, 179 insertions(+), 27 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 3ce2220ff57f..8a5696157121 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -74,9 +74,14 @@ int lookup_noperm_common(struct qstr *qname, struct dentry *base);
 /*
  * The stepwise engine under vfs_walk_ancestors(); fs-internal so iterating
  * consumers (BPF) can drive it, with the walk invariants staying in namei.c.
+ * In rcu mode (%VFS_WALK_RCU) the caller holds rcu_read_lock() over the
+ * whole walk, no references are held, and vfs_walk_next() returning -ECHILD
+ * invalidates everything derived from the walk.
  */
 struct vfs_ancestor_walk {
 	struct path pos;
+	unsigned int seq;	/* pos.dentry->d_seq sample (rcu mode) */
+	unsigned int m_seq;	/* mount_lock sample (rcu mode) */
 	unsigned int pos_flags;	/* VFS_WALK_POS_* describing pos */
 	unsigned int flags;	/* VFS_WALK_* */
 };
@@ -85,6 +90,8 @@ void vfs_walk_start(struct vfs_ancestor_walk *aw, const struct path *path,
 		    unsigned int flags);
 int vfs_walk_next(struct vfs_ancestor_walk *aw);
 void vfs_walk_end(struct vfs_ancestor_walk *aw);
+bool vfs_walk_handover(struct vfs_ancestor_walk *to,
+		       struct vfs_ancestor_walk *from);
 
 void __init filename_init(void);
 
diff --git a/fs/namei.c b/fs/namei.c
index 73f25152d917..31f96602d4e9 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2152,34 +2152,69 @@ static __always_inline const char *step_into(struct nameidata *nd, int flags,
 	return step_into_slowpath(nd, flags, dentry);
 }
 
-static struct dentry *follow_dotdot_rcu(struct nameidata *nd)
+/**
+ * __path_walk_parent_rcu - step towards the parent of the given struct path
+ * @path: position to step up from; updated in place on a mount crossing,
+ *        which is first if @path is the root of a mounted tree.  No
+ *        references are acquired; the callers layer their own bookkeeping
+ *        (path_connected(), nameidata updates, ...) on top
+ * @root: boundary as for choose_mountpoint_rcu(); if zero'ed, walk all the
+ *        way to the global root
+ * @flags: %LOOKUP_NO_XDEV fails a mount crossing with -ECHILD
+ * @m_seq: the walk's mount_lock sample
+ * @seqp: d_seq sample validating @path->dentry; updated to cover the new
+ *        @path->dentry when a mount is crossed
+ * @next_seqp: set to the returned parent's d_seq sample
+ *
+ * Returns: the parent dentry (which is @path->dentry itself if that is a
+ * disconnected root), NULL if @path is in the root with nothing to cross
+ * into, or ERR_PTR(-ECHILD) when a concurrent change was detected.
+ */
+static struct dentry *__path_walk_parent_rcu(struct path *path,
+					     const struct path *root, int flags,
+					     unsigned int m_seq, unsigned int *seqp,
+					     unsigned int *next_seqp)
 {
 	struct dentry *parent, *old;
 
-	if (path_equal(&nd->path, &nd->root))
-		goto in_root;
-	if (unlikely(nd->path.dentry == nd->path.mnt->mnt_root)) {
-		struct path path;
-		unsigned seq;
-		if (!choose_mountpoint_rcu(real_mount(nd->path.mnt),
-					   &nd->root, &path, &seq))
-			goto in_root;
-		if (unlikely(nd->flags & LOOKUP_NO_XDEV))
+	if (unlikely(path->dentry == path->mnt->mnt_root)) {
+		struct path mounted;
+		unsigned int seq;
+
+		if (!choose_mountpoint_rcu(real_mount(path->mnt),
+					   root, &mounted, &seq))
+			return NULL;
+		if (unlikely(flags & LOOKUP_NO_XDEV))
 			return ERR_PTR(-ECHILD);
-		nd->path = path;
-		nd->inode = path.dentry->d_inode;
-		nd->seq = seq;
+		*path = mounted;
+		*seqp = seq;
 		// makes sure that non-RCU pathwalk could reach this state
-		if (read_seqretry(&mount_lock, nd->m_seq))
+		if (read_seqretry(&mount_lock, m_seq))
 			return ERR_PTR(-ECHILD);
 		/* we know that mountpoint was pinned */
 	}
-	old = nd->path.dentry;
+	old = path->dentry;
 	parent = old->d_parent;
-	nd->next_seq = read_seqcount_begin(&parent->d_seq);
+	*next_seqp = read_seqcount_begin(&parent->d_seq);
 	// makes sure that non-RCU pathwalk could reach this state
-	if (read_seqcount_retry(&old->d_seq, nd->seq))
+	if (read_seqcount_retry(&old->d_seq, *seqp))
 		return ERR_PTR(-ECHILD);
+	return parent;
+}
+
+static struct dentry *follow_dotdot_rcu(struct nameidata *nd)
+{
+	struct dentry *parent;
+
+	if (path_equal(&nd->path, &nd->root))
+		goto in_root;
+	parent = __path_walk_parent_rcu(&nd->path, &nd->root, nd->flags,
+					nd->m_seq, &nd->seq, &nd->next_seq);
+	if (!parent)
+		goto in_root;
+	if (IS_ERR(parent))
+		return parent;
+	nd->inode = nd->path.dentry->d_inode;
 	if (unlikely(!path_connected(nd->path.mnt, parent)))
 		return ERR_PTR(-ECHILD);
 	return parent;
@@ -2247,7 +2282,9 @@ static const struct path vfs_walk_no_root;
  * vfs_walk_start - begin a stepwise ancestor walk
  * @aw: walk state, valid until vfs_walk_end()
  * @path: position to walk up from; never modified
- * @flags: %VFS_WALK_* flags; none defined yet, pass 0
+ * @flags: %VFS_WALK_RCU to walk lockless; the caller then holds
+ *         rcu_read_lock() from before vfs_walk_start() until after
+ *         vfs_walk_end(), and owns no references on yielded positions.
  */
 void vfs_walk_start(struct vfs_ancestor_walk *aw, const struct path *path,
 		    unsigned int flags)
@@ -2255,7 +2292,14 @@ void vfs_walk_start(struct vfs_ancestor_walk *aw, const struct path *path,
 	aw->pos = *path;
 	aw->flags = flags;
 	aw->pos_flags = 0;
-	path_get(&aw->pos);
+	if (flags & VFS_WALK_RCU) {
+		RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
+				 "rcu-mode ancestor walk outside of RCU read-side critical section");
+		aw->m_seq = read_seqbegin(&mount_lock);
+		aw->seq = raw_seqcount_begin(&aw->pos.dentry->d_seq);
+	} else {
+		path_get(&aw->pos);
+	}
 }
 
 static int vfs_walk_step_ref(struct vfs_ancestor_walk *aw)
@@ -2286,6 +2330,35 @@ static int vfs_walk_step_ref(struct vfs_ancestor_walk *aw)
 	return 0;
 }
 
+static int vfs_walk_step_rcu(struct vfs_ancestor_walk *aw)
+{
+	struct dentry *parent;
+	unsigned int next_seq;
+
+	if (unlikely(aw->pos_flags & VFS_WALK_POS_DISCONNECTED)) {
+		/* Resume at the root of the disconnected position's mount. */
+		aw->pos.dentry = aw->pos.mnt->mnt_root;
+		aw->seq = raw_seqcount_begin(&aw->pos.dentry->d_seq);
+		aw->pos_flags = 0;
+		return read_seqretry(&mount_lock, aw->m_seq) ? -ECHILD : 0;
+	}
+
+	parent = __path_walk_parent_rcu(&aw->pos, &vfs_walk_no_root, 0,
+					aw->m_seq, &aw->seq, &next_seq);
+	if (!parent)
+		/* The real root, unless the mount tree moved. */
+		return read_seqretry(&mount_lock, aw->m_seq) ? -ECHILD : 1;
+	if (IS_ERR(parent))
+		return PTR_ERR(parent);
+	/* A crossing onto a disconnected root, as in vfs_walk_step_ref(). */
+	aw->pos_flags = parent == aw->pos.dentry ?
+		VFS_WALK_POS_DISCONNECTED | VFS_WALK_POS_MOUNTPOINT :
+		vfs_walk_pos_flags(aw->pos.mnt, parent);
+	aw->pos.dentry = parent;
+	aw->seq = next_seq;
+	return 0;
+}
+
 /**
  * vfs_walk_next - yield the walk's next position in @aw->pos
  * @aw: the walk
@@ -2295,12 +2368,14 @@ static int vfs_walk_step_ref(struct vfs_ancestor_walk *aw)
  * described at vfs_walk_ancestors().
  *
  * Returns: 0 with @aw->pos valid, 1 once the walk has passed the real
- * root.
+ * root, -ECHILD when an rcu-mode walk lost a race and must be retried
+ * (typically in the referenced mode).
  */
 int vfs_walk_next(struct vfs_ancestor_walk *aw)
 {
 	if (aw->flags & VFS_WALK_STARTED) {
-		int err = vfs_walk_step_ref(aw);
+		int err = (aw->flags & VFS_WALK_RCU) ?
+			vfs_walk_step_rcu(aw) : vfs_walk_step_ref(aw);
 
 		if (err)
 			return err;
@@ -2308,6 +2383,10 @@ int vfs_walk_next(struct vfs_ancestor_walk *aw)
 		aw->flags |= VFS_WALK_STARTED;
 		aw->pos_flags = vfs_walk_pos_flags(aw->pos.mnt, aw->pos.dentry);
 	}
+	/* The flags must describe the dentry the seq covers. */
+	if ((aw->flags & VFS_WALK_RCU) &&
+	    read_seqcount_retry(&aw->pos.dentry->d_seq, aw->seq))
+		return -ECHILD;
 	return 0;
 }
 
@@ -2317,7 +2396,59 @@ int vfs_walk_next(struct vfs_ancestor_walk *aw)
  */
 void vfs_walk_end(struct vfs_ancestor_walk *aw)
 {
-	path_put(&aw->pos);
+	if (!(aw->flags & VFS_WALK_RCU))
+		path_put(&aw->pos);
+}
+
+/**
+ * vfs_walk_handover - continue an rcu-mode walk with references
+ * @to: walk state to begin at @from's current position, owning the
+ *      references acquired on it; valid until vfs_walk_end() either way
+ * @from: an rcu-mode walk, positioned by a 0 return from vfs_walk_next()
+ *
+ * Mirrors unlazy_walk(): @from's current position is legitimized and
+ * becomes the starting position of the referenced walk @to, which the
+ * first vfs_walk_next() on it yields.  A failed legitimization leaves no
+ * partial references behind and a successful one moves straight into @to,
+ * so nothing is ever put here and the handover is safe within the caller's
+ * RCU read-side critical section - where a path_put() would not be, dput()
+ * being allowed to sleep.  @to itself is only usable once the caller has
+ * left it, its walk being reference-based.
+ *
+ * @from is untouched on success and may keep stepping, lockless, from
+ * where it stands.  The references do not conclude its walk: a concurrent
+ * rename may relocate the position the instant they are taken, as it may
+ * during any reference-based walk.
+ *
+ * Returns: false iff @from lost a race; it is then dead, as after -ECHILD
+ * from vfs_walk_next(), and @to is zeroed - safe to vfs_walk_end(), but
+ * not to step, so the caller has to remember it never started.
+ */
+bool vfs_walk_handover(struct vfs_ancestor_walk *to,
+		       struct vfs_ancestor_walk *from)
+{
+	struct path pos = from->pos;
+	int err;
+
+	err = __legitimize_mnt(pos.mnt, from->m_seq);
+	if (unlikely(err)) {
+		if (err < 0)
+			mnt_undo_legitimize(real_mount(pos.mnt));
+		goto dead;
+	}
+	if (unlikely(read_seqcount_retry(&pos.dentry->d_seq, from->seq) ||
+		     !lockref_get_not_dead(&pos.dentry->d_lockref))) {
+		mnt_undo_legitimize(real_mount(pos.mnt));
+		goto dead;
+	}
+	to->pos = pos;
+	to->flags = 0;
+	to->pos_flags = 0;
+	return true;
+
+dead:
+	memset(to, 0, sizeof(*to));
+	return false;
 }
 
 /**
@@ -2326,18 +2457,25 @@ void vfs_walk_end(struct vfs_ancestor_walk *aw)
  * @cb: callback invoked on @path, then on each ancestor up to the real
  *      root, crossing mount boundaries.  @cb must not sleep and returns
  *      %VFS_WALK_CONTINUE, %VFS_WALK_STOP or a negative errno to abort the
- *      walk.  @ancestor is only valid during the invocation; @cb must take
- *      its own references to keep a position.
+ *      walk; -ECHILD is reserved (see below).  @ancestor is only valid
+ *      during the invocation; @cb must take its own references to keep a
+ *      position.
  *      A position whose dentry is a disconnected root is flagged with
  *      %VFS_WALK_POS_DISCONNECTED (plus %VFS_WALK_POS_MOUNTPOINT when it
  *      is a mountpoint a mount crossing landed on rather than a parent);
  *      if @cb continues over it, the walk resumes at the root of that
  *      position's mount.
+ *      With %VFS_WALK_RCU, @cb accepts positions the walk holds no
+ *      references on: the walk then runs lockless (under rcu_read_lock)
+ *      and returns -ECHILD when it loses a race, or when @cb returns
+ *      -ECHILD.  The caller should then discard any state @cb accumulated
+ *      and retry without %VFS_WALK_RCU.
  * @data: opaque argument passed to @cb
- * @flags: %VFS_WALK_* flags; none defined yet, pass 0
+ * @flags: %VFS_WALK_RCU if @cb copes with unreferenced positions
  *
- * Returns: 0 once the real root was reached, 1 if @cb stopped the walk, or
- * the negative errno @cb aborted with.
+ * Returns: 0 once the real root was reached, 1 if @cb stopped the walk,
+ * -ECHILD if a lockless walk must be retried with references, or the
+ * negative errno @cb aborted with.
  */
 int vfs_walk_ancestors(const struct path *path,
 		       int (*cb)(const struct path *ancestor,
@@ -2347,6 +2485,8 @@ int vfs_walk_ancestors(const struct path *path,
 	struct vfs_ancestor_walk aw;
 	int ret;
 
+	if (flags & VFS_WALK_RCU)
+		rcu_read_lock();
 	vfs_walk_start(&aw, path, flags);
 	for (;;) {
 		ret = vfs_walk_next(&aw);
@@ -2365,6 +2505,8 @@ int vfs_walk_ancestors(const struct path *path,
 		}
 	}
 	vfs_walk_end(&aw);
+	if (flags & VFS_WALK_RCU)
+		rcu_read_unlock();
 	return ret;
 }
 
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 3e198de7a0d3..8820a2833213 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -162,6 +162,9 @@ extern int follow_down_one(struct path *);
 extern int follow_down(struct path *path, unsigned int flags);
 extern int follow_up(struct path *);
 
+/* vfs_walk_ancestors() flags */
+#define VFS_WALK_RCU			BIT(0)
+
 /* per-position flags passed to the vfs_walk_ancestors() callback */
 #define VFS_WALK_POS_DISCONNECTED	BIT(0)
 /* the position is a mountpoint landed on by a mount crossing */
-- 
2.55.0

[RFC PATCH bpf-next 10/12] bpf: support "__uninit" iterator arguments in generic kfuncs

[Justin Suess]

Iterator constructors are recognised by their KF_ITER_NEW flag, which
binds them to the bpf_iter_<type>_new() name and so to a single
constructor per type.  A kfunc that initializes an iterator from inputs
that naming convention cannot express - e.g. another, already
initialized iterator - therefore cannot be a constructor.

Let such a kfunc mark its destination argument with the "__uninit"
suffix already used for dynptr out-arguments: process_iter_arg() now
decides per argument, rather than per kfunc, whether it initializes the
iterator or operates on an initialized one.  Only iterator-typed
arguments are reclassified, so dynptr "__uninit" out-arguments are
unaffected.

The first user is the path ancestor iterator handover added by the next
patch.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 kernel/bpf/verifier.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 066c4b838b85..3294b2a43167 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8312,6 +8312,24 @@ static bool is_iter_destroy_kfunc(struct bpf_call_arg_meta *meta)
 	return meta->kfunc_flags & KF_ITER_DESTROY;
 }
 
+/*
+ * An "__uninit"-suffixed iterator argument of a kfunc that is not itself an
+ * iterator method: the kfunc initializes that iterator state, as a
+ * bpf_iter_<type>_new() does, but from inputs the constructor naming
+ * convention cannot express - e.g. another, already-initialized iterator.
+ * Only iterator-typed arguments qualify, so "__uninit" dynptr out-arguments
+ * are not reclassified.
+ */
+static bool is_kfunc_arg_iter_init(struct bpf_call_arg_meta *meta, int arg_idx,
+				   const struct btf_param *arg)
+{
+	if (is_iter_kfunc(meta))
+		return false;
+
+	return btf_param_match_suffix(meta->btf, arg, "__uninit") &&
+	       btf_check_iter_arg(meta->btf, meta->func_proto, arg_idx) >= 0;
+}
+
 static bool is_kfunc_arg_iter(struct bpf_call_arg_meta *meta, int arg_idx,
 			      const struct btf_param *arg)
 {
@@ -8322,7 +8340,11 @@ static bool is_kfunc_arg_iter(struct bpf_call_arg_meta *meta, int arg_idx,
 		return arg_idx == 0;
 
 	/* iter passed as an argument to a generic kfunc */
-	return btf_param_match_suffix(meta->btf, arg, "__iter");
+	if (btf_param_match_suffix(meta->btf, arg, "__iter"))
+		return true;
+
+	/* iter state a generic kfunc initializes */
+	return is_kfunc_arg_iter_init(meta, arg_idx, arg);
 }
 
 static int process_iter_arg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
@@ -8332,6 +8354,7 @@ static int process_iter_arg(struct bpf_verifier_env *env, struct bpf_reg_state *
 	struct bpf_func_state *state = bpf_func(env, reg);
 	const struct btf_type *t;
 	int spi, err, i, nr_slots, btf_id;
+	bool init;
 
 	if (reg->type != PTR_TO_STACK) {
 		verbose(env, "%s expected pointer to an iterator on stack\n",
@@ -8363,8 +8386,16 @@ static int process_iter_arg(struct bpf_verifier_env *env, struct bpf_reg_state *
 	t = btf_type_by_id(meta->btf, btf_id);
 	nr_slots = t->size / BPF_REG_SIZE;
 
-	if (is_iter_new_kfunc(meta)) {
-		/* bpf_iter_<type>_new() expects pointer to uninit iter state */
+	/*
+	 * Whether this argument is the iterator the call initializes, rather
+	 * than an initialized one it operates on.
+	 */
+	init = is_iter_new_kfunc(meta) ||
+	       is_kfunc_arg_iter_init(meta, arg,
+				      &btf_params(meta->func_proto)[arg]);
+
+	if (init) {
+		/* expects a pointer to uninit iter state */
 		if (!is_iter_reg_valid_uninit(env, reg, nr_slots)) {
 			verbose(env, "expected uninitialized iter_%s as %s\n",
 				iter_type_str(meta->btf, btf_id), reg_arg_name(env, argno));
-- 
2.55.0

[RFC PATCH bpf-next 11/12] bpf: add a lockless path ancestor iterator

[Justin Suess]

Add a second variant of the ancestor iterator that drives the walk's
rcu mode: bpf_iter_path_ancestors_rcu is KF_RCU_PROTECTED, so the whole
iteration sits in one RCU read-side critical section - implicit in
non-sleepable programs, bpf_rcu_read_lock() in sleepable ones - within
which the verifier already rejects everything sleepable.  This is what
makes a path-based policy expressible from the non-sleepable LSM hooks,
and it drops the per-position reference traffic for the sleepable ones.

Positions come borrowed rather than acquired here: a lockless iteration
holds no reference to pass on, so bpf_iter_path_ancestors_rcu_next()
hands out the walk's own position, valid until the next step.  That is
what RCU protection buys and all it buys: the dentry cannot be freed
under the iteration, but nothing read out of the position may be passed
to a kfunc demanding a trusted argument.

A lockless iteration that loses a race ends with
BPF_PATH_ANCESTORS_RETRY readable through
bpf_path_ancestors_rcu_pos_flags(); the program discards what it
derived from the walk and retries on the referenced variant, so one
lockless attempt bounds the retries.

Escalation mirrors unlazy_walk(): bpf_path_ancestors_legitimize()
acquires the lockless iteration's current position straight into a
referenced iterator the program declared on its stack.  Nothing is
allocated, so nothing can fail for want of memory inside the RCU
read-side critical section, and the escalated position needs no
lifetime of its own: it is the resumed iteration's first position, held
by its reference, yielded by a bpf_iter_path_ancestors_next() that is
sleepable and so necessarily runs after the program has left the
critical section.  That ordering is the whole discipline, and the
verifier enforces it without being told to.

The handover cannot be an iterator constructor - KF_ITER_NEW binds a
type to a single bpf_iter_<type>_new() - so its destination argument is
one of the previous patch's "__uninit" iterator arguments.  Nothing
else is needed: the source argument's ordinary "__iter" classification
already rejects a handover from an iterator whose RCU read-side
critical section has ended.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 fs/bpf_fs_kfuncs.c | 105 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 2 deletions(-)

diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 265cb414a08a..9dad65df4267 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -505,16 +505,19 @@ __bpf_kfunc struct inode *bpf_real_data_inode(struct file *file)
 __bpf_kfunc_end_defs();
 
 enum bpf_path_ancestors_flag {
-	/* bpf_path_ancestors_pos_flags() bits */
+	/* bpf_path_ancestors[_rcu]_pos_flags() bits */
 	BPF_PATH_ANCESTORS_DISCONNECTED	= (1 << 0),
 	/* the position is a mountpoint a mount crossing landed on */
 	BPF_PATH_ANCESTORS_MOUNTPOINT	= (1 << 1),
 	/* the iteration ended on a failed allocation, not at the root */
 	BPF_PATH_ANCESTORS_NOMEM	= (1 << 2),
+	/* the lockless iteration lost a race and reached no conclusion */
+	BPF_PATH_ANCESTORS_RETRY	= (1 << 3),
 };
 
 /*
- * Walks over a path's ancestors.
+ * Walks over a path's ancestors, in two variants differing in how
+ * positions are kept alive:
  *
  * bpf_iter_path_ancestors runs with references.  Its kfuncs are
  * sleepable, so the iteration may sleep between positions.
@@ -525,11 +528,25 @@ enum bpf_path_ancestors_flag {
  * dentry saved for later, or the path a sleepable kfunc is still working
  * on - has to be kept alive by the program's reference rather than by the
  * iterator's.
+ *
+ * bpf_iter_path_ancestors_rcu runs lockless over one RCU read-side
+ * critical section, which the verifier enforces around the whole
+ * iteration and within which sleeping is impossible.  Its positions are
+ * borrowed, not acquired: they are only valid until the next step, and
+ * nothing derived from one may be passed to a kfunc demanding a trusted
+ * argument.  A lost race ends the iteration with
+ * BPF_PATH_ANCESTORS_RETRY set; the program discards what it derived
+ * from the walk and retries, typically on the referenced variant, or
+ * escalates mid-walk with bpf_path_ancestors_legitimize().
  */
 struct bpf_iter_path_ancestors {
 	__u64 __opaque[5];
 } __aligned(8);
 
+struct bpf_iter_path_ancestors_rcu {
+	__u64 __opaque[5];
+} __aligned(8);
+
 struct bpf_path_ancestors_kern {
 	struct vfs_ancestor_walk aw;
 	int step;	/* last vfs_walk_next() result, or -ENOMEM */
@@ -543,6 +560,10 @@ static int bpf_path_ancestors_new(struct bpf_path_ancestors_kern *kit,
 		     sizeof(struct bpf_iter_path_ancestors));
 	BUILD_BUG_ON(__alignof__(struct bpf_path_ancestors_kern) !=
 		     __alignof__(struct bpf_iter_path_ancestors));
+	BUILD_BUG_ON(sizeof(struct bpf_iter_path_ancestors) !=
+		     sizeof(struct bpf_iter_path_ancestors_rcu));
+	BUILD_BUG_ON(__alignof__(struct bpf_iter_path_ancestors) !=
+		     __alignof__(struct bpf_iter_path_ancestors_rcu));
 
 	if (flags) {
 		/* A zeroed walk makes destroying the iterator a no-op. */
@@ -570,6 +591,8 @@ static u32 bpf_path_ancestors_flags(const struct bpf_path_ancestors_kern *kit)
 
 	if (kit->step == -ENOMEM)
 		return BPF_PATH_ANCESTORS_NOMEM;
+	if (kit->step == -ECHILD)
+		return BPF_PATH_ANCESTORS_RETRY;
 	if (!kit->step) {
 		if (kit->aw.pos_flags & VFS_WALK_POS_DISCONNECTED)
 			flags |= BPF_PATH_ANCESTORS_DISCONNECTED;
@@ -633,6 +656,79 @@ bpf_path_ancestors_pos_flags(struct bpf_iter_path_ancestors *it__iter)
 	return bpf_path_ancestors_flags((void *)it__iter);
 }
 
+__bpf_kfunc int
+bpf_iter_path_ancestors_rcu_new(struct bpf_iter_path_ancestors_rcu *it,
+				struct path *path, u64 flags)
+{
+	return bpf_path_ancestors_new((void *)it, path, flags, VFS_WALK_RCU);
+}
+
+/*
+ * Unlike the referenced variant, this hands out the walk's own position:
+ * a lockless iteration holds no references to pass on, and the verifier
+ * keeps the whole of it inside one RCU read-side critical section.
+ */
+__bpf_kfunc struct path *
+bpf_iter_path_ancestors_rcu_next(struct bpf_iter_path_ancestors_rcu *it)
+{
+	return bpf_path_ancestors_step((void *)it);
+}
+
+__bpf_kfunc void
+bpf_iter_path_ancestors_rcu_destroy(struct bpf_iter_path_ancestors_rcu *it)
+{
+	vfs_walk_end(&((struct bpf_path_ancestors_kern *)it)->aw);
+}
+
+__bpf_kfunc u32
+bpf_path_ancestors_rcu_pos_flags(struct bpf_iter_path_ancestors_rcu *it__iter)
+{
+	return bpf_path_ancestors_flags((void *)it__iter);
+}
+
+/**
+ * bpf_path_ancestors_legitimize - hand a lockless iteration over to references
+ * @it__uninit: referenced ancestor iterator to begin at @rcu_it__iter's
+ *              current position; destroy it with
+ *              bpf_iter_path_ancestors_destroy() whether this succeeds or not
+ * @rcu_it__iter: lockless ancestor iterator, on the position to escalate at
+ *
+ * Mirrors unlazy_walk(): acquires the lockless iteration's current position
+ * and leaves @it__uninit ready to continue from it with references, which
+ * its first bpf_iter_path_ancestors_next() then yields - necessarily after
+ * the program has left its RCU read-side critical section, since that kfunc
+ * is sleepable.  Sleepable work on the escalated position therefore happens
+ * on the iteration's own reference, and nothing is allocated here.
+ *
+ * Return: 0, -%ENOENT if the lockless iteration was not on a position, or
+ * -%ECHILD if it lost the race to acquire one; %BPF_PATH_ANCESTORS_RETRY is
+ * then also flagged, and the program has reached no conclusion about the
+ * ancestry.  @it__uninit is initialized whatever this returns, so a program
+ * need not branch on the result: a walk that could not be escalated simply
+ * yields no position.
+ */
+__bpf_kfunc int
+bpf_path_ancestors_legitimize(struct bpf_iter_path_ancestors *it__uninit,
+			      struct bpf_iter_path_ancestors_rcu *rcu_it__iter)
+{
+	struct bpf_path_ancestors_kern *rcu_kit = (void *)rcu_it__iter;
+	struct bpf_path_ancestors_kern *kit = (void *)it__uninit;
+
+	/* A zeroed walk makes destroying the iterator a no-op. */
+	memset(kit, 0, sizeof(*kit));
+	kit->step = 1;
+
+	/* Drained, or already failed: nothing to hand over. */
+	if (rcu_kit->step)
+		return -ENOENT;
+	if (!vfs_walk_handover(&kit->aw, &rcu_kit->aw)) {
+		rcu_kit->step = -ECHILD;
+		return -ECHILD;
+	}
+	kit->step = 0;
+	return 0;
+}
+
 __bpf_kfunc void bpf_path_put(struct path *path)
 {
 	path_put(path);
@@ -659,6 +755,11 @@ BTF_ID_FLAGS(func, bpf_iter_path_ancestors_next,
 	     KF_ITER_NEXT | KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_iter_path_ancestors_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_path_ancestors_pos_flags)
+BTF_ID_FLAGS(func, bpf_iter_path_ancestors_rcu_new, KF_ITER_NEW | KF_RCU_PROTECTED)
+BTF_ID_FLAGS(func, bpf_iter_path_ancestors_rcu_next, KF_ITER_NEXT | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_iter_path_ancestors_rcu_destroy, KF_ITER_DESTROY)
+BTF_ID_FLAGS(func, bpf_path_ancestors_rcu_pos_flags)
+BTF_ID_FLAGS(func, bpf_path_ancestors_legitimize)
 BTF_ID_FLAGS(func, bpf_path_put, KF_RELEASE | KF_SLEEPABLE)
 BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
 
-- 
2.55.0

[RFC PATCH bpf-next 12/12] selftests/bpf: exercise the lockless path ancestor iterator

[Justin Suess]

Walk the same ancestry four ways and require the position counts to
agree: lockless from a non-sleepable program, where the RCU critical
section is implicit; lockless under an explicit bpf_rcu_read_lock();
referenced; and the hybrid that walks lockless to the second position,
hands it over to a referenced iteration, and resumes there - the
escalated position therefore being walked twice, once per mode.

The sleepable work the escalation exists for (d_path, an xattr read
through the position's dentry) runs on the resumed iteration's first
position, after bpf_rcu_read_unlock(), which is the only place a
sleepable kfunc can run at all.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 .../selftests/bpf/prog_tests/path_ancestors.c | 33 ++++++-
 .../selftests/bpf/progs/path_ancestors.c      | 89 ++++++++++++++++++-
 2 files changed, 118 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/path_ancestors.c b/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
index 2de79673a13b..ce1ded844c3a 100644
--- a/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
+++ b/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
@@ -2,6 +2,7 @@
 /* Copyright (c) 2026 Justin Suess */
 
 #include <sys/stat.h>
+#include <sys/xattr.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <test_progs.h>
@@ -12,6 +13,8 @@ void test_path_ancestors(void)
 	char base[] = "/tmp/path_ancestors_XXXXXX";
 	struct path_ancestors *skel = NULL;
 	char suba[280], subb[280];
+	bool xattr_works;
+	int err;
 
 	if (!ASSERT_OK_PTR(mkdtemp(base), "mkdtemp"))
 		return;
@@ -20,6 +23,12 @@ void test_path_ancestors(void)
 	if (!ASSERT_OK(mkdir(suba, 0755), "mkdir_a"))
 		goto out_rm;
 
+	/* Read back by the program at the escalated position (== base). */
+	err = setxattr(base, "user.walk", "hello", 6, 0);
+	xattr_works = !err;
+	if (err && errno != EOPNOTSUPP && !ASSERT_OK(err, "setxattr"))
+		goto out_rm;
+
 	skel = path_ancestors__open_and_load();
 	if (!ASSERT_OK_PTR(skel, "open_and_load"))
 		goto out_rm;
@@ -31,14 +40,34 @@ void test_path_ancestors(void)
 	if (!ASSERT_OK(mkdir(subb, 0755), "mkdir_b"))
 		goto out;
 
-	/* suba, base, /tmp, / at least. */
-	ASSERT_GE(skel->bss->ref_count, 3, "ref_count");
+	ASSERT_EQ(skel->bss->test_err, 0, "test_err");
+	ASSERT_EQ(skel->bss->escalate_err, 0, "escalate_err");
+	/* suba, base, /tmp, / at least; equality across modes is the point. */
+	ASSERT_GE(skel->bss->rcu_count, 3, "rcu_count");
+	ASSERT_EQ(skel->bss->ref_count, skel->bss->rcu_count, "ref_vs_rcu");
+	ASSERT_EQ(skel->bss->rcu_ns_count, skel->bss->rcu_count,
+		  "nonsleepable_vs_rcu");
+	/*
+	 * The escalated position is walked twice: once lockless, then again
+	 * as the resumed referenced iteration's first position.
+	 */
+	ASSERT_EQ(skel->bss->hybrid_count, skel->bss->rcu_count + 1,
+		  "hybrid_vs_rcu");
+	ASSERT_EQ(skel->bss->retry_flags, 0, "no_retry");
 	ASSERT_EQ(skel->bss->ref_flags, 0, "ref_flags");
 
 	/* The acquired second position, used after its step was taken. */
 	ASSERT_STREQ(skel->bss->second_path, base, "second_path");
 	ASSERT_EQ(skel->bss->second_len, strlen(base) + 1, "second_len");
 
+	/* The escalated position is the walk's second one: base. */
+	ASSERT_STREQ(skel->bss->escalated_path, base, "escalated_path");
+	ASSERT_EQ(skel->bss->escalated_len, strlen(base) + 1, "escalated_len");
+	if (xattr_works) {
+		ASSERT_EQ(skel->bss->xattr_ret, 6, "xattr_len");
+		ASSERT_STREQ(skel->bss->xattr_value, "hello", "xattr_value");
+	}
+
 out:
 	path_ancestors__destroy(skel);
 out_rm:
diff --git a/tools/testing/selftests/bpf/progs/path_ancestors.c b/tools/testing/selftests/bpf/progs/path_ancestors.c
index af6b777e8bec..50ce0ce163dd 100644
--- a/tools/testing/selftests/bpf/progs/path_ancestors.c
+++ b/tools/testing/selftests/bpf/progs/path_ancestors.c
@@ -11,29 +11,74 @@ char _license[] SEC("license") = "GPL";
 
 __u32 monitored_pid;
 
+int rcu_count;		/* positions seen by the pure lockless walk */
+int rcu_ns_count;	/* ditto, from the non-sleepable program */
 int ref_count;		/* positions seen by the pure referenced walk */
+int hybrid_count;	/* positions seen by the lockless+escalate walk */
+int retry_flags;	/* BPF_PATH_ANCESTORS_RETRY observations */
 int ref_flags;		/* pos flags seen by the referenced walk */
 int second_len;		/* d_path length of the walk's second position */
+int xattr_ret;		/* xattr read at the escalated position */
+int escalated_len;	/* d_path length of the escalated position */
+int escalate_err;	/* bpf_path_ancestors_legitimize() result */
+int test_err;
 char second_path[256];
+char escalated_path[256];
+char xattr_value[16];
 
 static bool monitored(void)
 {
 	return (bpf_get_current_pid_tgid() >> 32) == monitored_pid;
 }
 
+/*
+ * Lockless walk from a non-sleepable program: the RCU critical section is
+ * implicit, no bpf_rcu_read_lock() needed.
+ */
+SEC("lsm/path_mkdir")
+int BPF_PROG(rcu_nonsleepable, const struct path *dir, struct dentry *dentry,
+	     umode_t mode)
+{
+	struct bpf_iter_path_ancestors_rcu rit;
+
+	if (!monitored())
+		return 0;
+
+	bpf_iter_path_ancestors_rcu_new(&rit, (struct path *)dir, 0);
+	while (bpf_iter_path_ancestors_rcu_next(&rit))
+		rcu_ns_count++;
+	retry_flags |= bpf_path_ancestors_rcu_pos_flags(&rit);
+	bpf_iter_path_ancestors_rcu_destroy(&rit);
+	return 0;
+}
+
 SEC("lsm.s/path_mkdir")
 int BPF_PROG(walk_modes, const struct path *dir, struct dentry *dentry,
 	     umode_t mode)
 {
+	struct bpf_iter_path_ancestors_rcu rit;
 	struct bpf_iter_path_ancestors it;
+	struct bpf_dynptr value_ptr;
 	struct path *pos;
 
 	if (!monitored())
 		return 0;
 
 	/*
-	 * Referenced walk: every position comes acquired, so it stays valid
-	 * for sleepable work and past the step that yielded it.
+	 * Mode 1: pure lockless, under an explicit RCU critical section.
+	 * Positions are borrowed, so nothing is released here.
+	 */
+	bpf_rcu_read_lock();
+	bpf_iter_path_ancestors_rcu_new(&rit, (struct path *)dir, 0);
+	while (bpf_iter_path_ancestors_rcu_next(&rit))
+		rcu_count++;
+	retry_flags |= bpf_path_ancestors_rcu_pos_flags(&rit);
+	bpf_iter_path_ancestors_rcu_destroy(&rit);
+	bpf_rcu_read_unlock();
+
+	/*
+	 * Mode 2: pure referenced.  Every position comes acquired, so it
+	 * stays valid for sleepable work and past the step that yielded it.
 	 */
 	bpf_iter_path_ancestors_new(&it, (struct path *)dir, 0);
 	while ((pos = bpf_iter_path_ancestors_next(&it))) {
@@ -45,5 +90,45 @@ int BPF_PROG(walk_modes, const struct path *dir, struct dentry *dentry,
 		bpf_path_put(pos);
 	}
 	bpf_iter_path_ancestors_destroy(&it);
+
+	/*
+	 * Mode 3: hybrid.  Walk lockless to the second position, then hand
+	 * that position over to a referenced iteration which resumes there.
+	 */
+	bpf_rcu_read_lock();
+	bpf_iter_path_ancestors_rcu_new(&rit, (struct path *)dir, 0);
+	while (bpf_iter_path_ancestors_rcu_next(&rit)) {
+		hybrid_count++;
+		if (hybrid_count == 2)
+			break;
+	}
+	escalate_err = bpf_path_ancestors_legitimize(&it, &rit);
+	retry_flags |= bpf_path_ancestors_rcu_pos_flags(&rit);
+	bpf_iter_path_ancestors_rcu_destroy(&rit);
+	bpf_rcu_read_unlock();
+
+	if (escalate_err)
+		test_err = 1;
+
+	/*
+	 * Out of the RCU critical section.  The resumed iteration's first
+	 * position is the escalated one, kept alive by the reference the
+	 * iteration hands out, so sleepable work can run on it.
+	 */
+	while ((pos = bpf_iter_path_ancestors_next(&it))) {
+		hybrid_count++;
+		if (hybrid_count == 3) {
+			escalated_len = bpf_path_d_path(pos, escalated_path,
+							sizeof(escalated_path));
+			bpf_dynptr_from_mem(xattr_value, sizeof(xattr_value),
+					    0, &value_ptr);
+			/* A trusted path's dentry is trusted, never NULL. */
+			xattr_ret = bpf_get_dentry_xattr(pos->dentry,
+							 "user.walk",
+							 &value_ptr);
+		}
+		bpf_path_put(pos);
+	}
+	bpf_iter_path_ancestors_destroy(&it);
 	return 0;
 }
-- 
2.55.0

Re: [RFC PATCH bpf-next 04/12] bpf: mark struct path trusted

[bot+bpf-ci]

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

> bpf: mark struct path trusted

TL;DR: Marking struct path trusted may let a MEM_RCU path's dentry (from the
later RCU ancestor iterator) be typed PTR_TRUSTED, so sleepable kfuncs could
possibly use a freed dentry after RCU unlock.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -6308,6 +6308,10 @@ BTF_TYPE_SAFE_TRUSTED(struct scx_sub_detach_args) {
>  	struct sched_ext_ops *ops;
>  };
>
> +BTF_TYPE_SAFE_TRUSTED(struct path) {
> +	struct dentry *dentry;
> +};
> +

Can this entry also apply to a struct path pointer that is MEM_RCU rather
than trusted?

check_ptr_to_btf_access() consults the same allow-list for both kinds of
parent:

	} else if (is_trusted_reg(env, reg) || is_rcu_reg(reg)) {
		...
		if (type_is_trusted(env, reg, field_name, btf_id)) {
			flag |= PTR_TRUSTED;

The commit message only justifies the entry for a trusted struct path:
"Walking a trusted struct path yields a trusted dentry - a live path never
carries a NULL dentry".  The comment above the SAFE_TRUSTED group also says
these fields are trusted even outside of an RCU critical section.  That
holds for LSM hook arguments and for paths returned by an acquiring kfunc,
but it does not seem to hold for a MEM_RCU struct path pointer.

Later commits in this series create such a pointer.  Commit 47d25368b3cc
("bpf: add a lockless path ancestor iterator") adds
bpf_iter_path_ancestors_rcu_new() with KF_RCU_PROTECTED, and
bpf_iter_path_ancestors_rcu_next() with KF_ITER_NEXT | KF_RET_NULL and no
KF_ACQUIRE.  check_kfunc_call() types the result of an iter_next on a
MEM_RCU iterator as PTR_TO_BTF_ID | MEM_RCU, and the kfunc returns
&kit->aw.pos, the walk's own borrowed position.  That commit's message says
nothing read out of the position may be passed to a kfunc demanding a
trusted argument, but with this patch the first thing a program does
breaks that rule:

	pos = bpf_iter_path_ancestors_rcu_next(&rit);	/* PTR_TO_BTF_ID | MEM_RCU */
	d = pos->dentry;	/* now PTR_TO_BTF_ID | PTR_TRUSTED */

The register d carries only PTR_TRUSTED, not MEM_RCU.
invalidate_rcu_protected_refs() only downgrades MEM_RCU registers, so d
stays trusted after bpf_rcu_read_unlock().  A sleepable LSM program could
then do:

	bpf_rcu_read_lock();
	bpf_iter_path_ancestors_rcu_new(&rit, path, 0);
	pos = bpf_iter_path_ancestors_rcu_next(&rit);
	d = pos->dentry;
	...
	bpf_iter_path_ancestors_rcu_destroy(&rit);
	bpf_rcu_read_unlock();
	bpf_get_dentry_xattr(d, ...);	/* or bpf_set_dentry_xattr() /
					   bpf_remove_dentry_xattr() */

The verifier would accept this because d is PTR_TRUSTED.  In an rcu-walk
the position holds no reference on the dentry, so the dentry may already
have been killed and freed once the RCU section has ended.  Can the
sleepable kfunc then operate on a freed dentry?

Even inside the critical section, "trusted" promises a non-zero refcount,
which the lockless walk does not provide.

The position is also storage inside the iterator on the BPF stack, and pos
is not invalidated by bpf_iter_path_ancestors_rcu_destroy().  The destroy
releases the iterator's reference id, but the registers returned by
iter_next carry neither that id nor a parent_id.  So pos->dentry can be
re-read after the slot has been overwritten with program-chosen data, and
the result is still typed as a trusted dentry.  Before this patch the same
load gave a non-trusted pointer, which kfuncs reject.

Nothing later in the series restores the invariant: the verifier.c diff for
the rest of the series (7ed5aa71ad77..70352ce0817c) does not touch
type_is_trusted(), check_ptr_to_btf_access() or this path entry.  The
selftests only read pos->dentry on the referenced iterator, so they do not
exercise this case.

Should this entry apply only to trusted/referenced struct path parents, for
example by not consulting the SAFE_TRUSTED list for MEM_RCU parents of this
type?  Alternatively, could the rcu iterator return a position that the
verifier marks PTR_UNTRUSTED, or could the rcu variant return a dentry that
is MEM_RCU-tagged instead of PTR_TRUSTED?

[ ... ]

> @@ -6360,6 +6364,7 @@ static bool type_is_trusted(struct bpf_verifier_env *env,
>  	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct scx_cpu_release_args));
>  	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct scx_sub_attach_args));
>  	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct scx_sub_detach_args));
> +	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct path));
>
>  	return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id, "__safe_trusted");
>  }


---
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/37395354107

Re: [RFC PATCH bpf-next 12/12] selftests/bpf: exercise the lockless path ancestor iterator

[bot+bpf-ci]

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

> selftests/bpf: exercise the lockless path ancestor iterator

TL;DR: Test issue in path_ancestors selftest: if setxattr() gives EOPNOTSUPP,
the xattr checks are silently dropped and the run reports a full pass. Also a
redundant test_err variable and a stale comment (nits).

> diff --git a/tools/testing/selftests/bpf/prog_tests/path_ancestors.c b/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
> index 2de79673a13b..ce1ded844c3a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
> +++ b/tools/testing/selftests/bpf/prog_tests/path_ancestors.c

[ ... ]

> @@ -20,6 +23,12 @@ void test_path_ancestors(void)
>  	if (!ASSERT_OK(mkdir(suba, 0755), "mkdir_a"))
>  		goto out_rm;
>  
> +	/* Read back by the program at the escalated position (== base). */
> +	err = setxattr(base, "user.walk", "hello", 6, 0);
> +	xattr_works = !err;
> +	if (err && errno != EOPNOTSUPP && !ASSERT_OK(err, "setxattr"))
> +		goto out_rm;
> +

[ ... ]

> @@ -31,14 +40,34 @@ void test_path_ancestors(void)

[ ... ]

> +	/* The escalated position is the walk's second one: base. */
> +	ASSERT_STREQ(skel->bss->escalated_path, base, "escalated_path");
> +	ASSERT_EQ(skel->bss->escalated_len, strlen(base) + 1, "escalated_len");
> +	if (xattr_works) {
> +		ASSERT_EQ(skel->bss->xattr_ret, 6, "xattr_len");
> +		ASSERT_STREQ(skel->bss->xattr_value, "hello", "xattr_value");
> +	}
> +

When setxattr() fails with EOPNOTSUPP (for example /tmp on a tmpfs built
without CONFIG_TMPFS_XATTR), xattr_works is false and the xattr_ret and
xattr_value checks are dropped without any message.  test__skip() is not
called either, so the run reports a full pass.

A subsystem pattern flags this as potentially concerning: a test that
skips a capability-dependent check should say why, rather than skipping
silently.

The rest of the test (walk counts, d_path on the escalated position)
still runs, and the program's bpf_get_dentry_xattr() call just returns an
error that nothing looks at, so nothing fails spuriously.  But the xattr
read is one of the two sleepable operations the commit message says the
escalation exists for, and its coverage disappears from the report.

prog_tests/fs_kfuncs.c handles the same errno from setxattr() on /tmp
explicitly:

	if (err && errno == 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();
		...
	}

Could this test print a similar note, or run the xattr check as a subtest
that reports itself skipped, when xattr_works is false?

> diff --git a/tools/testing/selftests/bpf/progs/path_ancestors.c b/tools/testing/selftests/bpf/progs/path_ancestors.c
> index af6b777e8bec..50ce0ce163dd 100644
> --- a/tools/testing/selftests/bpf/progs/path_ancestors.c
> +++ b/tools/testing/selftests/bpf/progs/path_ancestors.c
> @@ -11,29 +11,74 @@ char _license[] SEC("license") = "GPL";

[ ... ]

> +int retry_flags;	/* BPF_PATH_ANCESTORS_RETRY observations */

[ ... ]

> +int escalate_err;	/* bpf_path_ancestors_legitimize() result */
> +int test_err;

[ ... ]

> @@ -45,5 +90,45 @@ int BPF_PROG(walk_modes, const struct path *dir, struct dentry *dentry,

[ ... ]

> +	escalate_err = bpf_path_ancestors_legitimize(&it, &rit);
> +	retry_flags |= bpf_path_ancestors_rcu_pos_flags(&rit);
> +	bpf_iter_path_ancestors_rcu_destroy(&rit);
> +	bpf_rcu_read_unlock();
> +
> +	if (escalate_err)
> +		test_err = 1;

this isn't a bug, but is test_err needed alongside escalate_err?  It is
set in exactly one place, from escalate_err being nonzero:

	if (escalate_err)
		test_err = 1;

and the userspace half asserts both of them are 0 one after the other:

	ASSERT_EQ(skel->bss->test_err, 0, "test_err");
	ASSERT_EQ(skel->bss->escalate_err, 0, "escalate_err");

Could the program keep just escalate_err, which already records the actual
error code, and have the test assert only that?

test_err is also the only new global without a describing comment.

Separately, the retry_flags comment only mentions BPF_PATH_ANCESTORS_RETRY
observations, but at the hybrid escalation point it ORs in the whole
bpf_path_ancestors_rcu_pos_flags() result, not just the RETRY bit.


---
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/37395354107

Re: [RFC PATCH bpf-next 07/12] selftests/bpf: exercise the path ancestor iterator

[bot+bpf-ci]

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

> selftests/bpf: exercise the path ancestor iterator

TL;DR:
- Selftest's ref_count >= 3 may not verify the walk reaches /; early stop or
  bad mount crossing could pass
- Test may not exercise using a position after a later step, as its commit
  message and comment claim

> diff --git a/tools/testing/selftests/bpf/prog_tests/path_ancestors.c b/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
> new file mode 100644
> index 000000000000..2de79673a13b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
> @@ -0,0 +1,48 @@

[ ... ]

> +	/* suba, base, /tmp, / at least. */
> +	ASSERT_GE(skel->bss->ref_count, 3, "ref_count");
> +	ASSERT_EQ(skel->bss->ref_flags, 0, "ref_flags");

The comment lists four positions (suba, base, /tmp, /), and the commit
message says the test checks "that the iteration reaches the root: the
temporary directory, its parent and / are visited".  Does ref_count >= 3
verify that?

For /tmp/path_ancestors_XXXXXX/a the walk should yield at least four
positions, so a count of exactly 3 would mean the walk stopped before /.
Two cases that would pass this check:

- a broken mount crossing out of a tmpfs /tmp, where the iteration ends at
  the tmpfs root.

- an iteration ended early by the allocation failure path in
  bpf_iter_path_ancestors_next():

    held = kmalloc_obj(*held);
    if (!held) {
        kit->step = -ENOMEM;
        return NULL;
    }

The ref_flags check cannot catch the second case, since
bpf_path_ancestors_pos_flags() is only called from the loop for positions
that were returned, and at that point kit->step is still 0.

Should this be ASSERT_GE(skel->bss->ref_count, 4, "ref_count")?  Should the
program also record bpf_path_ancestors_pos_flags() after the loop ends and
check that the last position was the root, so that "reaches the root" is
actually verified?

The follow-up commit 70352ce0817c ("selftests/bpf: exercise the lockless
path ancestor iterator") keeps the same comment and the same ">= 3" lower
bound (on rcu_count), so this is not tightened later in the series.

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/path_ancestors.c b/tools/testing/selftests/bpf/progs/path_ancestors.c
> new file mode 100644
> index 000000000000..af6b777e8bec
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/path_ancestors.c
> @@ -0,0 +1,49 @@

[ ... ]

> +	/*
> +	 * Referenced walk: every position comes acquired, so it stays valid
> +	 * for sleepable work and past the step that yielded it.
> +	 */
> +	bpf_iter_path_ancestors_new(&it, (struct path *)dir, 0);
> +	while ((pos = bpf_iter_path_ancestors_next(&it))) {
> +		ref_count++;
> +		ref_flags |= bpf_path_ancestors_pos_flags(&it);
> +		if (ref_count == 2)
> +			second_len = bpf_path_d_path(pos, second_path,
> +						     sizeof(second_path));
> +		bpf_path_put(pos);
> +	}

The commit message says the second position's pathname is resolved "from
the acquired position after the step that yielded it has been taken, so
that the acquired reference is what the sleepable kfunc runs on rather than
the walk's", and the test comment says "The acquired second position, used
after its step was taken."  Does this loop exercise that?

bpf_path_d_path() does not look sleepable.  It is registered without
KF_SLEEPABLE:

    BTF_ID_FLAGS(func, bpf_path_d_path)

Also, bpf_path_d_path(pos, ...) is called before the next
bpf_iter_path_ancestors_next() call.  At that point the walk's own view
still refers to the same position with its own reference, because
bpf_path_ancestors_step() returns &kit->aw.pos and vfs_walk_step_ref() only
drops that reference at the next step:

    dput(aw->pos.dentry);
    aw->pos.dentry = parent;

So during bpf_path_d_path() the position is held by both the acquired copy
and the walk, and the assertion on second_path cannot tell whether the
acquired reference is what keeps it alive.  Choosing ref_count == 2 instead
of the first position makes no difference here.

The commit 8dd2a68fe0e7 ("bpf: add a path ancestor iterator") justifies the
per-position allocation and KF_ACQUIRE with "a position that outlives the
step it came from ... has to be kept alive by the program's reference
rather than by the iterator's".  This test never uses a position after a
later step.

Could the program use a position after the next step, without a loop?  For
example:

    pos1 = bpf_iter_path_ancestors_next(&it);
    pos2 = bpf_iter_path_ancestors_next(&it);
    bpf_path_put(pos2);
    bpf_path_d_path(pos1, ...);
    bpf_path_put(pos1);

Otherwise, could the commit message and the test comment be reworded to
match what is tested?  The follow-up commit 70352ce0817c leaves this check
and comment unchanged.


---
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/37395354107

Re: [RFC PATCH bpf-next 11/12] bpf: add a lockless path ancestor iterator

[bot+bpf-ci]

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

> bpf: add a lockless path ancestor iterator

TL;DR:
- Verifier may give a plain trusted dentry from the lockless path iterator,
  risking use-after-free in sleepable kfuncs after RCU exit
- A failed legitimize may look like a finished walk, hiding a lost race

> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 265cb414a08a..9dad65df4267 100644
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c

[ ... ]

> @@ -633,6 +656,79 @@ bpf_path_ancestors_pos_flags(struct bpf_iter_path_ancestors *it__iter)

[ ... ]

> +/*
> + * Unlike the referenced variant, this hands out the walk's own position:
> + * a lockless iteration holds no references to pass on, and the verifier
> + * keeps the whole of it inside one RCU read-side critical section.
> + */
> +__bpf_kfunc struct path *
> +bpf_iter_path_ancestors_rcu_next(struct bpf_iter_path_ancestors_rcu *it)
> +{
> +	return bpf_path_ancestors_step((void *)it);
> +}

The commit message says that "nothing read out of the position may be passed
to a kfunc demanding a trusted argument", and the comment above struct
bpf_iter_path_ancestors says the same.  Does the verifier actually enforce
this?

The position returned by bpf_iter_path_ancestors_rcu_next() is typed
PTR_TO_BTF_ID | MEM_RCU, because check_kfunc_call() adds MEM_RCU to the
iter_next result of a KF_RCU_PROTECTED iterator.  Commit 7ed5aa71ad77 ("bpf:
mark struct path trusted") added:

    BTF_TYPE_SAFE_TRUSTED(struct path) {
            struct dentry *dentry;
    };

In check_ptr_to_btf_access(), the branch taken for
is_trusted_reg(env, reg) || is_rcu_reg(reg) applies type_is_trusted() and
sets PTR_TRUSTED on a safe-trusted field even when the base pointer is only
MEM_RCU.

So loading the dentry field of a lockless position yields a plain
PTR_TRUSTED dentry.  It does not keep MEM_RCU, so
invalidate_rcu_protected_refs() leaves it alone at bpf_rcu_read_unlock().

The safe-trusted rule for struct path assumes the path pins its dentry.  That
holds for the referenced variant, but the lockless walk takes no reference on
the dentry, and an ancestor's lifetime is not tied to the original path once
a concurrent rename moves the descendant.

After the program leaves the RCU read-side section, could a sleepable kfunc
that takes a trusted dentry (the xattr kfuncs in this set, for example) be
handed a dentry that has neither a reference nor RCU protection?

Would it work to keep MEM_RCU on fields loaded from a MEM_RCU struct path
(for example by applying the struct path safe-trusted rule only to trusted
bases), or to avoid exposing struct path as the return type of the lockless
iterator?  A negative selftest showing that such a load is rejected would
also help.

[ ... ]

> + * Return: 0, -%ENOENT if the lockless iteration was not on a position, or
> + * -%ECHILD if it lost the race to acquire one; %BPF_PATH_ANCESTORS_RETRY is
> + * then also flagged, and the program has reached no conclusion about the
> + * ancestry.  @it__uninit is initialized whatever this returns, so a program
> + * need not branch on the result: a walk that could not be escalated simply
> + * yields no position.
> + */
> +__bpf_kfunc int
> +bpf_path_ancestors_legitimize(struct bpf_iter_path_ancestors *it__uninit,
> +			      struct bpf_iter_path_ancestors_rcu *rcu_it__iter)
> +{
> +	struct bpf_path_ancestors_kern *rcu_kit = (void *)rcu_it__iter;
> +	struct bpf_path_ancestors_kern *kit = (void *)it__uninit;
> +
> +	/* A zeroed walk makes destroying the iterator a no-op. */
> +	memset(kit, 0, sizeof(*kit));
> +	kit->step = 1;
> +
> +	/* Drained, or already failed: nothing to hand over. */
> +	if (rcu_kit->step)
> +		return -ENOENT;
> +	if (!vfs_walk_handover(&kit->aw, &rcu_kit->aw)) {
> +		rcu_kit->step = -ECHILD;
> +		return -ECHILD;
> +	}
> +	kit->step = 0;
> +	return 0;
> +}

When the lockless iteration had already failed with -ECHILD, or when
vfs_walk_handover() fails, @it__uninit is left with step == 1.  That is the
same state as a referenced walk that has passed the real root, so
bpf_iter_path_ancestors_next() returns NULL and bpf_path_ancestors_pos_flags()
on that iterator returns 0.

The documented contract of bpf_iter_path_ancestors_next() says NULL comes
"once the walk has passed the real root - or on an allocation failure ...
reported as NOMEM", so a lost race reads as a completed walk.  The kernel-doc
above also tells programs they "need not branch on the result".

Can a policy program that follows that advice and only inspects the resumed
iterator treat a lost race as a finished ancestry walk with no match?
BPF_PATH_ANCESTORS_RETRY is flagged only on the lockless iterator, which has
to be destroyed before the resumed iteration can run.

Would setting kit->step = -ECHILD on the destination in both failure paths
work?  Stepping would still stop (step != 0), the zeroed walk would keep
destroy a no-op, and bpf_path_ancestors_pos_flags() on the resumed iterator
would report BPF_PATH_ANCESTORS_RETRY.

Separately, when the source had already lost a race, the first check returns
-ENOENT rather than -ECHILD, which conflicts with the kernel-doc describing
-ENOENT as "not on a position".


---
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/37395354107

Re: [RFC PATCH bpf-next 03/12] landlock: convert ancestor walk to vfs_walk_ancestors()

[bot+bpf-ci]

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

> landlock: convert ancestor walk to vfs_walk_ancestors()

TL;DR: Nit only: a comment in landlock's check_access_path_walk() refers to
"the old loop" that no longer exists; it should say why mountpoints reached on
a mount crossing are skipped.

> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index cab43892ec2f..369e2e9c83ad 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c

[ ... ]

> @@ -751,6 +751,107 @@ static void test_is_eacces_with_write(struct kunit *const test)
> +static int check_access_path_walk(const struct path *const ancestor,
> +				  const unsigned int pos_flags,
> +				  void *const data)
> +{
> +	struct landlock_walk_state *const state = data;
> +	struct landlock_id id = {
> +		.type = LANDLOCK_KEY_INODE,
> +	};
> +
> +	if (unlikely(pos_flags & VFS_WALK_POS_DISCONNECTED)) {

[ ... ]

> +		/*
> +		 * The old loop never visited the mountpoints a mount
> +		 * crossing lands on: don't match them against rules.  Other
> +		 * disconnected roots keep being matched below, as before.
> +		 */
> +		if (pos_flags & VFS_WALK_POS_MOUNTPOINT)
> +			return VFS_WALK_CONTINUE;
> +	}

This isn't a bug, but could this comment say why mountpoints reached on a
mount crossing are skipped (they are hidden under the mount above them)?

As written it refers to "The old loop" and to "as before", and once this
patch is applied there is no old loop left in the tree for a reader of
check_access_path_walk() to compare against.

The removed code stated the reason directly:

-				/* Ignores hidden mount points. */
-				goto jump_up;

The history of the conversion is already covered by the commit message.


---
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/37395354107

Re: [RFC PATCH bpf-next 09/12] namei: add an rcu-walk mode to the ancestor walk

[bot+bpf-ci]

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

> namei: add an rcu-walk mode to the ancestor walk

TL;DR: vfs_walk_handover() may drop the MOUNTPOINT flag on a disconnected
root, so an escalated walk could report different position flags and give
Landlock a different verdict than the lockless walk.

> diff --git a/fs/namei.c b/fs/namei.c
> index 73f25152d917..31f96602d4e9 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c

[ ... ]

> @@ -2286,6 +2330,35 @@ static int vfs_walk_step_ref(struct vfs_ancestor_walk *aw)

[ ... ]

> +	/* A crossing onto a disconnected root, as in vfs_walk_step_ref(). */
> +	aw->pos_flags = parent == aw->pos.dentry ?
> +		VFS_WALK_POS_DISCONNECTED | VFS_WALK_POS_MOUNTPOINT :
> +		vfs_walk_pos_flags(aw->pos.mnt, parent);
> +	aw->pos.dentry = parent;
> +	aw->seq = next_seq;
> +	return 0;
> +}
> +
>  /**

[ ... ]

> @@ -2308,6 +2383,10 @@ int vfs_walk_next(struct vfs_ancestor_walk *aw)
>  		aw->flags |= VFS_WALK_STARTED;
>  		aw->pos_flags = vfs_walk_pos_flags(aw->pos.mnt, aw->pos.dentry);
>  	}

[ ... ]

> @@ -2317,7 +2396,59 @@ int vfs_walk_next(struct vfs_ancestor_walk *aw)

[ ... ]

> +bool vfs_walk_handover(struct vfs_ancestor_walk *to,
> +		       struct vfs_ancestor_walk *from)
> +{
> +	struct path pos = from->pos;
> +	int err;
> +
> +	err = __legitimize_mnt(pos.mnt, from->m_seq);
> +	if (unlikely(err)) {
> +		if (err < 0)
> +			mnt_undo_legitimize(real_mount(pos.mnt));
> +		goto dead;
> +	}
> +	if (unlikely(read_seqcount_retry(&pos.dentry->d_seq, from->seq) ||
> +		     !lockref_get_not_dead(&pos.dentry->d_lockref))) {
> +		mnt_undo_legitimize(real_mount(pos.mnt));
> +		goto dead;
> +	}
> +	to->pos = pos;
> +	to->flags = 0;
> +	to->pos_flags = 0;
> +	return true;

Can the first vfs_walk_next() on @to yield the same VFS_WALK_POS_* flags
that @from reported for this position?

vfs_walk_step_rcu() can leave @from on a disconnected root with
two flags set:

    aw->pos_flags = parent == aw->pos.dentry ?
            VFS_WALK_POS_DISCONNECTED | VFS_WALK_POS_MOUNTPOINT :
            vfs_walk_pos_flags(aw->pos.mnt, parent);

vfs_walk_handover() then sets @to up as a walk that has not started
(to->flags = 0, to->pos_flags = 0).  The first vfs_walk_next() on @to takes
the not-started branch and recomputes the flags from the position alone:

    aw->flags |= VFS_WALK_STARTED;
    aw->pos_flags = vfs_walk_pos_flags(aw->pos.mnt, aw->pos.dentry);

vfs_walk_pos_flags() can only return VFS_WALK_POS_DISCONNECTED or 0.  So if
the handover happens while @from sits on such a position, @from reports
DISCONNECTED | MOUNTPOINT, but @to yields the same dentry as DISCONNECTED
only.  The kernel-doc of vfs_walk_handover() says this position is what "the
first vfs_walk_next() on it yields".

Whether a disconnected root is a mountpoint a crossing landed on is walk
state, not something that can be recomputed from the position.  The
bpf_path_ancestors_pos_flags() changelog (8dd2a68fe0e7, "bpf: add a path
ancestor iterator") says a program needs it "to reproduce Landlock's
evaluation of disconnected positions", and check_access_path_walk() in
security/landlock/fs.c skips rule matching on MOUNTPOINT positions but
matches rules on positions flagged only DISCONNECTED.

A program that escalates with bpf_path_ancestors_legitimize() (47d25368b3cc,
later in this series) at such a position would see a plain disconnected
root through bpf_path_ancestors_pos_flags().  It could then match rules
against the mountpoint dentry, so the same path can get a different verdict
depending on whether the walk escalated.

The position is reachable, since d_set_mounted() accepts an IS_ROOT
disconnected dentry as a mountpoint, for example one obtained through
open_by_handle_at().

Should the flags be carried over (to->pos_flags = from->pos_flags), with the
not-started branch of vfs_walk_next() ORing in vfs_walk_pos_flags() instead
of overwriting the flags?  vfs_walk_start() already zeroes pos_flags, so
ordinary walks would be unaffected.

No later commit in 3cc40137a219..70352ce0817c touches fs/namei.c.


---
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/37395354107

Re: [RFC PATCH bpf-next 06/12] bpf: add a path ancestor iterator

[bot+bpf-ci]

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

> bpf: add a path ancestor iterator

TL;DR:
- Possible use-after-free: dentry loaded from a position stays trusted after
  bpf_path_put()
- Possible i_rwsem deadlock or unlocked xattr change via ancestor dentries
- Commit message wrongly lists some non-sleepable LSM hooks

> bpf: add a path ancestor iterator
>
> Let BPF programs evaluate a path's ancestry with an open-coded iterator
> over the stepwise vfs_walk_ancestors() engine.  The iteration holds a
> reference on its current position, which is what lets a program sleep
> between positions - a dput() of the last reference may - so the kfuncs
> are KF_SLEEPABLE and the iterator is available to sleepable programs
> only.  That covers the LSM hooks a path-based policy attaches to:
> file_open, file_permission, the path_* hooks, mmap_file and bprm_* are
> all in the sleepable allowlist.

Is this description accurate?  sleepable_lsm_hooks in kernel/bpf/bpf_lsm.c,
which bpf_lsm_is_sleepable_hook() checks, does not appear to contain
bpf_lsm_file_permission, bpf_lsm_path_mknod, bpf_lsm_path_chroot or
bpf_lsm_path_post_mknod.

The iterator's bpf_iter_path_ancestors_new(), _next() and _destroy() are
all KF_SLEEPABLE, so programs attached to those hooks cannot use it.
path_mknod is one of the path hooks Landlock enforces on, and the message
presents reproducing Landlock's evaluation as the target.  No later commit
in the series adds these hooks to the allowlist.

Should the sentence be corrected, or should the hooks be added to the
allowlist?

> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 08f0847c4970..265cb414a08a 100644
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c
> @@ -13,7 +13,11 @@
>  #include <linux/kernfs.h>
>  #include <linux/lsm_hooks.h>
>  #include <linux/mm.h>
> +#include <linux/namei.h>
>  #include <linux/net.h>
> +#include <linux/slab.h>
> +
> +#include "internal.h"
>  #include <linux/xattr.h>

this isn't a bug, but should the local #include "internal.h" (and the blank
line before it) move below #include <linux/xattr.h>, so that the <linux/*.h>
block stays contiguous?

[ ... ]

> @@ -500,6 +504,143 @@ __bpf_kfunc struct inode *bpf_real_data_inode(struct file *file)

[ ... ]

> +__bpf_kfunc struct path *
> +bpf_iter_path_ancestors_next(struct bpf_iter_path_ancestors *it)
> +{
> +	struct bpf_path_ancestors_kern *kit = (void *)it;
> +	struct path *pos = bpf_path_ancestors_step(kit);
> +	struct path *held;
> +
> +	if (!pos)
> +		return NULL;
> +	/*
> +	 * The position must outlive the walk's own view of it, so it gets a
> +	 * reference and a struct path of its own to live in: struct path is
> +	 * a value type, with nothing a BPF reference could be taken on
> +	 * otherwise.  Sleepable, so no atomic allocation.
> +	 */
> +	held = kmalloc_obj(*held);
> +	if (!held) {
> +		kit->step = -ENOMEM;
> +		return NULL;
> +	}
> +	*held = *pos;
> +	path_get(held);
> +	return held;
> +}

Can the dentries reachable through this iterator be handed to
bpf_set_dentry_xattr() or bpf_remove_dentry_xattr() in a way that breaks the
parent-before-child i_rwsem order?

Both kfuncs are in the same kfunc set and both call
inode_lock(d_inode(dentry)).  They stay deadlock-free today because each
hook decides whether a dentry is locked: the trusted dentries a hook exposes
are its own arguments, and specialize_kfunc() swaps in the _locked variants
for the hooks in d_inode_locked_hooks.  A trusted ancestor reachable only
through this iterator is outside that model, because d_parent is not
trusted.

For example, with lsm.s/path_mkdir (sleepable, and not in
d_inode_locked_hooks):

Task A: mkdir("/a/b/c")
  filename_mkdirat() -> filename_create() -> start_dirop()
    inode_lock_nested(/a/b, I_MUTEX_PARENT)
  security_path_mkdir() -> BPF program iterates dir's ancestors
    2nd position = /a -> bpf_set_dentry_xattr(pos->dentry, ...)
      inode_lock(/a)                 <- blocks

Task B: rmdir("/a/b")
  do_rmdir() -> start_dirop()
    inode_lock_nested(/a, I_MUTEX_PARENT)
  vfs_rmdir()
    inode_lock(/a/b)                 <- blocks on Task A

Task A holds /a/b and waits for /a, while Task B holds /a and waits for
/a/b.

The first position (dir itself) already self-deadlocks on the rwsem that
Task A holds.  That case has been reachable as dir->dentry since 7ed5aa71ad77
("bpf: mark struct path trusted"), but ancestors are reachable only through
this commit.  The same thing happens from path_unlink, path_rmdir,
path_symlink, path_link and path_rename, which all run with a parent
directory locked.

In the d_inode_locked_hooks (inode_unlink, inode_rmdir, inode_setxattr,
...), bpf_set_dentry_xattr is rewritten to bpf_set_dentry_xattr_locked.  An
ancestor reached through the iterator (for example starting from
&bpf_get_task_exe_file()->f_path) would then be modified by
__vfs_setxattr() without its i_rwsem held.

No later commit in the series restricts these kfuncs for dentries derived
from the iterator.

[ ... ]

> +__bpf_kfunc void bpf_path_put(struct path *path)
> +{
> +	path_put(path);
> +	kfree(path);
> +}

Can a program keep using a dentry after bpf_path_put() has dropped the
reference of the position it was loaded from?

The commit message gives keeping a position alive as the reason for the
acquire/release pair:

  "a program that saves a position's dentry, or hands one to a sleepable
  kfunc, needs it to outlive the step it came from"

Nothing in the verifier ties a dentry loaded from an acquired position to
that position's reference.  Since 7ed5aa71ad77 ("bpf: mark struct path
trusted") added:

  BTF_TYPE_SAFE_TRUSTED(struct path) {
          struct dentry *dentry;
  }

check_ptr_to_btf_access() turns "d = pos->dentry" into
PTR_TO_BTF_ID | PTR_TRUSTED, and mark_btf_ld_reg() gives d no id and no
parent_id.  release_reference() only invalidates registers where
reg->id == id or reg->parent_id == id, so d is still a trusted dentry after
bpf_path_put(pos).

Before this commit, every struct path a program could reach was a hook
argument or was embedded in a trusted object, and stayed alive for the
whole program.  bpf_iter_path_ancestors_next() is the first source of a
struct path that can be released mid-program.

Example program in a sleepable LSM hook:

  bpf_iter_path_ancestors_new(&it, dir, 0);
  p = bpf_iter_path_ancestors_next(&it);   /* dir */
  bpf_path_put(p);
  p = bpf_iter_path_ancestors_next(&it);   /* an ancestor q */
  d = p->dentry;                           /* trusted, no ref link */
  bpf_path_put(p);
  bpf_iter_path_ancestors_destroy(&it);    /* walk's ref on q dropped */
  bpf_get_dentry_xattr(d, "security.bpf.x", &dynptr);  /* sleepable kfunc */

An ancestor q is pinned only by its child's d_parent reference.  If a
concurrent rename moves that child out of q and q is then rmdir'd, the put
and destroy in the program drop the last reference and the dentry is freed.
__d_free() goes through call_rcu(), and a sleepable program is not in a
normal RCU read-side section.

The later d->d_inode / xattr access is then a use-after-free.
bpf_set_dentry_xattr(d, ...) and the inode reached through d->d_inode
(trusted_or_null) are exposed the same way.

No later commit in the series ties derived pointers to the position's
reference.


---
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/37395354107