[PATCH 2/2] landlock: Remove collect_domain_accesses

[Justin Suess]

Remove collect_domain_accesses and replace with inline logic using the
new path walk helper in the check_current_refer_path.

Signed-off-by: Justin Suess <utilityemal77 at gmail.com>
---
 security/landlock/fs.c | 128 +++++++++++++++--------------------------
 1 file changed, 46 insertions(+), 82 deletions(-)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index c6ff686c9cde..efc65dc41c0d 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1013,77 +1013,6 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
 				  LANDLOCK_ACCESS_FS_REMOVE_FILE;
 }
 
-/**
- * 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.
- * @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.
- *
- * Because of disconnected directories, this walk may not reach @mnt_dir.  In
- * this case, the walk will continue to @mnt_dir 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.
- *
- * Returns:
- * - true if all the domain access rights are allowed for @dir;
- * - false if the walk reached @mnt_root.
- */
-static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
-				    const struct dentry *const mnt_root,
-				    struct dentry *dir,
-				    struct layer_access_masks *layer_masks_dom)
-{
-	bool ret = false;
-
-	if (WARN_ON_ONCE(!domain || !mnt_root || !dir || !layer_masks_dom))
-		return true;
-	if (is_nouser_or_private(dir))
-		return true;
-
-	if (!landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
-				       layer_masks_dom, LANDLOCK_KEY_INODE))
-		return true;
-
-	dget(dir);
-	while (true) {
-		struct dentry *parent_dentry;
-
-		/* Gets all layers allowing all domain accesses. */
-		if (landlock_unmask_layers(find_rule(domain, dir),
-					   layer_masks_dom)) {
-			/*
-			 * Stops when all handled accesses are allowed by at
-			 * least one rule in each layer.
-			 */
-			ret = true;
-			break;
-		}
-
-		/*
-		 * Stops at the mount point or the filesystem root for a disconnected
-		 * directory.
-		 */
-		if (dir == mnt_root || unlikely(IS_ROOT(dir)))
-			break;
-
-		parent_dentry = dget_parent(dir);
-		dput(dir);
-		dir = parent_dentry;
-	}
-	dput(dir);
-	return ret;
-}
-
 /**
  * current_check_refer_path - Check if a rename or link action is allowed
  *
@@ -1147,7 +1076,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 = {};
@@ -1202,20 +1131,55 @@ 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
+	 * OPEN_TREE_CLONE).  We do not need to 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 = unlikely(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);
-	allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
-						new_dir->dentry,
-						&layer_masks_parent2);
+	allow_parent1 = false;
+	allow_parent2 = false;
+	for (size_t i = 0; i < 2; i++) {
+		const struct path *const parent_path = i ? new_dir :
+							   &old_parent_path;
+		struct layer_access_masks *const layer_masks =
+			i ? &layer_masks_parent2 : &layer_masks_parent1;
+		bool *const allow_parent = i ? &allow_parent2 : &allow_parent1;
+
+		if (is_nouser_or_private(parent_path->dentry) ||
+		    !landlock_init_layer_masks(
+			    subject->domain, LANDLOCK_MASK_ACCESS_FS,
+			    layer_masks, LANDLOCK_KEY_INODE)) {
+			*allow_parent = true;
+			continue;
+		}
 
+		{
+			struct path walker = *parent_path;
+
+			path_get(&walker);
+			do {
+				/* Gets all layers allowing all domain accesses. */
+				if (landlock_unmask_layers(
+					    find_rule(subject->domain,
+						      walker.dentry),
+					    layer_masks)) {
+					/*
+					 * Stops when all handled accesses are
+					 * allowed by at least one rule in each
+					 * layer.
+					 */
+					*allow_parent = true;
+					break;
+				}
+			} while (landlock_walk_path_up(&walker) ==
+				 LANDLOCK_WALK_CONTINUE);
+			path_put(&walker);
+		}
+	}
 	if (allow_parent1 && allow_parent2)
 		return 0;
 
@@ -1233,7 +1197,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;
 		landlock_log_denial(subject, &request1);
 	}
 	if (request2.access) {
-- 
2.51.0