[PATCH v7 03/10] landlock: Use landlock_walk_path_up for collect_domain_accesses

[Justin Suess]

Use common path walk helper for collect_domain_accesses. This
extends the new centralized traversal logic to the current_check_refer
path code flow, and maintains consistency with the is_access_to_paths
allowed traversal while allowing reuse of traversal semantics.

Signed-off-by: Justin Suess <utilityemal77 at gmail.com>
---

Notes:
    v6..v7 changes:
    
      * New patch split out from the v6 core NO_INHERIT implementation.

 security/landlock/fs.c | 75 ++++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 35 deletions(-)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index b31bd2980e5c..7ec26c671f91 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1036,49 +1036,52 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
  * collect_domain_accesses - Walk through a file path and collect accesses
  *
  * @domain: Domain to check against.
- * @mnt_root: Last directory to check.
- * @dir: Directory to start the walk from.
+ * @path: Path to start the walk from and whose mount root is the last
+ *     directory to check.
  * @layer_masks_dom: Where to store the collected accesses.
  *
- * This helper is useful to begin a path walk from the @dir directory to a
- * @mnt_root directory used as a mount point.  This mount point is the common
- * ancestor between the source and the destination of a renamed and linked
- * file.  While walking from @dir to @mnt_root, we record all the domain's
- * allowed accesses in @layer_masks_dom.
+ * This helper is useful to begin a path walk from @path to the mount root
+ * directory used as a mount point.  This mount point is the common ancestor
+ * between the source and the destination of a renamed and linked file.  While
+ * walking from @path to that mount root, we record all the domain's allowed
+ * accesses in @layer_masks_dom.
  *
- * Because of disconnected directories, this walk may not reach @mnt_dir.  In
- * this case, the walk will continue to @mnt_dir after this call.
+ * Because of disconnected directories, this walk may not reach that mount
+ * root.  In this case, the walk will continue to the mount root after this
+ * call.
  *
  * This is similar to is_access_to_paths_allowed() but much simpler because it
  * only handles walking on the same mount point and only checks one set of
  * accesses.
  *
- * Return: True if all the domain access rights are allowed for @dir, false if
- * the walk reached @mnt_root.
+ * Return: True if all the domain access rights are allowed for @path, false if
+ * the walk reached the mount root.
  */
 static bool
 collect_domain_accesses(const struct landlock_ruleset *const domain,
-			const struct dentry *const mnt_root, struct dentry *dir,
+			const struct path *const path,
 			struct layer_access_masks *layer_masks_dom,
 			struct collected_rule_flags *const rule_flags)
 {
 	bool ret = false;
+	struct path walker_path;
 
-	if (WARN_ON_ONCE(!domain || !mnt_root || !dir || !layer_masks_dom))
+	if (WARN_ON_ONCE(!domain || !path || !path->dentry ||
+			 !path->mnt || !layer_masks_dom))
 		return true;
-	if (is_nouser_or_private(dir))
+	if (is_nouser_or_private(path->dentry))
 		return true;
 
 	if (!landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
 				       layer_masks_dom, LANDLOCK_KEY_INODE))
 		return true;
 
-	dget(dir);
+	walker_path = *path;
+	path_get(&walker_path);
 	while (true) {
-		struct dentry *parent_dentry;
-
 		/* Gets all layers allowing all domain accesses. */
-		if (landlock_unmask_layers(find_rule(domain, dir),
+		if (landlock_unmask_layers(find_rule(domain,
+						     walker_path.dentry),
 					   layer_masks_dom, rule_flags)) {
 			/*
 			 * Stops when all handled accesses are allowed by at
@@ -1092,14 +1095,16 @@ collect_domain_accesses(const struct landlock_ruleset *const domain,
 		 * Stops at the mount point or the filesystem root for a disconnected
 		 * directory.
 		 */
-		if (dir == mnt_root || unlikely(IS_ROOT(dir)))
+		if ((walker_path.dentry == path->mnt->mnt_root &&
+		     walker_path.mnt == path->mnt) ||
+		    unlikely(IS_ROOT(walker_path.dentry)))
 			break;
 
-		parent_dentry = dget_parent(dir);
-		dput(dir);
-		dir = parent_dentry;
+		if (WARN_ON_ONCE(landlock_walk_path_up(&walker_path) !=
+				 LANDLOCK_WALK_CONTINUE))
+			break;
 	}
-	dput(dir);
+	path_put(&walker_path);
 	return ret;
 }
 
@@ -1165,7 +1170,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 	bool allow_parent1, allow_parent2;
 	access_mask_t access_request_parent1, access_request_parent2;
 	struct path mnt_dir;
-	struct dentry *old_parent;
+	struct path old_parent_path;
 	struct layer_access_masks layer_masks_parent1 = {},
 				  layer_masks_parent2 = {};
 	struct landlock_request request1 = {}, request2 = {};
@@ -1223,19 +1228,19 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 	/*
 	 * old_dentry may be the root of the common mount point and
 	 * !IS_ROOT(old_dentry) at the same time (e.g. with open_tree() and
-	 * OPEN_TREE_CLONE).  We do not need to call dget(old_parent) because
-	 * we keep a reference to old_dentry.
+	 * OPEN_TREE_CLONE).  We do not need to call path_get(&old_parent_path)
+	 * because we keep a reference to old_dentry.
 	 */
-	old_parent = (old_dentry == mnt_dir.dentry) ? old_dentry :
-						      old_dentry->d_parent;
+	old_parent_path.mnt = mnt_dir.mnt;
+	old_parent_path.dentry = (old_dentry == mnt_dir.dentry) ?
+					 old_dentry :
+					 old_dentry->d_parent;
 
 	/* new_dir->dentry is equal to new_dentry->d_parent */
-	allow_parent1 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
-						old_parent,
-						&layer_masks_parent1,
-						&rule_flags_parent1);
-	allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
-						new_dir->dentry,
+	allow_parent1 = collect_domain_accesses(
+		subject->domain, &old_parent_path,
+		&layer_masks_parent1, &rule_flags_parent1);
+	allow_parent2 = collect_domain_accesses(subject->domain, new_dir,
 						&layer_masks_parent2,
 						&rule_flags_parent2);
 	if (allow_parent1 && allow_parent2)
@@ -1256,7 +1261,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 		return 0;
 
 	if (request1.access) {
-		request1.audit.u.path.dentry = old_parent;
+		request1.audit.u.path.dentry = old_parent_path.dentry;
 		request1.rule_flags = rule_flags_parent1;
 		landlock_log_denial(subject, &request1);
 	}
-- 
2.53.0