[PATCH 1/3] landlock: Fix layer composition of LANDLOCK_ACCESS_FS_REFER

[PATCH 1/3] landlock: Fix layer composition of LANDLOCK_ACCESS_FS_REFER

[Günther Noack]

In landlock_get_fs_access_mask(), only add the
LANDLOCK_ACCESS_FS_REFER right if any filesystem access rights are
handled at all.  This prevents us from accidentally treating
non-filesystem layers as if they handled LANDLOCK_ACCESS_FS_REFER.

landlock_get_fs_access_mask() is used when initializing the layer
masks for doing access right checks, so this bug meant that we
unintentionally denied "refer" (e.g., link, rename) operations in
layered cases, where the same operation would have been accepted if
only either one of the two layers had been enforced.  If the rulesets
individually allow an operation, the composition of these rulesets
should also allow it.

The bug only manifests as soon as one other layer handles filesystem
access rights.  Otherwise, Landlock filesystem path walk is guarded by
an early exit for the case that no filesystem-enforcing layer exists.

Reported-by: Dwi Siswanto <me@dw1.io>
Link: https://github.com/landlock-lsm/linux/issues/67
Link: https://github.com/landlock-lsm/go-landlock/issues/66
Fixes: fff69fb03dde ("landlock: Support network rules with TCP bind and connect")
Cc: stable@vger.kernel.org
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
 security/landlock/domain.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index 428c1d9890db..693e569f5387 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -258,9 +258,16 @@ static inline access_mask_t
 landlock_get_fs_access_mask(const struct landlock_domain *const domain,
 			    const u16 layer_level)
 {
-	/* Handles all initially denied by default access rights. */
-	return domain->handled_masks[layer_level].fs |
-	       _LANDLOCK_ACCESS_FS_INITIALLY_DENIED;
+	access_mask_t handled = domain->handled_masks[layer_level].fs;
+
+	/*
+	 * Handles all initially denied by default access rights, but only for
+	 * layers handling filesystem accesses.
+	 */
+	if (handled)
+		handled |= _LANDLOCK_ACCESS_FS_INITIALLY_DENIED;
+
+	return handled;
 }
 
 static inline access_mask_t
-- 
2.56.0

[PATCH 2/3] selftests/landlock: Test that layering does not break refer

[Günther Noack]

Regression test to exercise that layered Landlock domains do not
accidentally deny the LANDLOCK_ACCESS_FS_REFER access right.  This
used to be the case when layering a non-filesystem ruleset with a
filesystem-enforcing ruleset.  In that case, REFER operations that
would have been allowed for the individual rulesets would get denied
when layering the same rulesets.

Link: https://github.com/landlock-lsm/linux/issues/67
Link: https://github.com/landlock-lsm/go-landlock/issues/66
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
 tools/testing/selftests/landlock/fs_test.c | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 6e979cef884d..c319bcea1437 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -2619,6 +2619,45 @@ TEST_F_FORK(layout1, refer_denied_by_default4)
 				layer_dir_s1d1_refer);
 }
 
+/*
+ * Regression test for layered domains: The two enforced Landlock layers both do
+ * not restrict the tested "refer" operations on their own, so they should also
+ * not restrict them when they are layered.
+ */
+static void test_layering_does_not_break_refer(
+	struct __test_metadata *const _metadata,
+	const struct landlock_ruleset_attr *const non_fs_attr)
+{
+	ASSERT_EQ(0, unlink(file1_s1d2));
+
+	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_REFER, layer_dir_s1d1_refer);
+
+	drop_access_rights(_metadata, non_fs_attr);
+
+	EXPECT_EQ(0, test_rename(file1_s1d1, file1_s1d2));
+	EXPECT_EQ(0, test_rename(file1_s1d2, file1_s1d1));
+	EXPECT_EQ(0, test_exchange(file2_s1d1, file2_s1d2));
+	EXPECT_EQ(0, test_exchange(file2_s1d2, file2_s1d1));
+}
+
+TEST_F_FORK(layout1, layering_with_net_does_not_break_refer)
+{
+	const struct landlock_ruleset_attr attr = {
+		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP,
+	};
+
+	test_layering_does_not_break_refer(_metadata, &attr);
+}
+
+TEST_F_FORK(layout1, layering_with_scoped_does_not_break_refer)
+{
+	const struct landlock_ruleset_attr attr = {
+		.scoped = LANDLOCK_SCOPE_SIGNAL,
+	};
+
+	test_layering_does_not_break_refer(_metadata, &attr);
+}
+
 /*
  * Tests walking through a denied root mount.
  */
-- 
2.56.0

[PATCH 3/3] landlock: Simplify landlock_get_fs_access_mask()

[Günther Noack]

Return the handled FS access mask as it is, instead of artificially
adding LANDLOCK_ACCESS_FS_REFER to it.

Since commit d6c7cf84a24f ("landlock: Simplify initially denied access
rights"), the merge operation already bakes the initially denied
access rights (LANDLOCK_ACCESS_FS_REFER) into the a layer's handled FS
access rights. -- They don't need to be added explicitly any more.

No functional change.

Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
 security/landlock/domain.h | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index 693e569f5387..1aa85c817d0a 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -258,16 +258,12 @@ static inline access_mask_t
 landlock_get_fs_access_mask(const struct landlock_domain *const domain,
 			    const u16 layer_level)
 {
-	access_mask_t handled = domain->handled_masks[layer_level].fs;
-
 	/*
-	 * Handles all initially denied by default access rights, but only for
-	 * layers handling filesystem accesses.
+	 * Initially denied by default access rights are already included for
+	 * layers handling filesystem accesses, see
+	 * landlock_upgrade_handled_access_masks().
 	 */
-	if (handled)
-		handled |= _LANDLOCK_ACCESS_FS_INITIALLY_DENIED;
-
-	return handled;
+	return domain->handled_masks[layer_level].fs;
 }
 
 static inline access_mask_t
-- 
2.56.0

Re: [PATCH 1/3] landlock: Fix layer composition of LANDLOCK_ACCESS_FS_REFER

[Dwi Siswanto]

I saw the docs still describe the old one. include/uapi/linux/landlock.h
says:

   * For historical reasons, the %LANDLOCK_ACCESS_FS_REFER right is always denied
   * by default, even when its bit is not set in @handled_access_fs.

and, for LANDLOCK_ACCESS_FS_REFER itself:

   *   This is the only access right which is denied by default by any ruleset,
   *   even if the right is not specified as handled at ruleset creation time.

So ruleset that handles no filesystem access right never denies
LANDLOCK_ACCESS_FS_REFER -- alone or layered.

Also, the comment above _LANDLOCK_ACCESS_FS_INITIALLY_DENIED in access.h
has the same problem:

   * All access rights that are denied by default whether they are handled or not
   * by a ruleset/layer.  This must be ORed with all domain->handled_masks[]
   * entries when we need to get the absolute handled access masks, see
   * landlock_upgrade_handled_access_masks().

After this patch, the mask only applies to layers that handle filesystem
access, and after patch 3, nothing ORs it in at read time. *Someone* who
follows the comment could bring the bug back.

> +	access_mask_t handled = domain->handled_masks[layer_level].fs;
> +
> +	/*
> +	 * Handles all initially denied by default access rights, but only for
> +	 * layers handling filesystem accesses.
> +	 */
> +	if (handled)
> +		handled |= _LANDLOCK_ACCESS_FS_INITIALLY_DENIED;
> +
> +	return handled;

nit: I believe landlock_upgrade_handled_access_masks() already has the
same "only if fs is non-zero" rule, so this could be

	return landlock_upgrade_handled_access_masks(
		domain->handled_masks[layer_level]).fs;

Which keep the rule in one place. It matters little if patch 3 goes in
on top, and 6.12 doesn't have the helper. What do you think?

Best!
Dwi

Re: [PATCH 1/3] landlock: Fix layer composition of LANDLOCK_ACCESS_FS_REFER

[Günther Noack]

On Sun, Nov 01, 2026 at 07:47:50PM +0700, Dwi Siswanto wrote:
> I saw the docs still describe the old one. include/uapi/linux/landlock.h
> says:
> 
>    * For historical reasons, the %LANDLOCK_ACCESS_FS_REFER right is always denied
>    * by default, even when its bit is not set in @handled_access_fs.
> 
> and, for LANDLOCK_ACCESS_FS_REFER itself:
> 
>    *   This is the only access right which is denied by default by any ruleset,
>    *   even if the right is not specified as handled at ruleset creation time.
> 
> So ruleset that handles no filesystem access right never denies
> LANDLOCK_ACCESS_FS_REFER -- alone or layered.
> 
> Also, the comment above _LANDLOCK_ACCESS_FS_INITIALLY_DENIED in access.h
> has the same problem:
> 
>    * All access rights that are denied by default whether they are handled or not
>    * by a ruleset/layer.  This must be ORed with all domain->handled_masks[]
>    * entries when we need to get the absolute handled access masks, see
>    * landlock_upgrade_handled_access_masks().
> 
> After this patch, the mask only applies to layers that handle filesystem
> access, and after patch 3, nothing ORs it in at read time. *Someone* who
> follows the comment could bring the bug back.

Thanks for pointing this out, the documentation is indeed not up to date.

All this documentation likely stems from a time when *only* access_fs
but none of the other fields existed in struct landlock_ruleset_attr.

It is not very obvious from looking at this patch set alone, but the
intended semantics are clearer when you look at merge_ruleset() in
domain.c.

This in turn calls landlock_upgrade_handled_access_masks() in
access.h to calculcate the updated access masks with the same logic:

	if (access_masks.fs)
  		access_masks.fs |= _LANDLOCK_ACCESS_FS_INITIALLY_DENIED;

	return access_masks;

The behaviour that we have for non-layered domains is the intended one
here -- we only add the REFER right when either one of the other
filesystem access rights is handled, but we do not set it when only
other access right types are handled.

I'll send a documentation update for this, either in the next patch
set revision, or in a follow-up if needed.


> > +	access_mask_t handled = domain->handled_masks[layer_level].fs;
> > +
> > +	/*
> > +	 * Handles all initially denied by default access rights, but only for
> > +	 * layers handling filesystem accesses.
> > +	 */
> > +	if (handled)
> > +		handled |= _LANDLOCK_ACCESS_FS_INITIALLY_DENIED;
> > +
> > +	return handled;
> 
> nit: I believe landlock_upgrade_handled_access_masks() already has the
> same "only if fs is non-zero" rule, so this could be
> 
> 	return landlock_upgrade_handled_access_masks(
> 		domain->handled_masks[layer_level]).fs;
> 
> Which keep the rule in one place. It matters little if patch 3 goes in
> on top, and 6.12 doesn't have the helper. What do you think?

Good point, I agree, this function already implements this.

As you are saying, the first commit is mainly useful to backporting,
and the backport to 6.12 will be easier to compare with the original
for stable maintainers if we spell it out instead of using the helper
function.

Not feeling strongly about it though; I'd let Mickaël decide what he
prefers.  In either case, the backporting will require some manual
work because the affected code moved around between files.

–Günther