[PATCH v1] landlock: Remove warning in collect_domain_accesses()

[PATCH v1] landlock: Remove warning in collect_domain_accesses()

[Mickaël Salaün]

As in is_access_to_paths_allowed(), it is also possible to reach
disconnected root directories in collect_domain_accesses().

Remove a wrong WARN_ON_ONCE() canary in collect_domain_accesses() and
fix comment.  Using an unlikely() annotation doesn't seem appropriate
here.  A following patch from Tingmao tests this case [1].

Cc: Günther Noack <gnoack@google.com>
Reported-by: Tingmao Wang <m@maowtm.org>
Link: https://lore.kernel.org/r/09b24128f86973a6022e6aa8338945fcfb9a33e4.1749925391.git.m@maowtm.org [1]
Fixes: b91c3e4ea756 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 security/landlock/fs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 6fee7c20f64d..1d6c4e728f92 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1061,8 +1061,8 @@ 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 disconnected root directories. */
+		if (dir == mnt_root || IS_ROOT(dir))
 			break;
 
 		parent_dentry = dget_parent(dir);
-- 
2.49.0

Re: [PATCH v1] landlock: Remove warning in collect_domain_accesses()

[Günther Noack]

On Wed, Jun 18, 2025 at 03:47:31PM +0200, Mickaël Salaün wrote:
> As in is_access_to_paths_allowed(), it is also possible to reach
> disconnected root directories in collect_domain_accesses().
> 
> Remove a wrong WARN_ON_ONCE() canary in collect_domain_accesses() and
> fix comment.  Using an unlikely() annotation doesn't seem appropriate
> here.  A following patch from Tingmao tests this case [1].
> 
> Cc: Günther Noack <gnoack@google.com>
> Reported-by: Tingmao Wang <m@maowtm.org>
> Link: https://lore.kernel.org/r/09b24128f86973a6022e6aa8338945fcfb9a33e4.1749925391.git.m@maowtm.org [1]
> Fixes: b91c3e4ea756 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  security/landlock/fs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 6fee7c20f64d..1d6c4e728f92 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1061,8 +1061,8 @@ 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 disconnected root directories. */
> +		if (dir == mnt_root || IS_ROOT(dir))
>  			break;
>  
>  		parent_dentry = dget_parent(dir);
> -- 
> 2.49.0
> 

Reviewed-by: Günther Noack <gnoack@google.com>

Re: [PATCH v1] landlock: Remove warning in collect_domain_accesses()

[Mickaël Salaün]

On Wed, Jun 18, 2025 at 03:47:31PM +0200, Mickaël Salaün wrote:
> As in is_access_to_paths_allowed(), it is also possible to reach
> disconnected root directories in collect_domain_accesses().
> 
> Remove a wrong WARN_ON_ONCE() canary in collect_domain_accesses() and
> fix comment.  Using an unlikely() annotation doesn't seem appropriate
> here.  A following patch from Tingmao tests this case [1].
> 
> Cc: Günther Noack <gnoack@google.com>
> Reported-by: Tingmao Wang <m@maowtm.org>
> Link: https://lore.kernel.org/r/09b24128f86973a6022e6aa8338945fcfb9a33e4.1749925391.git.m@maowtm.org [1]
> Fixes: b91c3e4ea756 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>

This patch hides an edge cases that I though wasn't supposed to happen.
I'll drop it in favor of this other patch which fixes the underlying
issue:
https://lore.kernel.org/all/20250701183812.3201231-1-mic@digikod.net/

> ---
>  security/landlock/fs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 6fee7c20f64d..1d6c4e728f92 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1061,8 +1061,8 @@ 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 disconnected root directories. */
> +		if (dir == mnt_root || IS_ROOT(dir))
>  			break;
>  
>  		parent_dentry = dget_parent(dir);
> -- 
> 2.49.0
> 
>