Re: [PATCH 6.12.y 2/9] landlock: Fix handling of disconnected directories

["Mickaël Salaün" <mic@digikod.net>]

Thanks Harshit.  BTW, the following commit should also be backported
(and it was specifically created to ease backports): 6803b6ebb816
("landlock: Fix cosmetic change").

The current patch should be backported down to 5.15, but it needs to be
adapted.  Harshit, I can work on it, please let me know.

I'm wondering why I didn't get notified that some Fixes patch couldn't
automatically be backported.  Greg, is there some way to register for
this kind of issue?  What are the rules to not automatically backport
patches?

I also noticed that other Fixes commits were not backported to stable
branches whereas they can be cleanly cherry-picked.  I'm also wondering
why they weren't pick.

FYI, here are the ones that can be backported without needing changes:
- 602acfb54119 ("landlock: Optimize stack usage when !CONFIG_AUDIT")
- 60207df2ebf3 ("landlock: Remove useless include")
- 7aa593d8fb64 ("selftests/landlock: Fix missing semicolon")

They should all be backported, even if they look like cosmetic fixes
(because they might be needed for other fixes/backports).

Here is the other one that needs to be adapted:
- e4d82cbce225 ("landlock: Fix TCP handling of short AF_UNSPEC addresses")

Thanks,
 Mickaël

On Tue, Mar 24, 2026 at 07:04:49AM -0700, Harshit Mogalapalli wrote:
> From: Mickaël Salaün <mic@digikod.net>
> 
> [ Upstream commit 49c9e09d961025b22e61ef9ad56aa1c21b6ce2f1 ]
> 
> Disconnected files or directories can appear when they are visible and
> opened from a bind mount, but have been renamed or moved from the source
> of the bind mount in a way that makes them inaccessible from the mount
> point (i.e. out of scope).
> 
> Previously, access rights tied to files or directories opened through a
> disconnected directory were collected by walking the related hierarchy
> down to the root of the filesystem, without taking into account the
> mount point because it couldn't be found. This could lead to
> inconsistent access results, potential access right widening, and
> hard-to-debug renames, especially since such paths cannot be printed.
> 
> For a sandboxed task to create a disconnected directory, it needs to
> have write access (i.e. FS_MAKE_REG, FS_REMOVE_FILE, and FS_REFER) to
> the underlying source of the bind mount, and read access to the related
> mount point.   Because a sandboxed task cannot acquire more access
> rights than those defined by its Landlock domain, this could lead to
> inconsistent access rights due to missing permissions that should be
> inherited from the mount point hierarchy, while inheriting permissions
> from the filesystem hierarchy hidden by this mount point instead.
> 
> Landlock now handles files and directories opened from disconnected
> directories by taking into account the filesystem hierarchy when the
> mount point is not found in the hierarchy walk, and also always taking
> into account the mount point from which these disconnected directories
> were opened.  This ensures that a rename is not allowed if it would
> widen access rights [1].
> 
> The rationale is that, even if disconnected hierarchies might not be
> visible or accessible to a sandboxed task, relying on the collected
> access rights from them improves the guarantee that access rights will
> not be widened during a rename because of the access right comparison
> between the source and the destination (see LANDLOCK_ACCESS_FS_REFER).
> It may look like this would grant more access on disconnected files and
> directories, but the security policies are always enforced for all the
> evaluated hierarchies.  This new behavior should be less surprising to
> users and safer from an access control perspective.
> 
> Remove a wrong WARN_ON_ONCE() canary in collect_domain_accesses() and
> fix the related comment.
> 
> Because opened files have their access rights stored in the related file
> security properties, there is no impact for disconnected or unlinked
> files.
> 
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Günther Noack <gnoack@google.com>
> Cc: Song Liu <song@kernel.org>
> Reported-by: Tingmao Wang <m@maowtm.org>
> Closes: https://lore.kernel.org/r/027d5190-b37a-40a8-84e9-4ccbc352bcdf@maowtm.org
> Closes: https://lore.kernel.org/r/09b24128f86973a6022e6aa8338945fcfb9a33e4.1749925391.git.m@maowtm.org
> Fixes: b91c3e4ea756 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER")
> Fixes: cb2c7d1a1776 ("landlock: Support filesystem access-control")
> Link: https://lore.kernel.org/r/b0f46246-f2c5-42ca-93ce-0d629702a987@maowtm.org [1]
> Reviewed-by: Tingmao Wang <m@maowtm.org>
> Link: https://lore.kernel.org/r/20251128172200.760753-2-mic@digikod.net
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> (cherry picked from commit 49c9e09d961025b22e61ef9ad56aa1c21b6ce2f1)
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
>  security/landlock/errata/abi-1.h | 16 +++++++++++++
>  security/landlock/fs.c           | 40 ++++++++++++++++++++++----------
>  2 files changed, 44 insertions(+), 12 deletions(-)
>  create mode 100644 security/landlock/errata/abi-1.h
> 
> diff --git a/security/landlock/errata/abi-1.h b/security/landlock/errata/abi-1.h
> new file mode 100644
> index 000000000000..e8a2bff2e5b6
> --- /dev/null
> +++ b/security/landlock/errata/abi-1.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +/**
> + * DOC: erratum_3
> + *
> + * Erratum 3: Disconnected directory handling
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * This fix addresses an issue with disconnected directories that occur when a
> + * directory is moved outside the scope of a bind mount.  The change ensures
> + * that evaluated access rights include both those from the disconnected file
> + * hierarchy down to its filesystem root and those from the related mount point
> + * hierarchy.  This prevents access right widening through rename or link
> + * actions.
> + */
> +LANDLOCK_ERRATUM(3)
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index f0e94cb74fca..a26199568db2 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -899,21 +899,31 @@ static bool is_access_to_paths_allowed(
>  				break;
>  			}
>  		}
> +
>  		if (unlikely(IS_ROOT(walker_path.dentry))) {
> -			/*
> -			 * Stops at disconnected root directories.  Only allows
> -			 * access to internal filesystems (e.g. nsfs, which is
> -			 * reachable through /proc/<pid>/ns/<namespace>).
> -			 */
> -			if (walker_path.mnt->mnt_flags & MNT_INTERNAL) {
> +			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;
>  			}
> -			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 {
> +			parent_dentry = dget_parent(walker_path.dentry);
> +			dput(walker_path.dentry);
> +			walker_path.dentry = parent_dentry;
>  		}
> -		parent_dentry = dget_parent(walker_path.dentry);
> -		dput(walker_path.dentry);
> -		walker_path.dentry = parent_dentry;
>  	}
>  	path_put(&walker_path);
>  
> @@ -990,6 +1000,9 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
>   * 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.
> @@ -1031,8 +1044,11 @@ static bool collect_domain_accesses(
>  			break;
>  		}
>  
> -		/* We should not reach a root other than @mnt_root. */
> -		if (dir == mnt_root || WARN_ON_ONCE(IS_ROOT(dir)))
> +		/*
> +		 * 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);
> -- 
> 2.50.1
>