[PATCH v1] landlock: Clean up ruleset validation checks

[PATCH v1] landlock: Clean up ruleset validation checks

[Mickaël Salaün]

landlock_merge_ruleset() checks for a NULL ruleset after dereferencing
it in lockdep_assert_held().  Move the assertion after the check so the
defensive path remains effective.

The mask-validation comment originated in landlock_add_fs_access_mask()
to explain that its WARN_ON_ONCE() checked a caller invariant.  It
became self-referential when this helper and its network and scope
counterparts were inlined into landlock_create_ruleset().  Restate the
invariant without naming the caller.

Keep both as defensive callee checks.  Moving the assertion preserves
the NULL check's ability to warn and return -EINVAL, while invalid masks
remain warned about and masked.

Reported-by: Günther Noack <gnoack@google.com>
Closes: https://patch.msgid.link/aobYhIt3vcs2xN0b@google.com
Closes: https://patch.msgid.link/aobasxUDQ8b7GYXl@google.com
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 security/landlock/domain.c  | 3 ++-
 security/landlock/ruleset.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index 93c7104fd6b2..4031b581be07 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -439,10 +439,11 @@ landlock_merge_ruleset(struct landlock_domain *const parent,
 	int err;
 
 	might_sleep();
-	lockdep_assert_held(&ruleset->lock);
 	if (WARN_ON_ONCE(!ruleset))
 		return ERR_PTR(-EINVAL);
 
+	lockdep_assert_held(&ruleset->lock);
+
 	if (parent) {
 		if (parent->num_layers >= LANDLOCK_MAX_NUM_LAYERS)
 			return ERR_PTR(-E2BIG);
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 0d07707523cd..a5d135d085cb 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -58,7 +58,7 @@ landlock_create_ruleset(const access_mask_t fs_access_mask,
 	new_ruleset->id = landlock_get_id_range(1);
 #endif /* CONFIG_TRACEPOINTS */
 
-	/* Should already be checked in landlock_create_ruleset(). */
+	/* The caller must only pass supported access rights and scopes. */
 	if (fs_access_mask) {
 		const access_mask_t mask = fs_access_mask &
 					   LANDLOCK_MASK_ACCESS_FS;
-- 
2.55.0

Re: [PATCH v1] landlock: Clean up ruleset validation checks

[Günther Noack]

On Mon, Sep 07, 2026 at 12:36:08PM +0200, Mickaël Salaün wrote:
> landlock_merge_ruleset() checks for a NULL ruleset after dereferencing
> it in lockdep_assert_held().  Move the assertion after the check so the
> defensive path remains effective.
> 
> The mask-validation comment originated in landlock_add_fs_access_mask()
> to explain that its WARN_ON_ONCE() checked a caller invariant.  It
> became self-referential when this helper and its network and scope
> counterparts were inlined into landlock_create_ruleset().  Restate the
> invariant without naming the caller.
> 
> Keep both as defensive callee checks.  Moving the assertion preserves
> the NULL check's ability to warn and return -EINVAL, while invalid masks
> remain warned about and masked.
> 
> Reported-by: Günther Noack <gnoack@google.com>
> Closes: https://patch.msgid.link/aobYhIt3vcs2xN0b@google.com
> Closes: https://patch.msgid.link/aobasxUDQ8b7GYXl@google.com
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  security/landlock/domain.c  | 3 ++-
>  security/landlock/ruleset.c | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index 93c7104fd6b2..4031b581be07 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -439,10 +439,11 @@ landlock_merge_ruleset(struct landlock_domain *const parent,
>  	int err;
>  
>  	might_sleep();
> -	lockdep_assert_held(&ruleset->lock);
>  	if (WARN_ON_ONCE(!ruleset))
>  		return ERR_PTR(-EINVAL);
>  
> +	lockdep_assert_held(&ruleset->lock);
> +
>  	if (parent) {
>  		if (parent->num_layers >= LANDLOCK_MAX_NUM_LAYERS)
>  			return ERR_PTR(-E2BIG);
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index 0d07707523cd..a5d135d085cb 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -58,7 +58,7 @@ landlock_create_ruleset(const access_mask_t fs_access_mask,
>  	new_ruleset->id = landlock_get_id_range(1);
>  #endif /* CONFIG_TRACEPOINTS */
>  
> -	/* Should already be checked in landlock_create_ruleset(). */
> +	/* The caller must only pass supported access rights and scopes. */
>  	if (fs_access_mask) {
>  		const access_mask_t mask = fs_access_mask &
>  					   LANDLOCK_MASK_ACCESS_FS;
> -- 
> 2.55.0
> 

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

Thanks!
—Günther