[PATCH v2 0/3] landlock: Refactor layer masks

[PATCH v2 0/3] landlock: Refactor layer masks

[Günther Noack]

Hello!

This patch set "transposes" the layer masks matrix, which was
previously modeled as a access-max-sized array of layer masks, and
changes it to be a layer-max-sized array of access masks instead.
(It is a pure refactoring, there are no user-visible changes.)

This unlocks a few code simplifications and in multiple places it
removes the need for loops and branches that deal with individual
bits.  Instead, the changed data structure now lends itself for more
bitwise operations.  The underlying hypothesis for me was that by
using more bitwise operations and fewer branches, we would get an
overall speedup even when the data structure size increases slightly
in some cases.

Tentative results with and without this patch set show that the
hypothesis likely holds true.  The benchmark I used exercises a "worst
case" scenario that attempts to be bottlenecked on the affected code:
constructs a large number of nested directories, with one "path
beneath" rule each and then tries to open the innermost directory many
times.  The benchmark is intentionally unrealistic to amplify the
amount of time used for the path walk logic and forces Landlock to
walk the full path (eventually failing the open syscall).  (I'll send
the benchmark program in a reply to this mail for full transparency.)

Measured with the benchmark program, the patch set results in a
speedup of about -10%.  The benchmark results are only tentative and
have been produced in Qemu:

With the patch, the benchmark runs in 6046 clocks (measured with
times(3)):

*** Benchmark ***
10000 dirs, 100000 iterations, with landlock
*** Benchmark concluded ***
System: 6046 clocks
User  : 1 clocks
Clocks per second: 1000000

Without the patch, we get 6713 clocks, which is 11% more

*** Benchmark ***
10000 dirs, 100000 iterations, with landlock
*** Benchmark concluded ***
System: 6713 clocks
User  : 0 clocks
Clocks per second: 1000000

The base revision used for benchmarking was commit 7a51784da76d
("tools/sched_ext: update scx_show_state.py for scx_aborting change")

In real-life scenarios, the speed improvement from this patch set will
be less pronounced than in the artificial benchmark, as people do not
usually stack directories that deeply and attach so many rules to
them, and the EACCES error should also be the exception rather than
the norm.

I am looking forward to your feedback.

P.S.: I am open to suggestions on what the "layer masks" variables
should be called, because the name "layer masks" might be less
appropriate after this change.  I have not fixed up the name
everywhere because fixing up the code took priority for now.

---
Changes since previous versions:

V2: (This patch set)

* Remove the refactoring around the deny_mask_t type,
  it is better to send that as a separate patch (mic review)
* Added the benchmark program to the selftests
* Fix unused variable report for "access_dom":
  https://lore.kernel.org/all/202601200900.wonk9M0m-lkp@intel.com/
* Use size_t and ARRAY_SIZE to loop over the layers (mic review)
* Documentation
  * Fixing up and adding back documentaiton (mic review)
  * Documented landlock_unmask_layers()
  * Fixed up kernel docs in a place where it was improperly updated
    (Spotted by Randy Dunlap
    https://lore.kernel.org/all/20260123025121.3713403-1-rdunlap@infradead.org/)
* Minor
  * Const, some newlines (mic review)

V1: (Initial version)

https://lore.kernel.org/all/20251230103917.10549-3-gnoack3000@gmail.com/

Günther Noack (3):
  selftests/landlock: Add filesystem access benchmark
  landlock: access_mask_subset() helper
  landlock: transpose the layer masks data structure

 security/landlock/access.h                  |  16 +-
 security/landlock/audit.c                   |  84 ++---
 security/landlock/audit.h                   |   3 +-
 security/landlock/domain.c                  |  45 +--
 security/landlock/domain.h                  |   4 +-
 security/landlock/fs.c                      | 354 +++++++++-----------
 security/landlock/net.c                     |  11 +-
 security/landlock/ruleset.c                 |  88 ++---
 security/landlock/ruleset.h                 |  21 +-
 tools/testing/selftests/landlock/.gitignore |   1 +
 tools/testing/selftests/landlock/Makefile   |   1 +
 tools/testing/selftests/landlock/fs_bench.c | 161 +++++++++
 12 files changed, 444 insertions(+), 345 deletions(-)
 create mode 100644 tools/testing/selftests/landlock/fs_bench.c

-- 
2.52.0

[PATCH v2 1/3] selftests/landlock: Add filesystem access benchmark

[Günther Noack]

fs_bench benchmarks the performance of Landlock's path walk
by exercising it in a scenario that amplifies Landlock's overhead:

* Create a large number of nested directories
* Enforce a Landlock policy in which a rule is associated with each of
  these subdirectories
* Benchmark openat() applied to the deepest directory,
  forcing Landlock to walk the entire path.

Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
 tools/testing/selftests/landlock/.gitignore |   1 +
 tools/testing/selftests/landlock/Makefile   |   1 +
 tools/testing/selftests/landlock/fs_bench.c | 161 ++++++++++++++++++++
 3 files changed, 163 insertions(+)
 create mode 100644 tools/testing/selftests/landlock/fs_bench.c

diff --git a/tools/testing/selftests/landlock/.gitignore b/tools/testing/selftests/landlock/.gitignore
index a820329cae0d..1974e17a2611 100644
--- a/tools/testing/selftests/landlock/.gitignore
+++ b/tools/testing/selftests/landlock/.gitignore
@@ -1,4 +1,5 @@
 /*_test
+/fs_bench
 /sandbox-and-launch
 /true
 /wait-pipe
diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
index 044b83bde16e..fc43225d319a 100644
--- a/tools/testing/selftests/landlock/Makefile
+++ b/tools/testing/selftests/landlock/Makefile
@@ -9,6 +9,7 @@ LOCAL_HDRS += $(wildcard *.h)
 src_test := $(wildcard *_test.c)
 
 TEST_GEN_PROGS := $(src_test:.c=)
+TEST_GEN_PROGS += fs_bench
 
 TEST_GEN_PROGS_EXTENDED := \
 	true \
diff --git a/tools/testing/selftests/landlock/fs_bench.c b/tools/testing/selftests/landlock/fs_bench.c
new file mode 100644
index 000000000000..a3b686418bc5
--- /dev/null
+++ b/tools/testing/selftests/landlock/fs_bench.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Landlock filesystem benchmark
+ */
+
+#define _GNU_SOURCE
+#include <err.h>
+#include <fcntl.h>
+#include <linux/landlock.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/prctl.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/times.h>
+#include <time.h>
+#include <unistd.h>
+
+void usage(const char *argv0)
+{
+	printf("Usage:\n");
+	printf("  %s [OPTIONS]\n", argv0);
+	printf("\n");
+	printf("  Benchmark expensive Landlock checks for D nested dirs\n");
+	printf("\n");
+	printf("Options:\n");
+	printf("  -h	help\n");
+	printf("  -L	disable Landlock (as a baseline)\n");
+	printf("  -d D	set directory depth to D\n");
+	printf("  -n N	set number of benchmark iterations to N\n");
+}
+
+/*
+ * Build a deep directory, enforce Landlock and return the FD to the
+ * deepest dir.  On any failure, exit the process with an error.
+ */
+int build_directory(size_t depth, bool use_landlock)
+{
+	const char *path = "d"; /* directory name */
+	int abi, ruleset_fd, current, previous;
+
+	if (use_landlock) {
+		abi = syscall(SYS_landlock_create_ruleset, NULL, 0,
+			      LANDLOCK_CREATE_RULESET_VERSION);
+		if (abi < 7)
+			err(1, "Landlock ABI too low: got %d, wanted 7+", abi);
+	}
+
+	ruleset_fd = -1;
+	if (use_landlock) {
+		struct landlock_ruleset_attr attr = {
+			.handled_access_fs =
+				0xffff, /* All FS access rights as of 2026-01 */
+		};
+		ruleset_fd = syscall(SYS_landlock_create_ruleset, &attr,
+				     sizeof(attr), 0U);
+		if (ruleset_fd < 0)
+			err(1, "landlock_create_ruleset");
+	}
+
+	current = open(".", O_PATH);
+	if (current < 0)
+		err(1, "open(.)");
+
+	while (depth--) {
+		if (use_landlock) {
+			struct landlock_path_beneath_attr attr = {
+				.allowed_access = LANDLOCK_ACCESS_FS_IOCTL_DEV,
+				.parent_fd = current,
+			};
+			if (syscall(SYS_landlock_add_rule, ruleset_fd,
+				    LANDLOCK_RULE_PATH_BENEATH, &attr, 0) < 0)
+				err(1, "landlock_add_rule");
+		}
+
+		if (mkdirat(current, path, 0700) < 0)
+			err(1, "mkdirat(%s)", path);
+
+		previous = current;
+		current = openat(current, path, O_PATH);
+		if (current < 0)
+			err(1, "open(%s)", path);
+
+		close(previous);
+	}
+
+	if (use_landlock) {
+		if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0)
+			err(1, "prctl");
+
+		if (syscall(SYS_landlock_restrict_self, ruleset_fd, 0) < 0)
+			err(1, "landlock_restrict_self");
+	}
+
+	close(ruleset_fd);
+	return current;
+}
+
+int main(int argc, char *argv[])
+{
+	bool use_landlock = true;
+	size_t num_iterations = 100000;
+	size_t num_subdirs = 10000;
+	int c, current, fd;
+	struct tms start_time, end_time;
+
+	setbuf(stdout, NULL);
+	while ((c = getopt(argc, argv, "hLd:n:")) != -1) {
+		switch (c) {
+		case 'h':
+			usage(argv[0]);
+			return EXIT_SUCCESS;
+		case 'L':
+			use_landlock = false;
+			break;
+		case 'd':
+			num_subdirs = atoi(optarg);
+			break;
+		case 'n':
+			num_iterations = atoi(optarg);
+			break;
+		default:
+			usage(argv[0]);
+			return EXIT_FAILURE;
+		}
+	}
+
+	printf("*** Benchmark ***\n");
+	printf("%zu dirs, %zu iterations, %s landlock\n", num_subdirs,
+	       num_iterations, use_landlock ? "with" : "without");
+
+	if (times(&start_time) == -1)
+		err(1, "times");
+
+	current = build_directory(num_subdirs, use_landlock);
+
+	for (int i = 0; i < num_iterations; i++) {
+		fd = openat(current, ".", O_DIRECTORY);
+		if (fd != -1) {
+			if (use_landlock)
+				errx(1, "openat succeeded, expected error");
+
+			close(fd);
+		}
+	}
+
+	if (times(&end_time) == -1)
+		err(1, "times");
+
+	printf("*** Benchmark concluded ***\n");
+	printf("System: %ld clocks\n",
+	       end_time.tms_stime - start_time.tms_stime);
+	printf("User  : %ld clocks\n",
+	       end_time.tms_utime - start_time.tms_utime);
+	printf("Clocks per second: %ld\n", CLOCKS_PER_SEC);
+
+	close(current);
+}
-- 
2.52.0

Re: [PATCH v2 1/3] selftests/landlock: Add filesystem access benchmark

[Mickaël Salaün]

On Sun, Jan 25, 2026 at 08:58:51PM +0100, Günther Noack wrote:
> fs_bench benchmarks the performance of Landlock's path walk
> by exercising it in a scenario that amplifies Landlock's overhead:
> 
> * Create a large number of nested directories
> * Enforce a Landlock policy in which a rule is associated with each of
>   these subdirectories
> * Benchmark openat() applied to the deepest directory,
>   forcing Landlock to walk the entire path.
> 
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
>  tools/testing/selftests/landlock/.gitignore |   1 +
>  tools/testing/selftests/landlock/Makefile   |   1 +
>  tools/testing/selftests/landlock/fs_bench.c | 161 ++++++++++++++++++++
>  3 files changed, 163 insertions(+)
>  create mode 100644 tools/testing/selftests/landlock/fs_bench.c
> 
> diff --git a/tools/testing/selftests/landlock/.gitignore b/tools/testing/selftests/landlock/.gitignore
> index a820329cae0d..1974e17a2611 100644
> --- a/tools/testing/selftests/landlock/.gitignore
> +++ b/tools/testing/selftests/landlock/.gitignore
> @@ -1,4 +1,5 @@
>  /*_test
> +/fs_bench
>  /sandbox-and-launch
>  /true
>  /wait-pipe
> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
> index 044b83bde16e..fc43225d319a 100644
> --- a/tools/testing/selftests/landlock/Makefile
> +++ b/tools/testing/selftests/landlock/Makefile
> @@ -9,6 +9,7 @@ LOCAL_HDRS += $(wildcard *.h)
>  src_test := $(wildcard *_test.c)
>  
>  TEST_GEN_PROGS := $(src_test:.c=)
> +TEST_GEN_PROGS += fs_bench
>  
>  TEST_GEN_PROGS_EXTENDED := \
>  	true \
> diff --git a/tools/testing/selftests/landlock/fs_bench.c b/tools/testing/selftests/landlock/fs_bench.c
> new file mode 100644
> index 000000000000..a3b686418bc5
> --- /dev/null
> +++ b/tools/testing/selftests/landlock/fs_bench.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Landlock filesystem benchmark

You might want to add some copyright.

> + */
> +
> +#define _GNU_SOURCE
> +#include <err.h>
> +#include <fcntl.h>
> +#include <linux/landlock.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/prctl.h>
> +#include <sys/stat.h>
> +#include <sys/syscall.h>
> +#include <sys/times.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +void usage(const char *argv0)

const

> +{
> +	printf("Usage:\n");
> +	printf("  %s [OPTIONS]\n", argv0);
> +	printf("\n");
> +	printf("  Benchmark expensive Landlock checks for D nested dirs\n");
> +	printf("\n");
> +	printf("Options:\n");
> +	printf("  -h	help\n");
> +	printf("  -L	disable Landlock (as a baseline)\n");
> +	printf("  -d D	set directory depth to D\n");
> +	printf("  -n N	set number of benchmark iterations to N\n");
> +}
> +
> +/*
> + * Build a deep directory, enforce Landlock and return the FD to the
> + * deepest dir.  On any failure, exit the process with an error.
> + */
> +int build_directory(size_t depth, bool use_landlock)

const

> +{
> +	const char *path = "d"; /* directory name */
> +	int abi, ruleset_fd, current, previous;
> +
> +	if (use_landlock) {
> +		abi = syscall(SYS_landlock_create_ruleset, NULL, 0,
> +			      LANDLOCK_CREATE_RULESET_VERSION);

Please include wrappers.h and use the related syscall helpers.  One of
the benefit is to use __NR_* constants defined by the installed kernel
headers.

> +		if (abi < 7)
> +			err(1, "Landlock ABI too low: got %d, wanted 7+", abi);
> +	}
> +
> +	ruleset_fd = -1;
> +	if (use_landlock) {
> +		struct landlock_ruleset_attr attr = {
> +			.handled_access_fs =
> +				0xffff, /* All FS access rights as of 2026-01 */
> +		};
> +		ruleset_fd = syscall(SYS_landlock_create_ruleset, &attr,
> +				     sizeof(attr), 0U);
> +		if (ruleset_fd < 0)
> +			err(1, "landlock_create_ruleset");
> +	}
> +
> +	current = open(".", O_PATH);
> +	if (current < 0)
> +		err(1, "open(.)");
> +
> +	while (depth--) {
> +		if (use_landlock) {
> +			struct landlock_path_beneath_attr attr = {
> +				.allowed_access = LANDLOCK_ACCESS_FS_IOCTL_DEV,
> +				.parent_fd = current,
> +			};
> +			if (syscall(SYS_landlock_add_rule, ruleset_fd,
> +				    LANDLOCK_RULE_PATH_BENEATH, &attr, 0) < 0)
> +				err(1, "landlock_add_rule");
> +		}
> +
> +		if (mkdirat(current, path, 0700) < 0)
> +			err(1, "mkdirat(%s)", path);

We should have a loop to build the directories, then start the timer and
have another loop to add Landlock rules.

> +
> +		previous = current;
> +		current = openat(current, path, O_PATH);
> +		if (current < 0)
> +			err(1, "open(%s)", path);
> +
> +		close(previous);
> +	}
> +
> +	if (use_landlock) {
> +		if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0)
> +			err(1, "prctl");
> +
> +		if (syscall(SYS_landlock_restrict_self, ruleset_fd, 0) < 0)
> +			err(1, "landlock_restrict_self");
> +	}
> +
> +	close(ruleset_fd);
> +	return current;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	bool use_landlock = true;
> +	size_t num_iterations = 100000;
> +	size_t num_subdirs = 10000;
> +	int c, current, fd;
> +	struct tms start_time, end_time;
> +
> +	setbuf(stdout, NULL);
> +	while ((c = getopt(argc, argv, "hLd:n:")) != -1) {
> +		switch (c) {
> +		case 'h':
> +			usage(argv[0]);
> +			return EXIT_SUCCESS;
> +		case 'L':
> +			use_landlock = false;
> +			break;
> +		case 'd':
> +			num_subdirs = atoi(optarg);
> +			break;
> +		case 'n':
> +			num_iterations = atoi(optarg);
> +			break;
> +		default:
> +			usage(argv[0]);
> +			return EXIT_FAILURE;
> +		}
> +	}
> +
> +	printf("*** Benchmark ***\n");

We should probably use ksft_*() helpers in main (see
seccomp_benchmark.c).

> +	printf("%zu dirs, %zu iterations, %s landlock\n", num_subdirs,
> +	       num_iterations, use_landlock ? "with" : "without");
> +
> +	if (times(&start_time) == -1)
> +		err(1, "times");
> +
> +	current = build_directory(num_subdirs, use_landlock);
> +
> +	for (int i = 0; i < num_iterations; i++) {
> +		fd = openat(current, ".", O_DIRECTORY);

We can use AT_EMPTY_PATH (with an empty path) instead of "."
I guess the benchmark should not change, but better to check again.

> +		if (fd != -1) {
> +			if (use_landlock)
> +				errx(1, "openat succeeded, expected error");
> +
> +			close(fd);
> +		}
> +	}
> +
> +	if (times(&end_time) == -1)
> +		err(1, "times");

The created directories should be removed here (setup and teardown).

> +
> +	printf("*** Benchmark concluded ***\n");
> +	printf("System: %ld clocks\n",
> +	       end_time.tms_stime - start_time.tms_stime);
> +	printf("User  : %ld clocks\n",
> +	       end_time.tms_utime - start_time.tms_utime);
> +	printf("Clocks per second: %ld\n", CLOCKS_PER_SEC);
> +
> +	close(current);
> +}
> -- 
> 2.52.0
> 
> 

Re: [PATCH v2 1/3] selftests/landlock: Add filesystem access benchmark

[Günther Noack]

Hello!

On Wed, Jan 28, 2026 at 10:31:23PM +0100, Mickaël Salaün wrote:
> On Sun, Jan 25, 2026 at 08:58:51PM +0100, Günther Noack wrote:
> > --- /dev/null
> > +++ b/tools/testing/selftests/landlock/fs_bench.c
> > @@ -0,0 +1,161 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Landlock filesystem benchmark
> 
> You might want to add some copyright.

Done.


> > +void usage(const char *argv0)
> 
> const

Done.


> > +int build_directory(size_t depth, bool use_landlock)
> 
> const

Done.


> > +	if (use_landlock) {
> > +		abi = syscall(SYS_landlock_create_ruleset, NULL, 0,
> > +			      LANDLOCK_CREATE_RULESET_VERSION);
> 
> Please include wrappers.h and use the related syscall helpers.  One of
> the benefit is to use __NR_* constants defined by the installed kernel
> headers.

Done.


> > +		if (abi < 7)
> > +			err(1, "Landlock ABI too low: got %d, wanted 7+", abi);
> > +	}
> > +
> > +	ruleset_fd = -1;
> > +	if (use_landlock) {
> > +		struct landlock_ruleset_attr attr = {
> > +			.handled_access_fs =
> > +				0xffff, /* All FS access rights as of 2026-01 */
> > +		};
> > +		ruleset_fd = syscall(SYS_landlock_create_ruleset, &attr,
> > +				     sizeof(attr), 0U);
> > +		if (ruleset_fd < 0)
> > +			err(1, "landlock_create_ruleset");
> > +	}
> > +
> > +	current = open(".", O_PATH);
> > +	if (current < 0)
> > +		err(1, "open(.)");
> > +
> > +	while (depth--) {
> > +		if (use_landlock) {
> > +			struct landlock_path_beneath_attr attr = {
> > +				.allowed_access = LANDLOCK_ACCESS_FS_IOCTL_DEV,
> > +				.parent_fd = current,
> > +			};
> > +			if (syscall(SYS_landlock_add_rule, ruleset_fd,
> > +				    LANDLOCK_RULE_PATH_BENEATH, &attr, 0) < 0)
> > +				err(1, "landlock_add_rule");
> > +		}
> > +
> > +		if (mkdirat(current, path, 0700) < 0)
> > +			err(1, "mkdirat(%s)", path);
> 
> We should have a loop to build the directories, then start the timer and
> have another loop to add Landlock rules.

I have to politely push back on this; the granularity of time
measurement is not high enough and the measurement below only works
because we repeat it 100000 times.  This is not the case when we
construct a Landlock ruleset, and it would IMHO be weird to build the
ruleset multiple times as well.  It feels like this would better be
measured in a separate benchmark.

Adding a rule is an operation whose runtime does not depend on the
depth of the nested directories, so such a separate benchmark would
then also be simpler and wouldn't need to construct such a deeply
nested hierarchy.


> > +	printf("*** Benchmark ***\n");
> 
> We should probably use ksft_*() helpers in main (see
> seccomp_benchmark.c).

Among the benchmarks, the seccomp benchmark is the one exception in
that it uses these ksft_*() helpers, and it's not clear to me that it
has any benefit.  These helpers are for producing TAP-formatted
output, and assume that there will be individual test cases with
success/failure results, which is not the case here.  The seccomp test
uses approximate assertions about the expected timing of operations
(+-10%), but I don't think we can easily do that in our case.

I would therefore prefer to use a normal textual output format,
similar to the other benchmarks in tools/testing/kselftests.


> > +	printf("%zu dirs, %zu iterations, %s landlock\n", num_subdirs,
> > +	       num_iterations, use_landlock ? "with" : "without");
> > +
> > +	if (times(&start_time) == -1)
> > +		err(1, "times");
> > +
> > +	current = build_directory(num_subdirs, use_landlock);
> > +
> > +	for (int i = 0; i < num_iterations; i++) {
> > +		fd = openat(current, ".", O_DIRECTORY);
> 
> We can use AT_EMPTY_PATH (with an empty path) instead of "."
> I guess the benchmark should not change, but better to check again.

This had to change anyway; now that I added cleanup of the created
directories, I had to use another operation here that would trigger
the path walk (file open for creation).  Opening directories and
removing directories both need to continue working so that we can
later remove the directories. (See discussion below.)


> > +		if (fd != -1) {
> > +			if (use_landlock)
> > +				errx(1, "openat succeeded, expected error");
> > +
> > +			close(fd);
> > +		}
> > +	}
> > +
> > +	if (times(&end_time) == -1)
> > +		err(1, "times");
> 
> The created directories should be removed here (setup and teardown).

Done.

Minor implementation remark: This is also done with explicit loops
that use openat() to walk the directory tree with file descriptors and
then unlinkat(fd, "d", ...).  At this nesting depth, the paths don't
fit into PATH_MAX any more and relative dirfds are the only way to do
that AFAIK.  (The directory walk function nftw(3) also breaks down
FWIW, because it uses long paths relative to cwd.)

–Günther

Re: [PATCH v2 1/3] selftests/landlock: Add filesystem access benchmark

[Mickaël Salaün]

On Fri, Feb 06, 2026 at 01:24:02PM +0100, Günther Noack wrote:
> Hello!
> 
> On Wed, Jan 28, 2026 at 10:31:23PM +0100, Mickaël Salaün wrote:
> > On Sun, Jan 25, 2026 at 08:58:51PM +0100, Günther Noack wrote:
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/landlock/fs_bench.c

> > > +		if (abi < 7)
> > > +			err(1, "Landlock ABI too low: got %d, wanted 7+", abi);
> > > +	}
> > > +
> > > +	ruleset_fd = -1;
> > > +	if (use_landlock) {
> > > +		struct landlock_ruleset_attr attr = {
> > > +			.handled_access_fs =
> > > +				0xffff, /* All FS access rights as of 2026-01 */
> > > +		};
> > > +		ruleset_fd = syscall(SYS_landlock_create_ruleset, &attr,
> > > +				     sizeof(attr), 0U);
> > > +		if (ruleset_fd < 0)
> > > +			err(1, "landlock_create_ruleset");
> > > +	}
> > > +
> > > +	current = open(".", O_PATH);
> > > +	if (current < 0)
> > > +		err(1, "open(.)");
> > > +
> > > +	while (depth--) {
> > > +		if (use_landlock) {
> > > +			struct landlock_path_beneath_attr attr = {
> > > +				.allowed_access = LANDLOCK_ACCESS_FS_IOCTL_DEV,
> > > +				.parent_fd = current,
> > > +			};
> > > +			if (syscall(SYS_landlock_add_rule, ruleset_fd,
> > > +				    LANDLOCK_RULE_PATH_BENEATH, &attr, 0) < 0)
> > > +				err(1, "landlock_add_rule");
> > > +		}
> > > +
> > > +		if (mkdirat(current, path, 0700) < 0)
> > > +			err(1, "mkdirat(%s)", path);
> > 
> > We should have a loop to build the directories, then start the timer and
> > have another loop to add Landlock rules.
> 
> I have to politely push back on this; the granularity of time
> measurement is not high enough and the measurement below only works
> because we repeat it 100000 times.  This is not the case when we
> construct a Landlock ruleset, and it would IMHO be weird to build the
> ruleset multiple times as well.  It feels like this would better be
> measured in a separate benchmark.
> 
> Adding a rule is an operation whose runtime does not depend on the
> depth of the nested directories, so such a separate benchmark would
> then also be simpler and wouldn't need to construct such a deeply
> nested hierarchy.

OK.  Please add this explanation in a comment.

> 
> 
> > > +	printf("*** Benchmark ***\n");
> > 
> > We should probably use ksft_*() helpers in main (see
> > seccomp_benchmark.c).
> 
> Among the benchmarks, the seccomp benchmark is the one exception in
> that it uses these ksft_*() helpers, and it's not clear to me that it
> has any benefit.  These helpers are for producing TAP-formatted
> output, and assume that there will be individual test cases with
> success/failure results, which is not the case here.  The seccomp test
> uses approximate assertions about the expected timing of operations
> (+-10%), but I don't think we can easily do that in our case.
> 
> I would therefore prefer to use a normal textual output format,
> similar to the other benchmarks in tools/testing/kselftests.

OK

> 
> 
> > > +	printf("%zu dirs, %zu iterations, %s landlock\n", num_subdirs,
> > > +	       num_iterations, use_landlock ? "with" : "without");
> > > +
> > > +	if (times(&start_time) == -1)
> > > +		err(1, "times");
> > > +
> > > +	current = build_directory(num_subdirs, use_landlock);
> > > +
> > > +	for (int i = 0; i < num_iterations; i++) {
> > > +		fd = openat(current, ".", O_DIRECTORY);
> > 
> > We can use AT_EMPTY_PATH (with an empty path) instead of "."
> > I guess the benchmark should not change, but better to check again.
> 
> This had to change anyway; now that I added cleanup of the created
> directories, I had to use another operation here that would trigger
> the path walk (file open for creation).  Opening directories and
> removing directories both need to continue working so that we can
> later remove the directories. (See discussion below.)
> 
> 
> > > +		if (fd != -1) {
> > > +			if (use_landlock)
> > > +				errx(1, "openat succeeded, expected error");
> > > +
> > > +			close(fd);
> > > +		}
> > > +	}
> > > +
> > > +	if (times(&end_time) == -1)
> > > +		err(1, "times");
> > 
> > The created directories should be removed here (setup and teardown).
> 
> Done.
> 
> Minor implementation remark: This is also done with explicit loops
> that use openat() to walk the directory tree with file descriptors and
> then unlinkat(fd, "d", ...).  At this nesting depth, the paths don't
> fit into PATH_MAX any more and relative dirfds are the only way to do
> that AFAIK.  (The directory walk function nftw(3) also breaks down
> FWIW, because it uses long paths relative to cwd.)
> 
> –Günther
> 

Re: [PATCH v2 1/3] selftests/landlock: Add filesystem access benchmark

[Günther Noack]

On Fri, Feb 06, 2026 at 01:59:41PM +0100, Mickaël Salaün wrote:
> On Fri, Feb 06, 2026 at 01:24:02PM +0100, Günther Noack wrote:
> > On Wed, Jan 28, 2026 at 10:31:23PM +0100, Mickaël Salaün wrote:
> > > We should have a loop to build the directories, then start the timer and
> > > have another loop to add Landlock rules.
> > 
> > I have to politely push back on this; the granularity of time
> > measurement is not high enough and the measurement below only works
> > because we repeat it 100000 times.  This is not the case when we
> > construct a Landlock ruleset, and it would IMHO be weird to build the
> > ruleset multiple times as well.  It feels like this would better be
> > measured in a separate benchmark.
> > 
> > Adding a rule is an operation whose runtime does not depend on the
> > depth of the nested directories, so such a separate benchmark would
> > then also be simpler and wouldn't need to construct such a deeply
> > nested hierarchy.
> 
> OK.  Please add this explanation in a comment.

Done.

–Günther

[PATCH v2 2/3] landlock: access_mask_subset() helper

[Günther Noack]

This helper function checks whether an access_mask_t has a subset of the
bits enabled than another one.  This expresses the intent a bit smoother
in the code and does not cost us anything when it gets inlined.

Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
 security/landlock/access.h | 6 ++++++
 security/landlock/fs.c     | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/security/landlock/access.h b/security/landlock/access.h
index 7961c6630a2d..5c0caef9eaf6 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -97,4 +97,10 @@ landlock_upgrade_handled_access_masks(struct access_masks access_masks)
 	return access_masks;
 }
 
+/** access_mask_subset - true iff a has a subset of the bits of b. */
+static inline bool access_mask_subset(access_mask_t a, access_mask_t b)
+{
+	return (a | b) == b;
+}
+
 #endif /* _SECURITY_LANDLOCK_ACCESS_H */
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 8205673c8b1c..bf8e37fcc7c0 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1704,7 +1704,7 @@ static int hook_file_open(struct file *const file)
 		ARRAY_SIZE(layer_masks));
 #endif /* CONFIG_AUDIT */
 
-	if ((open_access_request & allowed_access) == open_access_request)
+	if (access_mask_subset(open_access_request, allowed_access))
 		return 0;
 
 	/* Sets access to reflect the actual request. */
-- 
2.52.0

Re: [PATCH v2 2/3] landlock: access_mask_subset() helper

[Randy Dunlap]

Hi,

On 1/25/26 11:58 AM, Günther Noack wrote:
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 7961c6630a2d..5c0caef9eaf6 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -97,4 +97,10 @@ landlock_upgrade_handled_access_masks(struct access_masks access_masks)
>  	return access_masks;
>  }
>  
> +/** access_mask_subset - true iff a has a subset of the bits of b. */
> +static inline bool access_mask_subset(access_mask_t a, access_mask_t b)
> +{
> +	return (a | b) == b;
> +}

Don't use "/**" for comments that are not in kernel-doc format.
This function doesn't need kernel-doc comments, so just use "/*"
here, please.

-- 
~Randy

Re: [PATCH v2 2/3] landlock: access_mask_subset() helper

[Günther Noack]

On Sun, Jan 25, 2026 at 01:48:52PM -0800, Randy Dunlap wrote:
> Hi,
> 
> On 1/25/26 11:58 AM, Günther Noack wrote:
> > diff --git a/security/landlock/access.h b/security/landlock/access.h
> > index 7961c6630a2d..5c0caef9eaf6 100644
> > --- a/security/landlock/access.h
> > +++ b/security/landlock/access.h
> > @@ -97,4 +97,10 @@ landlock_upgrade_handled_access_masks(struct access_masks access_masks)
> >  	return access_masks;
> >  }
> >  
> > +/** access_mask_subset - true iff a has a subset of the bits of b. */
> > +static inline bool access_mask_subset(access_mask_t a, access_mask_t b)
> > +{
> > +	return (a | b) == b;
> > +}
> 
> Don't use "/**" for comments that are not in kernel-doc format.
> This function doesn't need kernel-doc comments, so just use "/*"
> here, please.

Thanks for the correction, will be fixed for next revision. :)

–Günther

Re: [PATCH v2 2/3] landlock: access_mask_subset() helper

[Mickaël Salaün]

On Sun, Jan 25, 2026 at 08:58:52PM +0100, Günther Noack wrote:
> This helper function checks whether an access_mask_t has a subset of the
> bits enabled than another one.  This expresses the intent a bit smoother
> in the code and does not cost us anything when it gets inlined.
> 
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
>  security/landlock/access.h | 6 ++++++
>  security/landlock/fs.c     | 2 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 7961c6630a2d..5c0caef9eaf6 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -97,4 +97,10 @@ landlock_upgrade_handled_access_masks(struct access_masks access_masks)
>  	return access_masks;
>  }
>  
> +/** access_mask_subset - true iff a has a subset of the bits of b. */
> +static inline bool access_mask_subset(access_mask_t a, access_mask_t b)

What about renaming "a" to "subset" and "b" to "superset"?

> +{
> +	return (a | b) == b;
> +}
> +
>  #endif /* _SECURITY_LANDLOCK_ACCESS_H */
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 8205673c8b1c..bf8e37fcc7c0 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1704,7 +1704,7 @@ static int hook_file_open(struct file *const file)
>  		ARRAY_SIZE(layer_masks));
>  #endif /* CONFIG_AUDIT */
>  
> -	if ((open_access_request & allowed_access) == open_access_request)
> +	if (access_mask_subset(open_access_request, allowed_access))
>  		return 0;
>  
>  	/* Sets access to reflect the actual request. */
> -- 
> 2.52.0
> 
> 

Re: [PATCH v2 2/3] landlock: access_mask_subset() helper

[Günther Noack]

On Wed, Jan 28, 2026 at 10:31:52PM +0100, Mickaël Salaün wrote:
> On Sun, Jan 25, 2026 at 08:58:52PM +0100, Günther Noack wrote:
> > --- a/security/landlock/access.h
> > +++ b/security/landlock/access.h
> > @@ -97,4 +97,10 @@ landlock_upgrade_handled_access_masks(struct access_masks access_masks)
> >  	return access_masks;
> >  }
> >  
> > +/** access_mask_subset - true iff a has a subset of the bits of b. */
> > +static inline bool access_mask_subset(access_mask_t a, access_mask_t b)
> 
> What about renaming "a" to "subset" and "b" to "superset"?

Sure, sounds reasonable. Will be done in V3.

> > +{
> > +	return (a | b) == b;
> > +}

–Günther

[PATCH v2 3/3] landlock: transpose the layer masks data structure

[Günther Noack]

The layer masks data structure tracks the requested but unfulfilled
access rights during an operation's security check.  It stores one bit
for each combination of access right and layer index.  If the bit is
set, that access right is not granted (yet) in the given layer and we
have to traverse the path further upwards to grant it.

Previously, the layer masks were stored as arrays mapping from access
right indices to layer_mask_t.  The layer_mask_t value then indicates
all layers in which the given access right is still (tentatively)
denied.

This patch introduces struct layer_access_masks instead: This struct
contains an array with the access_mask_t of each (tentatively) denied
access right in that layer.

The hypothesis of this patch is that this simplifies the code enough
so that the resulting code will run faster:

* We can use bitwise operations in multiple places where we previously
  looped over bits individually with macros.  (Should require less
  branch speculation and lends itself to better loop unrolling.)

* Code is ~75 lines smaller.

Other noteworthy changes:

* In no_more_access(), call a new helper function may_refer(), which
  only solves the asymmetric case.  Previously, the code interleaved
  the checks for the two symmetric cases in RENAME_EXCHANGE.  It feels
  that the code is clearer when renames without RENAME_EXCHANGE are
  more obviously the normal case.

Tradeoffs:

This change improves performance, at a slight size increase to the
layer masks data structure.

At the moment, for the filesystem access rights, the data structure
has the same size as before, but once we introduce the 17th filesystem
access right, it will double in size (from 32 to 64 bytes), as
access_mask_t grows from 16 to 32 bit.  See the link below for
measurements.

Link: https://lore.kernel.org/all/20260120.haeCh4li9Vae@digikod.net/
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
 security/landlock/access.h  |  10 +-
 security/landlock/audit.c   |  84 +++------
 security/landlock/audit.h   |   3 +-
 security/landlock/domain.c  |  45 +++--
 security/landlock/domain.h  |   4 +-
 security/landlock/fs.c      | 352 ++++++++++++++++--------------------
 security/landlock/net.c     |  11 +-
 security/landlock/ruleset.c |  88 ++++-----
 security/landlock/ruleset.h |  21 ++-
 9 files changed, 274 insertions(+), 344 deletions(-)

diff --git a/security/landlock/access.h b/security/landlock/access.h
index 5c0caef9eaf6..1c911fa3555d 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -61,14 +61,14 @@ union access_masks_all {
 static_assert(sizeof(typeof_member(union access_masks_all, masks)) ==
 	      sizeof(typeof_member(union access_masks_all, all)));
 
-typedef u16 layer_mask_t;
-
-/* Makes sure all layers can be checked. */
-static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
-
 /*
  * Tracks domains responsible of a denied access.  This is required to avoid
  * storing in each object the full layer_masks[] required by update_request().
+ *
+ * Each nibble represents the layer index of the newest layer which denied a
+ * certain access right.  For file system access rights, the upper four bits are
+ * the index of the layer which denies LANDLOCK_ACCESS_FS_IOCTL_DEV and the
+ * lower nibble represents LANDLOCK_ACCESS_FS_TRUNCATE.
  */
 typedef u8 deny_masks_t;
 
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index e899995f1fd5..979a33f480aa 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -180,38 +180,21 @@ static void test_get_hierarchy(struct kunit *const test)
 
 #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
 
+/* get_denied_layer - get the youngest layer that denied the access_request */
 static size_t get_denied_layer(const struct landlock_ruleset *const domain,
 			       access_mask_t *const access_request,
-			       const layer_mask_t (*const layer_masks)[],
-			       const size_t layer_masks_size)
+			       const struct layer_access_masks *masks)
 {
-	const unsigned long access_req = *access_request;
-	unsigned long access_bit;
-	access_mask_t missing = 0;
-	long youngest_layer = -1;
-
-	for_each_set_bit(access_bit, &access_req, layer_masks_size) {
-		const layer_mask_t mask = (*layer_masks)[access_bit];
-		long layer;
-
-		if (!mask)
-			continue;
-
-		/* __fls(1) == 0 */
-		layer = __fls(mask);
-		if (layer > youngest_layer) {
-			youngest_layer = layer;
-			missing = BIT(access_bit);
-		} else if (layer == youngest_layer) {
-			missing |= BIT(access_bit);
+	for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
+		if (masks->access[i] & *access_request) {
+			*access_request &= masks->access[i];
+			return i;
 		}
 	}
 
-	*access_request = missing;
-	if (youngest_layer == -1)
-		return domain->num_layers - 1;
-
-	return youngest_layer;
+	/* Not found - fall back to default values */
+	*access_request = 0;
+	return domain->num_layers - 1;
 }
 
 #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
@@ -221,50 +204,39 @@ static void test_get_denied_layer(struct kunit *const test)
 	const struct landlock_ruleset dom = {
 		.num_layers = 5,
 	};
-	const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT(0),
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT(1),
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = BIT(1) | BIT(0),
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = BIT(2),
+	const struct layer_access_masks masks = {
+		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
+			     LANDLOCK_ACCESS_FS_READ_DIR,
+		.access[1] = LANDLOCK_ACCESS_FS_READ_FILE |
+			     LANDLOCK_ACCESS_FS_READ_DIR,
+		.access[2] = LANDLOCK_ACCESS_FS_REMOVE_DIR,
 	};
 	access_mask_t access;
 
 	access = LANDLOCK_ACCESS_FS_EXECUTE;
-	KUNIT_EXPECT_EQ(test, 0,
-			get_denied_layer(&dom, &access, &layer_masks,
-					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, 0, get_denied_layer(&dom, &access, &masks));
 	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_EXECUTE);
 
 	access = LANDLOCK_ACCESS_FS_READ_FILE;
-	KUNIT_EXPECT_EQ(test, 1,
-			get_denied_layer(&dom, &access, &layer_masks,
-					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
 	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_FILE);
 
 	access = LANDLOCK_ACCESS_FS_READ_DIR;
-	KUNIT_EXPECT_EQ(test, 1,
-			get_denied_layer(&dom, &access, &layer_masks,
-					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
 	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
 
 	access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR;
-	KUNIT_EXPECT_EQ(test, 1,
-			get_denied_layer(&dom, &access, &layer_masks,
-					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
 	KUNIT_EXPECT_EQ(test, access,
 			LANDLOCK_ACCESS_FS_READ_FILE |
 				LANDLOCK_ACCESS_FS_READ_DIR);
 
 	access = LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_READ_DIR;
-	KUNIT_EXPECT_EQ(test, 1,
-			get_denied_layer(&dom, &access, &layer_masks,
-					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
 	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
 
 	access = LANDLOCK_ACCESS_FS_WRITE_FILE;
-	KUNIT_EXPECT_EQ(test, 4,
-			get_denied_layer(&dom, &access, &layer_masks,
-					 sizeof(layer_masks)));
+	KUNIT_EXPECT_EQ(test, 4, get_denied_layer(&dom, &access, &masks));
 	KUNIT_EXPECT_EQ(test, access, 0);
 }
 
@@ -361,18 +333,15 @@ static bool is_valid_request(const struct landlock_request *const request)
 		return false;
 
 	if (request->access) {
-		if (WARN_ON_ONCE(!(!!request->layer_masks ^
+		if (WARN_ON_ONCE(!(!!request->masks ^
 				   !!request->all_existing_optional_access)))
 			return false;
 	} else {
-		if (WARN_ON_ONCE(request->layer_masks ||
+		if (WARN_ON_ONCE(request->masks ||
 				 request->all_existing_optional_access))
 			return false;
 	}
 
-	if (WARN_ON_ONCE(!!request->layer_masks ^ !!request->layer_masks_size))
-		return false;
-
 	if (request->deny_masks) {
 		if (WARN_ON_ONCE(!request->all_existing_optional_access))
 			return false;
@@ -405,13 +374,12 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
 	missing = request->access;
 	if (missing) {
 		/* Gets the nearest domain that denies the request. */
-		if (request->layer_masks) {
+		if (request->masks) {
 			youngest_layer = get_denied_layer(
-				subject->domain, &missing, request->layer_masks,
-				request->layer_masks_size);
+				subject->domain, &missing, request->masks);
 		} else {
 			youngest_layer = get_layer_from_deny_masks(
-				&missing, request->all_existing_optional_access,
+				&missing, _LANDLOCK_ACCESS_FS_OPTIONAL,
 				request->deny_masks);
 		}
 		youngest_denied =
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 92428b7fc4d8..104472060ef5 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -43,8 +43,7 @@ struct landlock_request {
 	access_mask_t access;
 
 	/* Required fields for requests with layer masks. */
-	const layer_mask_t (*layer_masks)[];
-	size_t layer_masks_size;
+	const struct layer_access_masks *masks;
 
 	/* Required fields for requests with deny masks. */
 	const access_mask_t all_existing_optional_access;
diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index a647b68e8d06..5b11ddb22d3a 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -7,6 +7,7 @@
  * Copyright © 2024-2025 Microsoft Corporation
  */
 
+#include "ruleset.h"
 #include <kunit/test.h>
 #include <linux/bitops.h>
 #include <linux/bits.h>
@@ -182,32 +183,36 @@ static void test_get_layer_deny_mask(struct kunit *const test)
 deny_masks_t
 landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
 			const access_mask_t optional_access,
-			const layer_mask_t (*const layer_masks)[],
-			const size_t layer_masks_size)
+			const struct layer_access_masks *const masks)
 {
 	const unsigned long access_opt = optional_access;
 	unsigned long access_bit;
+	access_mask_t all_denied = 0;
 	deny_masks_t deny_masks = 0;
 
 	/* This may require change with new object types. */
-	WARN_ON_ONCE(access_opt !=
-		     (optional_access & all_existing_optional_access));
+	WARN_ON_ONCE(!access_mask_subset(optional_access,
+					 all_existing_optional_access));
 
-	if (WARN_ON_ONCE(!layer_masks))
+	if (WARN_ON_ONCE(!masks))
 		return 0;
 
 	if (WARN_ON_ONCE(!access_opt))
 		return 0;
 
-	for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
-		const layer_mask_t mask = (*layer_masks)[access_bit];
+	for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
+		const access_mask_t denied = masks->access[i] & optional_access;
+		const unsigned long newly_denied = denied & ~all_denied;
 
-		if (!mask)
+		if (!newly_denied)
 			continue;
 
-		/* __fls(1) == 0 */
-		deny_masks |= get_layer_deny_mask(all_existing_optional_access,
-						  access_bit, __fls(mask));
+		for_each_set_bit(access_bit, &newly_denied,
+				 8 * sizeof(access_mask_t)) {
+			deny_masks |= get_layer_deny_mask(
+				all_existing_optional_access, access_bit, i);
+		}
+		all_denied |= denied;
 	}
 	return deny_masks;
 }
@@ -216,28 +221,28 @@ landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
 
 static void test_landlock_get_deny_masks(struct kunit *const test)
 {
-	const layer_mask_t layers1[BITS_PER_TYPE(access_mask_t)] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
-							  BIT_ULL(9),
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = BIT_ULL(1),
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = BIT_ULL(2) |
-							    BIT_ULL(0),
+	const struct layer_access_masks layers1 = {
+		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
+			     LANDLOCK_ACCESS_FS_IOCTL_DEV,
+		.access[1] = LANDLOCK_ACCESS_FS_TRUNCATE,
+		.access[2] = LANDLOCK_ACCESS_FS_IOCTL_DEV,
+		.access[9] = LANDLOCK_ACCESS_FS_EXECUTE,
 	};
 
 	KUNIT_EXPECT_EQ(test, 0x1,
 			landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
 						LANDLOCK_ACCESS_FS_TRUNCATE,
-						&layers1, ARRAY_SIZE(layers1)));
+						&layers1));
 	KUNIT_EXPECT_EQ(test, 0x20,
 			landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
 						LANDLOCK_ACCESS_FS_IOCTL_DEV,
-						&layers1, ARRAY_SIZE(layers1)));
+						&layers1));
 	KUNIT_EXPECT_EQ(
 		test, 0x21,
 		landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
 					LANDLOCK_ACCESS_FS_TRUNCATE |
 						LANDLOCK_ACCESS_FS_IOCTL_DEV,
-					&layers1, ARRAY_SIZE(layers1)));
+					&layers1));
 }
 
 #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index 621f054c9a2b..227066d667f7 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -10,6 +10,7 @@
 #ifndef _SECURITY_LANDLOCK_DOMAIN_H
 #define _SECURITY_LANDLOCK_DOMAIN_H
 
+#include "ruleset.h"
 #include <linux/limits.h>
 #include <linux/mm.h>
 #include <linux/path.h>
@@ -122,8 +123,7 @@ struct landlock_hierarchy {
 deny_masks_t
 landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
 			const access_mask_t optional_access,
-			const layer_mask_t (*const layer_masks)[],
-			size_t layer_masks_size);
+			const struct layer_access_masks *const masks);
 
 int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy);
 
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index bf8e37fcc7c0..cef0013c2cf6 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -398,57 +398,55 @@ static const struct access_masks any_fs = {
 	.fs = ~0,
 };
 
+/*
+ * Returns true iff the child file with the given src_child access rights under
+ * src_parent would result in having the same or fewer access rights if it were
+ * moved under new_parent.
+ */
+static bool may_refer(const struct layer_access_masks *const src_parent,
+		      const struct layer_access_masks *const src_child,
+		      const struct layer_access_masks *const new_parent,
+		      const bool child_is_dir)
+{
+	for (size_t i = 0; i < ARRAY_SIZE(new_parent->access); i++) {
+		access_mask_t child_access = src_parent->access[i] &
+					     src_child->access[i];
+		access_mask_t parent_access = new_parent->access[i];
+
+		if (!child_is_dir) {
+			child_access &= ACCESS_FILE;
+			parent_access &= ACCESS_FILE;
+		}
+
+		if (!access_mask_subset(child_access, parent_access))
+			return false;
+	}
+	return true;
+}
+
 /*
  * Check that a destination file hierarchy has more restrictions than a source
  * file hierarchy.  This is only used for link and rename actions.
  *
- * @layer_masks_child2: Optional child masks.
+ * Returns: true if child1 may be moved from parent1 to parent2 without
+ * increasing its access rights.  If child2 is set, an additional condition is
+ * that child2 may be used from parent2 to parent1 without increasing its access
+ * rights.
  */
-static bool no_more_access(
-	const layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
-	const layer_mask_t (*const layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS],
-	const bool child1_is_directory,
-	const layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
-	const layer_mask_t (*const layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS],
-	const bool child2_is_directory)
+static bool no_more_access(const struct layer_access_masks *const parent1,
+			   const struct layer_access_masks *const child1,
+			   const bool child1_is_dir,
+			   const struct layer_access_masks *const parent2,
+			   const struct layer_access_masks *const child2,
+			   const bool child2_is_dir)
 {
-	unsigned long access_bit;
+	if (!may_refer(parent1, child1, parent2, child1_is_dir))
+		return false;
 
-	for (access_bit = 0; access_bit < ARRAY_SIZE(*layer_masks_parent2);
-	     access_bit++) {
-		/* Ignores accesses that only make sense for directories. */
-		const bool is_file_access =
-			!!(BIT_ULL(access_bit) & ACCESS_FILE);
+	if (!child2)
+		return true;
 
-		if (child1_is_directory || is_file_access) {
-			/*
-			 * Checks if the destination restrictions are a
-			 * superset of the source ones (i.e. inherited access
-			 * rights without child exceptions):
-			 * restrictions(parent2) >= restrictions(child1)
-			 */
-			if ((((*layer_masks_parent1)[access_bit] &
-			      (*layer_masks_child1)[access_bit]) |
-			     (*layer_masks_parent2)[access_bit]) !=
-			    (*layer_masks_parent2)[access_bit])
-				return false;
-		}
-
-		if (!layer_masks_child2)
-			continue;
-		if (child2_is_directory || is_file_access) {
-			/*
-			 * Checks inverted restrictions for RENAME_EXCHANGE:
-			 * restrictions(parent1) >= restrictions(child2)
-			 */
-			if ((((*layer_masks_parent2)[access_bit] &
-			      (*layer_masks_child2)[access_bit]) |
-			     (*layer_masks_parent1)[access_bit]) !=
-			    (*layer_masks_parent1)[access_bit])
-				return false;
-		}
-	}
-	return true;
+	return may_refer(parent2, child2, parent1, child2_is_dir);
 }
 
 #define NMA_TRUE(...) KUNIT_EXPECT_TRUE(test, no_more_access(__VA_ARGS__))
@@ -458,25 +456,25 @@ static bool no_more_access(
 
 static void test_no_more_access(struct kunit *const test)
 {
-	const layer_mask_t rx0[LANDLOCK_NUM_ACCESS_FS] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT_ULL(0),
+	const struct layer_access_masks rx0 = {
+		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
+			     LANDLOCK_ACCESS_FS_READ_FILE,
 	};
-	const layer_mask_t mx0[LANDLOCK_NUM_ACCESS_FS] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_REG)] = BIT_ULL(0),
+	const struct layer_access_masks mx0 = {
+		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
+			     LANDLOCK_ACCESS_FS_MAKE_REG,
 	};
-	const layer_mask_t x0[LANDLOCK_NUM_ACCESS_FS] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
+	const struct layer_access_masks x0 = {
+		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
 	};
-	const layer_mask_t x1[LANDLOCK_NUM_ACCESS_FS] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(1),
+	const struct layer_access_masks x1 = {
+		.access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
 	};
-	const layer_mask_t x01[LANDLOCK_NUM_ACCESS_FS] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
-							  BIT_ULL(1),
+	const struct layer_access_masks x01 = {
+		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
+		.access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
 	};
-	const layer_mask_t allows_all[LANDLOCK_NUM_ACCESS_FS] = {};
+	const struct layer_access_masks allows_all = {};
 
 	/* Checks without restriction. */
 	NMA_TRUE(&x0, &allows_all, false, &allows_all, NULL, false);
@@ -564,31 +562,30 @@ static void test_no_more_access(struct kunit *const test)
 #undef NMA_TRUE
 #undef NMA_FALSE
 
-static bool is_layer_masks_allowed(
-	layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
+static bool is_layer_masks_allowed(const struct layer_access_masks *masks)
 {
-	return !memchr_inv(layer_masks, 0, sizeof(*layer_masks));
+	return !memchr_inv(&masks->access, 0, sizeof(masks->access));
 }
 
 /*
- * Removes @layer_masks accesses that are not requested.
+ * Removes @masks accesses that are not requested.
  *
  * Returns true if the request is allowed, false otherwise.
  */
-static bool
-scope_to_request(const access_mask_t access_request,
-		 layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
+static bool scope_to_request(const access_mask_t access_request,
+			     struct layer_access_masks *masks)
 {
-	const unsigned long access_req = access_request;
-	unsigned long access_bit;
+	bool saw_unfulfilled_access = false;
 
-	if (WARN_ON_ONCE(!layer_masks))
+	if (WARN_ON_ONCE(!masks))
 		return true;
 
-	for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
-		(*layer_masks)[access_bit] = 0;
-
-	return is_layer_masks_allowed(layer_masks);
+	for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
+		masks->access[i] &= access_request;
+		if (masks->access[i])
+			saw_unfulfilled_access = true;
+	}
+	return !saw_unfulfilled_access;
 }
 
 #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
@@ -596,48 +593,41 @@ scope_to_request(const access_mask_t access_request,
 static void test_scope_to_request_with_exec_none(struct kunit *const test)
 {
 	/* Allows everything. */
-	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+	struct layer_access_masks masks = {};
 
 	/* Checks and scopes with execute. */
-	KUNIT_EXPECT_TRUE(test, scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE,
-						 &layer_masks));
-	KUNIT_EXPECT_EQ(test, 0,
-			layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
-	KUNIT_EXPECT_EQ(test, 0,
-			layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
+	KUNIT_EXPECT_TRUE(test,
+			  scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE, &masks));
+	KUNIT_EXPECT_EQ(test, 0, masks.access[0]);
 }
 
 static void test_scope_to_request_with_exec_some(struct kunit *const test)
 {
 	/* Denies execute and write. */
-	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(1),
+	struct layer_access_masks masks = {
+		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
+		.access[1] = LANDLOCK_ACCESS_FS_WRITE_FILE,
 	};
 
 	/* Checks and scopes with execute. */
 	KUNIT_EXPECT_FALSE(test, scope_to_request(LANDLOCK_ACCESS_FS_EXECUTE,
-						  &layer_masks));
-	KUNIT_EXPECT_EQ(test, BIT_ULL(0),
-			layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
-	KUNIT_EXPECT_EQ(test, 0,
-			layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
+						  &masks));
+	KUNIT_EXPECT_EQ(test, LANDLOCK_ACCESS_FS_EXECUTE, masks.access[0]);
+	KUNIT_EXPECT_EQ(test, 0, masks.access[1]);
 }
 
 static void test_scope_to_request_without_access(struct kunit *const test)
 {
 	/* Denies execute and write. */
-	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(1),
+	struct layer_access_masks masks = {
+		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
+		.access[1] = LANDLOCK_ACCESS_FS_WRITE_FILE,
 	};
 
 	/* Checks and scopes without access request. */
-	KUNIT_EXPECT_TRUE(test, scope_to_request(0, &layer_masks));
-	KUNIT_EXPECT_EQ(test, 0,
-			layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)]);
-	KUNIT_EXPECT_EQ(test, 0,
-			layer_masks[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)]);
+	KUNIT_EXPECT_TRUE(test, scope_to_request(0, &masks));
+	KUNIT_EXPECT_EQ(test, 0, masks.access[0]);
+	KUNIT_EXPECT_EQ(test, 0, masks.access[1]);
 }
 
 #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
@@ -646,20 +636,16 @@ static void test_scope_to_request_without_access(struct kunit *const test)
  * Returns true if there is at least one access right different than
  * LANDLOCK_ACCESS_FS_REFER.
  */
-static bool
-is_eacces(const layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS],
-	  const access_mask_t access_request)
+static bool is_eacces(const struct layer_access_masks *masks,
+		      const access_mask_t access_request)
 {
-	unsigned long access_bit;
-	/* LANDLOCK_ACCESS_FS_REFER alone must return -EXDEV. */
-	const unsigned long access_check = access_request &
-					   ~LANDLOCK_ACCESS_FS_REFER;
-
-	if (!layer_masks)
+	if (!masks)
 		return false;
 
-	for_each_set_bit(access_bit, &access_check, ARRAY_SIZE(*layer_masks)) {
-		if ((*layer_masks)[access_bit])
+	for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
+		/* LANDLOCK_ACCESS_FS_REFER alone must return -EXDEV. */
+		if (masks->access[i] & access_request &
+		    ~LANDLOCK_ACCESS_FS_REFER)
 			return true;
 	}
 	return false;
@@ -672,37 +658,37 @@ is_eacces(const layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS],
 
 static void test_is_eacces_with_none(struct kunit *const test)
 {
-	const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+	const struct layer_access_masks masks = {};
 
-	IE_FALSE(&layer_masks, 0);
-	IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
-	IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
-	IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
+	IE_FALSE(&masks, 0);
+	IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
+	IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
+	IE_FALSE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
 }
 
 static void test_is_eacces_with_refer(struct kunit *const test)
 {
-	const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = BIT_ULL(0),
+	const struct layer_access_masks masks = {
+		.access[0] = LANDLOCK_ACCESS_FS_REFER,
 	};
 
-	IE_FALSE(&layer_masks, 0);
-	IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
-	IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
-	IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
+	IE_FALSE(&masks, 0);
+	IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
+	IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
+	IE_FALSE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
 }
 
 static void test_is_eacces_with_write(struct kunit *const test)
 {
-	const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
-		[BIT_INDEX(LANDLOCK_ACCESS_FS_WRITE_FILE)] = BIT_ULL(0),
+	const struct layer_access_masks masks = {
+		.access[0] = LANDLOCK_ACCESS_FS_WRITE_FILE,
 	};
 
-	IE_FALSE(&layer_masks, 0);
-	IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_REFER);
-	IE_FALSE(&layer_masks, LANDLOCK_ACCESS_FS_EXECUTE);
+	IE_FALSE(&masks, 0);
+	IE_FALSE(&masks, LANDLOCK_ACCESS_FS_REFER);
+	IE_FALSE(&masks, LANDLOCK_ACCESS_FS_EXECUTE);
 
-	IE_TRUE(&layer_masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
+	IE_TRUE(&masks, LANDLOCK_ACCESS_FS_WRITE_FILE);
 }
 
 #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
@@ -752,26 +738,25 @@ static void test_is_eacces_with_write(struct kunit *const test)
  * - true if the access request is granted;
  * - false otherwise.
  */
-static bool is_access_to_paths_allowed(
-	const struct landlock_ruleset *const domain,
-	const struct path *const path,
-	const access_mask_t access_request_parent1,
-	layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
-	struct landlock_request *const log_request_parent1,
-	struct dentry *const dentry_child1,
-	const access_mask_t access_request_parent2,
-	layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
-	struct landlock_request *const log_request_parent2,
-	struct dentry *const dentry_child2)
+static bool
+is_access_to_paths_allowed(const struct landlock_ruleset *const domain,
+			   const struct path *const path,
+			   const access_mask_t access_request_parent1,
+			   struct layer_access_masks *layer_masks_parent1,
+			   struct landlock_request *const log_request_parent1,
+			   struct dentry *const dentry_child1,
+			   const access_mask_t access_request_parent2,
+			   struct layer_access_masks *layer_masks_parent2,
+			   struct landlock_request *const log_request_parent2,
+			   struct dentry *const dentry_child2)
 {
 	bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
 	     child1_is_directory = true, child2_is_directory = true;
 	struct path walker_path;
 	access_mask_t access_masked_parent1, access_masked_parent2;
-	layer_mask_t _layer_masks_child1[LANDLOCK_NUM_ACCESS_FS],
-		_layer_masks_child2[LANDLOCK_NUM_ACCESS_FS];
-	layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
-	(*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
+	struct layer_access_masks _layer_masks_child1, _layer_masks_child2;
+	struct layer_access_masks *layer_masks_child1 = NULL,
+				  *layer_masks_child2 = NULL;
 
 	if (!access_request_parent1 && !access_request_parent2)
 		return true;
@@ -811,22 +796,20 @@ static bool is_access_to_paths_allowed(
 	}
 
 	if (unlikely(dentry_child1)) {
-		landlock_unmask_layers(
-			find_rule(domain, dentry_child1),
-			landlock_init_layer_masks(
-				domain, LANDLOCK_MASK_ACCESS_FS,
-				&_layer_masks_child1, LANDLOCK_KEY_INODE),
-			&_layer_masks_child1, ARRAY_SIZE(_layer_masks_child1));
+		if (landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
+					      &_layer_masks_child1,
+					      LANDLOCK_KEY_INODE))
+			landlock_unmask_layers(find_rule(domain, dentry_child1),
+					       &_layer_masks_child1);
 		layer_masks_child1 = &_layer_masks_child1;
 		child1_is_directory = d_is_dir(dentry_child1);
 	}
 	if (unlikely(dentry_child2)) {
-		landlock_unmask_layers(
-			find_rule(domain, dentry_child2),
-			landlock_init_layer_masks(
-				domain, LANDLOCK_MASK_ACCESS_FS,
-				&_layer_masks_child2, LANDLOCK_KEY_INODE),
-			&_layer_masks_child2, ARRAY_SIZE(_layer_masks_child2));
+		if (landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
+					      &_layer_masks_child2,
+					      LANDLOCK_KEY_INODE))
+			landlock_unmask_layers(find_rule(domain, dentry_child2),
+					       &_layer_masks_child2);
 		layer_masks_child2 = &_layer_masks_child2;
 		child2_is_directory = d_is_dir(dentry_child2);
 	}
@@ -881,16 +864,12 @@ static bool is_access_to_paths_allowed(
 		}
 
 		rule = find_rule(domain, walker_path.dentry);
-		allowed_parent1 = allowed_parent1 ||
-				  landlock_unmask_layers(
-					  rule, access_masked_parent1,
-					  layer_masks_parent1,
-					  ARRAY_SIZE(*layer_masks_parent1));
-		allowed_parent2 = allowed_parent2 ||
-				  landlock_unmask_layers(
-					  rule, access_masked_parent2,
-					  layer_masks_parent2,
-					  ARRAY_SIZE(*layer_masks_parent2));
+		allowed_parent1 =
+			allowed_parent1 ||
+			landlock_unmask_layers(rule, layer_masks_parent1);
+		allowed_parent2 =
+			allowed_parent2 ||
+			landlock_unmask_layers(rule, layer_masks_parent2);
 
 		/* Stops when a rule from each layer grants access. */
 		if (allowed_parent1 && allowed_parent2)
@@ -949,9 +928,7 @@ static bool is_access_to_paths_allowed(
 		log_request_parent1->audit.type = LSM_AUDIT_DATA_PATH;
 		log_request_parent1->audit.u.path = *path;
 		log_request_parent1->access = access_masked_parent1;
-		log_request_parent1->layer_masks = layer_masks_parent1;
-		log_request_parent1->layer_masks_size =
-			ARRAY_SIZE(*layer_masks_parent1);
+		log_request_parent1->masks = layer_masks_parent1;
 	}
 
 	if (!allowed_parent2 && log_request_parent2) {
@@ -959,9 +936,7 @@ static bool is_access_to_paths_allowed(
 		log_request_parent2->audit.type = LSM_AUDIT_DATA_PATH;
 		log_request_parent2->audit.u.path = *path;
 		log_request_parent2->access = access_masked_parent2;
-		log_request_parent2->layer_masks = layer_masks_parent2;
-		log_request_parent2->layer_masks_size =
-			ARRAY_SIZE(*layer_masks_parent2);
+		log_request_parent2->masks = layer_masks_parent2;
 	}
 #endif /* CONFIG_AUDIT */
 
@@ -976,7 +951,7 @@ static int current_check_access_path(const struct path *const path,
 	};
 	const struct landlock_cred_security *const subject =
 		landlock_get_applicable_subject(current_cred(), masks, NULL);
-	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+	struct layer_access_masks layer_masks;
 	struct landlock_request request = {};
 
 	if (!subject)
@@ -1051,12 +1026,11 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
  * - 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,
-	layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS])
+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)
 {
-	unsigned long access_dom;
 	bool ret = false;
 
 	if (WARN_ON_ONCE(!domain || !mnt_root || !dir || !layer_masks_dom))
@@ -1064,18 +1038,17 @@ static bool collect_domain_accesses(
 	if (is_nouser_or_private(dir))
 		return true;
 
-	access_dom = landlock_init_layer_masks(domain, LANDLOCK_MASK_ACCESS_FS,
-					       layer_masks_dom,
-					       LANDLOCK_KEY_INODE);
+	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), access_dom,
-					   layer_masks_dom,
-					   ARRAY_SIZE(*layer_masks_dom))) {
+		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.
@@ -1163,8 +1136,8 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 	access_mask_t access_request_parent1, access_request_parent2;
 	struct path mnt_dir;
 	struct dentry *old_parent;
-	layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
-		     layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
+	struct layer_access_masks layer_masks_parent1 = {},
+				  layer_masks_parent2 = {};
 	struct landlock_request request1 = {}, request2 = {};
 
 	if (!subject)
@@ -1640,7 +1613,7 @@ static bool is_device(const struct file *const file)
 
 static int hook_file_open(struct file *const file)
 {
-	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
+	struct layer_access_masks layer_masks = {};
 	access_mask_t open_access_request, full_access_request, allowed_access,
 		optional_access;
 	const struct landlock_cred_security *const subject =
@@ -1675,20 +1648,14 @@ static int hook_file_open(struct file *const file)
 		    &layer_masks, &request, NULL, 0, NULL, NULL, NULL)) {
 		allowed_access = full_access_request;
 	} else {
-		unsigned long access_bit;
-		const unsigned long access_req = full_access_request;
-
 		/*
 		 * Calculate the actual allowed access rights from layer_masks.
-		 * Add each access right to allowed_access which has not been
-		 * vetoed by any layer.
+		 * Remove the access rights from the full access request which
+		 * are still unfulfilled in any of the layers.
 		 */
-		allowed_access = 0;
-		for_each_set_bit(access_bit, &access_req,
-				 ARRAY_SIZE(layer_masks)) {
-			if (!layer_masks[access_bit])
-				allowed_access |= BIT_ULL(access_bit);
-		}
+		allowed_access = full_access_request;
+		for (size_t i = 0; i < ARRAY_SIZE(layer_masks.access); i++)
+			allowed_access &= ~layer_masks.access[i];
 	}
 
 	/*
@@ -1700,8 +1667,7 @@ static int hook_file_open(struct file *const file)
 	landlock_file(file)->allowed_access = allowed_access;
 #ifdef CONFIG_AUDIT
 	landlock_file(file)->deny_masks = landlock_get_deny_masks(
-		_LANDLOCK_ACCESS_FS_OPTIONAL, optional_access, &layer_masks,
-		ARRAY_SIZE(layer_masks));
+		_LANDLOCK_ACCESS_FS_OPTIONAL, optional_access, &layer_masks);
 #endif /* CONFIG_AUDIT */
 
 	if (access_mask_subset(open_access_request, allowed_access))
diff --git a/security/landlock/net.c b/security/landlock/net.c
index e6367e30e5b0..2a66b69b77a9 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -47,7 +47,7 @@ static int current_check_access_socket(struct socket *const sock,
 				       access_mask_t access_request)
 {
 	__be16 port;
-	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
+	struct layer_access_masks layer_masks = {};
 	const struct landlock_rule *rule;
 	struct landlock_id id = {
 		.type = LANDLOCK_KEY_NET_PORT,
@@ -194,8 +194,10 @@ static int current_check_access_socket(struct socket *const sock,
 	access_request = landlock_init_layer_masks(subject->domain,
 						   access_request, &layer_masks,
 						   LANDLOCK_KEY_NET_PORT);
-	if (landlock_unmask_layers(rule, access_request, &layer_masks,
-				   ARRAY_SIZE(layer_masks)))
+	if (!access_request)
+		return 0;
+
+	if (landlock_unmask_layers(rule, &layer_masks))
 		return 0;
 
 	audit_net.family = address->sa_family;
@@ -205,8 +207,7 @@ static int current_check_access_socket(struct socket *const sock,
 				    .audit.type = LSM_AUDIT_DATA_NET,
 				    .audit.u.net = &audit_net,
 				    .access = access_request,
-				    .layer_masks = &layer_masks,
-				    .layer_masks_size = ARRAY_SIZE(layer_masks),
+				    .masks = &layer_masks,
 			    });
 	return -EACCES;
 }
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 0a5b0c76b3f7..8c40f319f4d3 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -612,22 +612,23 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
 	return NULL;
 }
 
-/*
- * @layer_masks is read and may be updated according to the access request and
- * the matching rule.
- * @masks_array_size must be equal to ARRAY_SIZE(*layer_masks).
+/**
+ * landlock_unmask_layers - Cross off access rights granted in @rule in @masks
  *
- * Returns true if the request is allowed (i.e. relevant layer masks for the
- * request are empty).
+ * Updates the set of (per-layer) unfulfilled access rights @masks
+ * so that all the access rights granted in @rule are removed from it
+ * (because they are now fulfilled).
+ *
+ * @rule: A rule that grants a set of access rights for each layer
+ * @masks: A matrix of unfulfilled access rights for each layer
+ *
+ * Returns true if the request is allowed (i.e. the access rights granted all
+ * remaining unfulfilled access rights and masks has no leftover set bits).
  */
 bool landlock_unmask_layers(const struct landlock_rule *const rule,
-			    const access_mask_t access_request,
-			    layer_mask_t (*const layer_masks)[],
-			    const size_t masks_array_size)
+			    struct layer_access_masks *masks)
 {
-	size_t layer_level;
-
-	if (!access_request || !layer_masks)
+	if (!masks)
 		return true;
 	if (!rule)
 		return false;
@@ -642,28 +643,18 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
 	 * by only one rule, but by the union (binary OR) of multiple rules.
 	 * E.g. /a/b <execute> + /a <read> => /a/b <execute + read>
 	 */
-	for (layer_level = 0; layer_level < rule->num_layers; layer_level++) {
-		const struct landlock_layer *const layer =
-			&rule->layers[layer_level];
-		const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
-		const unsigned long access_req = access_request;
-		unsigned long access_bit;
-		bool is_empty;
+	for (size_t i = 0; i < rule->num_layers; i++) {
+		const struct landlock_layer *const layer = &rule->layers[i];
 
-		/*
-		 * Records in @layer_masks which layer grants access to each requested
-		 * access: bit cleared if the related layer grants access.
-		 */
-		is_empty = true;
-		for_each_set_bit(access_bit, &access_req, masks_array_size) {
-			if (layer->access & BIT_ULL(access_bit))
-				(*layer_masks)[access_bit] &= ~layer_bit;
-			is_empty = is_empty && !(*layer_masks)[access_bit];
-		}
-		if (is_empty)
-			return true;
+		/* Clear the bits where the layer in the rule grants access. */
+		masks->access[layer->level - 1] &= ~layer->access;
 	}
-	return false;
+
+	for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
+		if (masks->access[i])
+			return false;
+	}
+	return true;
 }
 
 typedef access_mask_t
@@ -673,13 +664,12 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
 /**
  * landlock_init_layer_masks - Initialize layer masks from an access request
  *
- * Populates @layer_masks such that for each access right in @access_request,
+ * Populates @masks such that for each access right in @access_request,
  * the bits for all the layers are set where this access right is handled.
  *
  * @domain: The domain that defines the current restrictions.
  * @access_request: The requested access rights to check.
- * @layer_masks: It must contain %LANDLOCK_NUM_ACCESS_FS or
- * %LANDLOCK_NUM_ACCESS_NET elements according to @key_type.
+ * @masks: Layer access masks to populate.
  * @key_type: The key type to switch between access masks of different types.
  *
  * Returns: An access mask where each access right bit is set which is handled
@@ -688,23 +678,20 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
 access_mask_t
 landlock_init_layer_masks(const struct landlock_ruleset *const domain,
 			  const access_mask_t access_request,
-			  layer_mask_t (*const layer_masks)[],
+			  struct layer_access_masks *const masks,
 			  const enum landlock_key_type key_type)
 {
 	access_mask_t handled_accesses = 0;
-	size_t layer_level, num_access;
 	get_access_mask_t *get_access_mask;
 
 	switch (key_type) {
 	case LANDLOCK_KEY_INODE:
 		get_access_mask = landlock_get_fs_access_mask;
-		num_access = LANDLOCK_NUM_ACCESS_FS;
 		break;
 
 #if IS_ENABLED(CONFIG_INET)
 	case LANDLOCK_KEY_NET_PORT:
 		get_access_mask = landlock_get_net_access_mask;
-		num_access = LANDLOCK_NUM_ACCESS_NET;
 		break;
 #endif /* IS_ENABLED(CONFIG_INET) */
 
@@ -713,27 +700,18 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
 		return 0;
 	}
 
-	memset(layer_masks, 0,
-	       array_size(sizeof((*layer_masks)[0]), num_access));
-
 	/* An empty access request can happen because of O_WRONLY | O_RDWR. */
 	if (!access_request)
 		return 0;
 
-	/* Saves all handled accesses per layer. */
-	for (layer_level = 0; layer_level < domain->num_layers; layer_level++) {
-		const unsigned long access_req = access_request;
-		const access_mask_t access_mask =
-			get_access_mask(domain, layer_level);
-		unsigned long access_bit;
+	for (size_t i = 0; i < domain->num_layers; i++) {
+		const access_mask_t handled = get_access_mask(domain, i);
 
-		for_each_set_bit(access_bit, &access_req, num_access) {
-			if (BIT_ULL(access_bit) & access_mask) {
-				(*layer_masks)[access_bit] |=
-					BIT_ULL(layer_level);
-				handled_accesses |= BIT_ULL(access_bit);
-			}
-		}
+		masks->access[i] = access_request & handled;
+		handled_accesses |= masks->access[i];
 	}
+	for (size_t i = domain->num_layers; i < ARRAY_SIZE(masks->access); i++)
+		masks->access[i] = 0;
+
 	return handled_accesses;
 }
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 1a78cba662b2..1ceb5fd674c9 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -301,15 +301,28 @@ landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
 	return ruleset->access_masks[layer_level].scope;
 }
 
+/**
+ * struct layer_access_masks - A boolean matrix of layers and access rights
+ *
+ * This has a bit for each combination of layer numbers and access rights.
+ * During access checks, it is used to represent the access rights for each
+ * layer which still need to be fulfilled.  When all bits are 0, the access
+ * request is considered to be fulfilled.
+ */
+struct layer_access_masks {
+	/**
+	 * @access: The unfulfilled access rights for each layer.
+	 */
+	access_mask_t access[LANDLOCK_MAX_NUM_LAYERS];
+};
+
 bool landlock_unmask_layers(const struct landlock_rule *const rule,
-			    const access_mask_t access_request,
-			    layer_mask_t (*const layer_masks)[],
-			    const size_t masks_array_size);
+			    struct layer_access_masks *masks);
 
 access_mask_t
 landlock_init_layer_masks(const struct landlock_ruleset *const domain,
 			  const access_mask_t access_request,
-			  layer_mask_t (*const layer_masks)[],
+			  struct layer_access_masks *masks,
 			  const enum landlock_key_type key_type);
 
 #endif /* _SECURITY_LANDLOCK_RULESET_H */
-- 
2.52.0

Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure

[Randy Dunlap]

The first line here is confusing: "in @rule in @masks"
Maybe:

On 1/25/26 11:58 AM, Günther Noack wrote:
> +/**
> + * landlock_unmask_layers - Cross off access rights granted in @rule in @masks

                             - Update (or Remove) access rights in @masks that are
                               granted in @rules

?

>   *
> - * Returns true if the request is allowed (i.e. relevant layer masks for the
> - * request are empty).
> + * Updates the set of (per-layer) unfulfilled access rights @masks
> + * so that all the access rights granted in @rule are removed from it
> + * (because they are now fulfilled).
> + *
> + * @rule: A rule that grants a set of access rights for each layer
> + * @masks: A matrix of unfulfilled access rights for each layer
> + *
> + * Returns true if the request is allowed (i.e. the access rights granted all
> + * remaining unfulfilled access rights and masks has no leftover set bits).
>   */

-- 
~Randy

Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure

[Günther Noack]

On Sun, Jan 25, 2026 at 02:02:50PM -0800, Randy Dunlap wrote:
> The first line here is confusing: "in @rule in @masks"
> Maybe:
> 
> On 1/25/26 11:58 AM, Günther Noack wrote:
> > +/**
> > + * landlock_unmask_layers - Cross off access rights granted in @rule in @masks
> 
>                              - Update (or Remove) access rights in @masks that are
>                                granted in @rules
> 
> ?

Thanks, that is a better wording indeed.

Will be included in next patch set version,
I think this is less grammatically ambiguous:

- * landlock_unmask_layers - Cross off access rights granted in @rule in @masks
+ * landlock_unmask_layers - Remove the access rights in @masks
+ *                          which are granted in @rule

–Günther

Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure

[Randy Dunlap]

On 1/26/26 8:52 AM, Günther Noack wrote:
> On Sun, Jan 25, 2026 at 02:02:50PM -0800, Randy Dunlap wrote:
>> The first line here is confusing: "in @rule in @masks"
>> Maybe:
>>
>> On 1/25/26 11:58 AM, Günther Noack wrote:
>>> +/**
>>> + * landlock_unmask_layers - Cross off access rights granted in @rule in @masks
>>
>>                              - Update (or Remove) access rights in @masks that are
>>                                granted in @rules
>>
>> ?
> 
> Thanks, that is a better wording indeed.
> 
> Will be included in next patch set version,
> I think this is less grammatically ambiguous:
> 
> - * landlock_unmask_layers - Cross off access rights granted in @rule in @masks
> + * landlock_unmask_layers - Remove the access rights in @masks
> + *                          which are granted in @rule
> 

Ack, that's good. Thanks.

-- 
~Randy

Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure

[Mickaël Salaün]

On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> The layer masks data structure tracks the requested but unfulfilled
> access rights during an operation's security check.  It stores one bit
> for each combination of access right and layer index.  If the bit is
> set, that access right is not granted (yet) in the given layer and we
> have to traverse the path further upwards to grant it.
> 
> Previously, the layer masks were stored as arrays mapping from access
> right indices to layer_mask_t.  The layer_mask_t value then indicates
> all layers in which the given access right is still (tentatively)
> denied.
> 
> This patch introduces struct layer_access_masks instead: This struct
> contains an array with the access_mask_t of each (tentatively) denied
> access right in that layer.
> 
> The hypothesis of this patch is that this simplifies the code enough
> so that the resulting code will run faster:
> 
> * We can use bitwise operations in multiple places where we previously
>   looped over bits individually with macros.  (Should require less
>   branch speculation and lends itself to better loop unrolling.)
> 
> * Code is ~75 lines smaller.
> 
> Other noteworthy changes:
> 
> * In no_more_access(), call a new helper function may_refer(), which
>   only solves the asymmetric case.  Previously, the code interleaved
>   the checks for the two symmetric cases in RENAME_EXCHANGE.  It feels
>   that the code is clearer when renames without RENAME_EXCHANGE are
>   more obviously the normal case.
> 
> Tradeoffs:
> 
> This change improves performance, at a slight size increase to the
> layer masks data structure.
> 
> At the moment, for the filesystem access rights, the data structure
> has the same size as before, but once we introduce the 17th filesystem
> access right, it will double in size (from 32 to 64 bytes), as

...for all access rights (e.g. even if there is no new network one)

> access_mask_t grows from 16 to 32 bit.  See the link below for
> measurements.
> 
> Link: https://lore.kernel.org/all/20260120.haeCh4li9Vae@digikod.net/
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
>  security/landlock/access.h  |  10 +-
>  security/landlock/audit.c   |  84 +++------
>  security/landlock/audit.h   |   3 +-
>  security/landlock/domain.c  |  45 +++--
>  security/landlock/domain.h  |   4 +-
>  security/landlock/fs.c      | 352 ++++++++++++++++--------------------
>  security/landlock/net.c     |  11 +-
>  security/landlock/ruleset.c |  88 ++++-----
>  security/landlock/ruleset.h |  21 ++-
>  9 files changed, 274 insertions(+), 344 deletions(-)
> 
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 5c0caef9eaf6..1c911fa3555d 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -61,14 +61,14 @@ union access_masks_all {
>  static_assert(sizeof(typeof_member(union access_masks_all, masks)) ==
>  	      sizeof(typeof_member(union access_masks_all, all)));
>  
> -typedef u16 layer_mask_t;
> -
> -/* Makes sure all layers can be checked. */
> -static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
> -
>  /*
>   * Tracks domains responsible of a denied access.  This is required to avoid
>   * storing in each object the full layer_masks[] required by update_request().
> + *
> + * Each nibble represents the layer index of the newest layer which denied a
> + * certain access right.  For file system access rights, the upper four bits are
> + * the index of the layer which denies LANDLOCK_ACCESS_FS_IOCTL_DEV and the
> + * lower nibble represents LANDLOCK_ACCESS_FS_TRUNCATE.
>   */
>  typedef u8 deny_masks_t;
>  
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index e899995f1fd5..979a33f480aa 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -180,38 +180,21 @@ static void test_get_hierarchy(struct kunit *const test)
>  
>  #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
>  
> +/* get_denied_layer - get the youngest layer that denied the access_request */

/* Get the youngest layer that denied the access_request. */

>  static size_t get_denied_layer(const struct landlock_ruleset *const domain,
>  			       access_mask_t *const access_request,
> -			       const layer_mask_t (*const layer_masks)[],
> -			       const size_t layer_masks_size)
> +			       const struct layer_access_masks *masks)
>  {
> -	const unsigned long access_req = *access_request;
> -	unsigned long access_bit;
> -	access_mask_t missing = 0;
> -	long youngest_layer = -1;
> -
> -	for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> -		const layer_mask_t mask = (*layer_masks)[access_bit];
> -		long layer;
> -
> -		if (!mask)
> -			continue;
> -
> -		/* __fls(1) == 0 */
> -		layer = __fls(mask);
> -		if (layer > youngest_layer) {
> -			youngest_layer = layer;
> -			missing = BIT(access_bit);
> -		} else if (layer == youngest_layer) {
> -			missing |= BIT(access_bit);
> +	for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {

size_t i

> +		if (masks->access[i] & *access_request) {
> +			*access_request &= masks->access[i];
> +			return i;
>  		}
>  	}
>  
> -	*access_request = missing;
> -	if (youngest_layer == -1)
> -		return domain->num_layers - 1;
> -
> -	return youngest_layer;
> +	/* Not found - fall back to default values */
> +	*access_request = 0;
> +	return domain->num_layers - 1;
>  }
>  
>  #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> @@ -221,50 +204,39 @@ static void test_get_denied_layer(struct kunit *const test)
>  	const struct landlock_ruleset dom = {
>  		.num_layers = 5,
>  	};
> -	const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT(0),
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT(1),
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = BIT(1) | BIT(0),
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = BIT(2),
> +	const struct layer_access_masks masks = {
> +		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> +			     LANDLOCK_ACCESS_FS_READ_DIR,
> +		.access[1] = LANDLOCK_ACCESS_FS_READ_FILE |
> +			     LANDLOCK_ACCESS_FS_READ_DIR,
> +		.access[2] = LANDLOCK_ACCESS_FS_REMOVE_DIR,
>  	};
>  	access_mask_t access;
>  
>  	access = LANDLOCK_ACCESS_FS_EXECUTE;
> -	KUNIT_EXPECT_EQ(test, 0,
> -			get_denied_layer(&dom, &access, &layer_masks,
> -					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, 0, get_denied_layer(&dom, &access, &masks));
>  	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_EXECUTE);
>  
>  	access = LANDLOCK_ACCESS_FS_READ_FILE;
> -	KUNIT_EXPECT_EQ(test, 1,
> -			get_denied_layer(&dom, &access, &layer_masks,
> -					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
>  	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_FILE);
>  
>  	access = LANDLOCK_ACCESS_FS_READ_DIR;
> -	KUNIT_EXPECT_EQ(test, 1,
> -			get_denied_layer(&dom, &access, &layer_masks,
> -					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
>  	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
>  
>  	access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR;
> -	KUNIT_EXPECT_EQ(test, 1,
> -			get_denied_layer(&dom, &access, &layer_masks,
> -					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
>  	KUNIT_EXPECT_EQ(test, access,
>  			LANDLOCK_ACCESS_FS_READ_FILE |
>  				LANDLOCK_ACCESS_FS_READ_DIR);
>  
>  	access = LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_READ_DIR;
> -	KUNIT_EXPECT_EQ(test, 1,
> -			get_denied_layer(&dom, &access, &layer_masks,
> -					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
>  	KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
>  
>  	access = LANDLOCK_ACCESS_FS_WRITE_FILE;
> -	KUNIT_EXPECT_EQ(test, 4,
> -			get_denied_layer(&dom, &access, &layer_masks,
> -					 sizeof(layer_masks)));
> +	KUNIT_EXPECT_EQ(test, 4, get_denied_layer(&dom, &access, &masks));
>  	KUNIT_EXPECT_EQ(test, access, 0);
>  }
>  
> @@ -361,18 +333,15 @@ static bool is_valid_request(const struct landlock_request *const request)
>  		return false;
>  
>  	if (request->access) {
> -		if (WARN_ON_ONCE(!(!!request->layer_masks ^
> +		if (WARN_ON_ONCE(!(!!request->masks ^
>  				   !!request->all_existing_optional_access)))
>  			return false;
>  	} else {
> -		if (WARN_ON_ONCE(request->layer_masks ||
> +		if (WARN_ON_ONCE(request->masks ||
>  				 request->all_existing_optional_access))
>  			return false;
>  	}
>  
> -	if (WARN_ON_ONCE(!!request->layer_masks ^ !!request->layer_masks_size))
> -		return false;
> -
>  	if (request->deny_masks) {
>  		if (WARN_ON_ONCE(!request->all_existing_optional_access))
>  			return false;
> @@ -405,13 +374,12 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
>  	missing = request->access;
>  	if (missing) {
>  		/* Gets the nearest domain that denies the request. */
> -		if (request->layer_masks) {
> +		if (request->masks) {
>  			youngest_layer = get_denied_layer(
> -				subject->domain, &missing, request->layer_masks,
> -				request->layer_masks_size);
> +				subject->domain, &missing, request->masks);
>  		} else {
>  			youngest_layer = get_layer_from_deny_masks(
> -				&missing, request->all_existing_optional_access,
> +				&missing, _LANDLOCK_ACCESS_FS_OPTIONAL,
>  				request->deny_masks);
>  		}
>  		youngest_denied =
> diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> index 92428b7fc4d8..104472060ef5 100644
> --- a/security/landlock/audit.h
> +++ b/security/landlock/audit.h
> @@ -43,8 +43,7 @@ struct landlock_request {
>  	access_mask_t access;
>  
>  	/* Required fields for requests with layer masks. */
> -	const layer_mask_t (*layer_masks)[];
> -	size_t layer_masks_size;
> +	const struct layer_access_masks *masks;
>  
>  	/* Required fields for requests with deny masks. */
>  	const access_mask_t all_existing_optional_access;
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index a647b68e8d06..5b11ddb22d3a 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -7,6 +7,7 @@
>   * Copyright © 2024-2025 Microsoft Corporation
>   */
>  
> +#include "ruleset.h"
>  #include <kunit/test.h>
>  #include <linux/bitops.h>
>  #include <linux/bits.h>
> @@ -182,32 +183,36 @@ static void test_get_layer_deny_mask(struct kunit *const test)
>  deny_masks_t
>  landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
>  			const access_mask_t optional_access,
> -			const layer_mask_t (*const layer_masks)[],
> -			const size_t layer_masks_size)
> +			const struct layer_access_masks *const masks)
>  {
>  	const unsigned long access_opt = optional_access;
>  	unsigned long access_bit;
> +	access_mask_t all_denied = 0;
>  	deny_masks_t deny_masks = 0;
>  
>  	/* This may require change with new object types. */
> -	WARN_ON_ONCE(access_opt !=
> -		     (optional_access & all_existing_optional_access));
> +	WARN_ON_ONCE(!access_mask_subset(optional_access,
> +					 all_existing_optional_access));
>  
> -	if (WARN_ON_ONCE(!layer_masks))
> +	if (WARN_ON_ONCE(!masks))
>  		return 0;
>  
>  	if (WARN_ON_ONCE(!access_opt))
>  		return 0;
>  
> -	for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
> -		const layer_mask_t mask = (*layer_masks)[access_bit];
> +	for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {

size_t i

> +		const access_mask_t denied = masks->access[i] & optional_access;
> +		const unsigned long newly_denied = denied & ~all_denied;
>  
> -		if (!mask)
> +		if (!newly_denied)
>  			continue;
>  
> -		/* __fls(1) == 0 */
> -		deny_masks |= get_layer_deny_mask(all_existing_optional_access,
> -						  access_bit, __fls(mask));
> +		for_each_set_bit(access_bit, &newly_denied,
> +				 8 * sizeof(access_mask_t)) {
> +			deny_masks |= get_layer_deny_mask(
> +				all_existing_optional_access, access_bit, i);
> +		}
> +		all_denied |= denied;
>  	}
>  	return deny_masks;
>  }
> @@ -216,28 +221,28 @@ landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
>  
>  static void test_landlock_get_deny_masks(struct kunit *const test)
>  {
> -	const layer_mask_t layers1[BITS_PER_TYPE(access_mask_t)] = {
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
> -							  BIT_ULL(9),
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = BIT_ULL(1),
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = BIT_ULL(2) |
> -							    BIT_ULL(0),
> +	const struct layer_access_masks layers1 = {
> +		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> +			     LANDLOCK_ACCESS_FS_IOCTL_DEV,
> +		.access[1] = LANDLOCK_ACCESS_FS_TRUNCATE,
> +		.access[2] = LANDLOCK_ACCESS_FS_IOCTL_DEV,
> +		.access[9] = LANDLOCK_ACCESS_FS_EXECUTE,
>  	};
>  
>  	KUNIT_EXPECT_EQ(test, 0x1,
>  			landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
>  						LANDLOCK_ACCESS_FS_TRUNCATE,
> -						&layers1, ARRAY_SIZE(layers1)));
> +						&layers1));
>  	KUNIT_EXPECT_EQ(test, 0x20,
>  			landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
>  						LANDLOCK_ACCESS_FS_IOCTL_DEV,
> -						&layers1, ARRAY_SIZE(layers1)));
> +						&layers1));
>  	KUNIT_EXPECT_EQ(
>  		test, 0x21,
>  		landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
>  					LANDLOCK_ACCESS_FS_TRUNCATE |
>  						LANDLOCK_ACCESS_FS_IOCTL_DEV,
> -					&layers1, ARRAY_SIZE(layers1)));
> +					&layers1));
>  }
>  
>  #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> index 621f054c9a2b..227066d667f7 100644
> --- a/security/landlock/domain.h
> +++ b/security/landlock/domain.h
> @@ -10,6 +10,7 @@
>  #ifndef _SECURITY_LANDLOCK_DOMAIN_H
>  #define _SECURITY_LANDLOCK_DOMAIN_H
>  
> +#include "ruleset.h"
>  #include <linux/limits.h>
>  #include <linux/mm.h>
>  #include <linux/path.h>
> @@ -122,8 +123,7 @@ struct landlock_hierarchy {
>  deny_masks_t
>  landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
>  			const access_mask_t optional_access,
> -			const layer_mask_t (*const layer_masks)[],
> -			size_t layer_masks_size);
> +			const struct layer_access_masks *const masks);
>  
>  int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy);
>  
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index bf8e37fcc7c0..cef0013c2cf6 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -398,57 +398,55 @@ static const struct access_masks any_fs = {
>  	.fs = ~0,
>  };
>  
> +/*
> + * Returns true iff the child file with the given src_child access rights under
> + * src_parent would result in having the same or fewer access rights if it were
> + * moved under new_parent.
> + */
> +static bool may_refer(const struct layer_access_masks *const src_parent,
> +		      const struct layer_access_masks *const src_child,
> +		      const struct layer_access_masks *const new_parent,
> +		      const bool child_is_dir)
> +{
> +	for (size_t i = 0; i < ARRAY_SIZE(new_parent->access); i++) {
> +		access_mask_t child_access = src_parent->access[i] &
> +					     src_child->access[i];
> +		access_mask_t parent_access = new_parent->access[i];
> +
> +		if (!child_is_dir) {
> +			child_access &= ACCESS_FILE;
> +			parent_access &= ACCESS_FILE;
> +		}
> +
> +		if (!access_mask_subset(child_access, parent_access))
> +			return false;
> +	}
> +	return true;
> +}
> +
>  /*
>   * Check that a destination file hierarchy has more restrictions than a source
>   * file hierarchy.  This is only used for link and rename actions.
>   *
> - * @layer_masks_child2: Optional child masks.
> + * Returns: true if child1 may be moved from parent1 to parent2 without
> + * increasing its access rights.  If child2 is set, an additional condition is
> + * that child2 may be used from parent2 to parent1 without increasing its access
> + * rights.
>   */
> -static bool no_more_access(
> -	const layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
> -	const layer_mask_t (*const layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS],
> -	const bool child1_is_directory,
> -	const layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
> -	const layer_mask_t (*const layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS],
> -	const bool child2_is_directory)
> +static bool no_more_access(const struct layer_access_masks *const parent1,
> +			   const struct layer_access_masks *const child1,
> +			   const bool child1_is_dir,
> +			   const struct layer_access_masks *const parent2,
> +			   const struct layer_access_masks *const child2,
> +			   const bool child2_is_dir)
>  {
> -	unsigned long access_bit;
> +	if (!may_refer(parent1, child1, parent2, child1_is_dir))
> +		return false;
>  
> -	for (access_bit = 0; access_bit < ARRAY_SIZE(*layer_masks_parent2);
> -	     access_bit++) {
> -		/* Ignores accesses that only make sense for directories. */
> -		const bool is_file_access =
> -			!!(BIT_ULL(access_bit) & ACCESS_FILE);
> +	if (!child2)
> +		return true;
>  
> -		if (child1_is_directory || is_file_access) {
> -			/*
> -			 * Checks if the destination restrictions are a
> -			 * superset of the source ones (i.e. inherited access
> -			 * rights without child exceptions):
> -			 * restrictions(parent2) >= restrictions(child1)
> -			 */
> -			if ((((*layer_masks_parent1)[access_bit] &
> -			      (*layer_masks_child1)[access_bit]) |
> -			     (*layer_masks_parent2)[access_bit]) !=
> -			    (*layer_masks_parent2)[access_bit])
> -				return false;
> -		}
> -
> -		if (!layer_masks_child2)
> -			continue;
> -		if (child2_is_directory || is_file_access) {
> -			/*
> -			 * Checks inverted restrictions for RENAME_EXCHANGE:
> -			 * restrictions(parent1) >= restrictions(child2)
> -			 */
> -			if ((((*layer_masks_parent2)[access_bit] &
> -			      (*layer_masks_child2)[access_bit]) |
> -			     (*layer_masks_parent1)[access_bit]) !=
> -			    (*layer_masks_parent1)[access_bit])
> -				return false;
> -		}
> -	}
> -	return true;
> +	return may_refer(parent2, child2, parent1, child2_is_dir);
>  }
>  
>  #define NMA_TRUE(...) KUNIT_EXPECT_TRUE(test, no_more_access(__VA_ARGS__))
> @@ -458,25 +456,25 @@ static bool no_more_access(
>  
>  static void test_no_more_access(struct kunit *const test)
>  {
> -	const layer_mask_t rx0[LANDLOCK_NUM_ACCESS_FS] = {
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT_ULL(0),
> +	const struct layer_access_masks rx0 = {
> +		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> +			     LANDLOCK_ACCESS_FS_READ_FILE,
>  	};
> -	const layer_mask_t mx0[LANDLOCK_NUM_ACCESS_FS] = {
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_REG)] = BIT_ULL(0),
> +	const struct layer_access_masks mx0 = {
> +		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> +			     LANDLOCK_ACCESS_FS_MAKE_REG,
>  	};
> -	const layer_mask_t x0[LANDLOCK_NUM_ACCESS_FS] = {
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> +	const struct layer_access_masks x0 = {
> +		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
>  	};
> -	const layer_mask_t x1[LANDLOCK_NUM_ACCESS_FS] = {
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(1),
> +	const struct layer_access_masks x1 = {
> +		.access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
>  	};
> -	const layer_mask_t x01[LANDLOCK_NUM_ACCESS_FS] = {
> -		[BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
> -							  BIT_ULL(1),
> +	const struct layer_access_masks x01 = {
> +		.access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> +		.access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
>  	};
> -	const layer_mask_t allows_all[LANDLOCK_NUM_ACCESS_FS] = {};
> +	const struct layer_access_masks allows_all = {};
>  
>  	/* Checks without restriction. */
>  	NMA_TRUE(&x0, &allows_all, false, &allows_all, NULL, false);
> @@ -564,31 +562,30 @@ static void test_no_more_access(struct kunit *const test)
>  #undef NMA_TRUE
>  #undef NMA_FALSE
>  
> -static bool is_layer_masks_allowed(
> -	layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> +static bool is_layer_masks_allowed(const struct layer_access_masks *masks)
>  {
> -	return !memchr_inv(layer_masks, 0, sizeof(*layer_masks));
> +	return !memchr_inv(&masks->access, 0, sizeof(masks->access));
>  }
>  
>  /*
> - * Removes @layer_masks accesses that are not requested.
> + * Removes @masks accesses that are not requested.
>   *
>   * Returns true if the request is allowed, false otherwise.
>   */
> -static bool
> -scope_to_request(const access_mask_t access_request,
> -		 layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> +static bool scope_to_request(const access_mask_t access_request,
> +			     struct layer_access_masks *masks)
>  {
> -	const unsigned long access_req = access_request;
> -	unsigned long access_bit;
> +	bool saw_unfulfilled_access = false;
>  
> -	if (WARN_ON_ONCE(!layer_masks))
> +	if (WARN_ON_ONCE(!masks))
>  		return true;
>  
> -	for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> -		(*layer_masks)[access_bit] = 0;
> -
> -	return is_layer_masks_allowed(layer_masks);
> +	for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
> +		masks->access[i] &= access_request;
> +		if (masks->access[i])

{

> +			saw_unfulfilled_access = true;

break;
}

> +	}
> +	return !saw_unfulfilled_access;
>  }
>  
>  #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST

Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure

[Günther Noack]

On Wed, Jan 28, 2026 at 10:34:02PM +0100, Mickaël Salaün wrote:
> On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> > Tradeoffs:
> > 
> > This change improves performance, at a slight size increase to the
> > layer masks data structure.
> > 
> > At the moment, for the filesystem access rights, the data structure
> > has the same size as before, but once we introduce the 17th filesystem
> > access right, it will double in size (from 32 to 64 bytes), as
> 
> ...for all access rights (e.g. even if there is no new network one)

Added.

> > --- a/security/landlock/audit.c
> > +++ b/security/landlock/audit.c
> > @@ -180,38 +180,21 @@ static void test_get_hierarchy(struct kunit *const test)
> >  
> >  #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> >  
> > +/* get_denied_layer - get the youngest layer that denied the access_request */
> 
> /* Get the youngest layer that denied the access_request. */

OK, done.  I also changed to non-docstring style for the
access_mask_subset() helper.

> 
> >  static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> >  			       access_mask_t *const access_request,
> > -			       const layer_mask_t (*const layer_masks)[],
> > -			       const size_t layer_masks_size)
> > +			       const struct layer_access_masks *masks)
> >  {
> > -	const unsigned long access_req = *access_request;
> > -	unsigned long access_bit;
> > -	access_mask_t missing = 0;
> > -	long youngest_layer = -1;
> > -
> > -	for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> > -		const layer_mask_t mask = (*layer_masks)[access_bit];
> > -		long layer;
> > -
> > -		if (!mask)
> > -			continue;
> > -
> > -		/* __fls(1) == 0 */
> > -		layer = __fls(mask);
> > -		if (layer > youngest_layer) {
> > -			youngest_layer = layer;
> > -			missing = BIT(access_bit);
> > -		} else if (layer == youngest_layer) {
> > -			missing |= BIT(access_bit);
> > +	for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
> 
> size_t i

This is one of the two places where this didn't work.

The loop goes from top to bottom here, and the "i >= 0" check would
always be true for a size_t.

If there is a more idiomatic way to write that loop, I can switch to
it, but would otherwise lean towards keeping it as it is?


> > +		if (masks->access[i] & *access_request) {
> > +			*access_request &= masks->access[i];
> > +			return i;
> >  		}
> >  	}
> >  
> > -	for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
> > -		const layer_mask_t mask = (*layer_masks)[access_bit];
> > +	for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
> 
> size_t i

Ditto, the loop goes from top to bottom here.


> > +		const access_mask_t denied = masks->access[i] & optional_access;
> > +		const unsigned long newly_denied = denied & ~all_denied;
> >  


> > -static bool
> > -scope_to_request(const access_mask_t access_request,
> > -		 layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> > +static bool scope_to_request(const access_mask_t access_request,
> > +			     struct layer_access_masks *masks)
> >  {
> > -	const unsigned long access_req = access_request;
> > -	unsigned long access_bit;
> > +	bool saw_unfulfilled_access = false;
> >  
> > -	if (WARN_ON_ONCE(!layer_masks))
> > +	if (WARN_ON_ONCE(!masks))
> >  		return true;
> >  
> > -	for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> > -		(*layer_masks)[access_bit] = 0;
> > -
> > -	return is_layer_masks_allowed(layer_masks);
> > +	for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
> > +		masks->access[i] &= access_request;
> > +		if (masks->access[i])
> 
> {
> 
> > +			saw_unfulfilled_access = true;
> 
> break;
> }

Two lines above, this loop mutates masks->access[...]:

  masks->access[i] &= access_request

If we break the loop early, we would not actually scope it down to the
request entirely?  Is this safe?

> > +	}
> > +	return !saw_unfulfilled_access;
> >  }

–Günther

Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure

[Mickaël Salaün]

On Thu, Jan 29, 2026 at 08:56:37AM +0100, Günther Noack wrote:
> On Wed, Jan 28, 2026 at 10:34:02PM +0100, Mickaël Salaün wrote:
> > On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> > > Tradeoffs:
> > > 
> > > This change improves performance, at a slight size increase to the
> > > layer masks data structure.
> > > 
> > > At the moment, for the filesystem access rights, the data structure
> > > has the same size as before, but once we introduce the 17th filesystem
> > > access right, it will double in size (from 32 to 64 bytes), as
> > 
> > ...for all access rights (e.g. even if there is no new network one)
> 
> Added.
> 
> > > --- a/security/landlock/audit.c
> > > +++ b/security/landlock/audit.c
> > > @@ -180,38 +180,21 @@ static void test_get_hierarchy(struct kunit *const test)
> > >  
> > >  #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> > >  
> > > +/* get_denied_layer - get the youngest layer that denied the access_request */
> > 
> > /* Get the youngest layer that denied the access_request. */
> 
> OK, done.  I also changed to non-docstring style for the
> access_mask_subset() helper.
> 
> > 
> > >  static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> > >  			       access_mask_t *const access_request,
> > > -			       const layer_mask_t (*const layer_masks)[],
> > > -			       const size_t layer_masks_size)
> > > +			       const struct layer_access_masks *masks)
> > >  {
> > > -	const unsigned long access_req = *access_request;
> > > -	unsigned long access_bit;
> > > -	access_mask_t missing = 0;
> > > -	long youngest_layer = -1;
> > > -
> > > -	for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> > > -		const layer_mask_t mask = (*layer_masks)[access_bit];
> > > -		long layer;
> > > -
> > > -		if (!mask)
> > > -			continue;
> > > -
> > > -		/* __fls(1) == 0 */
> > > -		layer = __fls(mask);
> > > -		if (layer > youngest_layer) {
> > > -			youngest_layer = layer;
> > > -			missing = BIT(access_bit);
> > > -		} else if (layer == youngest_layer) {
> > > -			missing |= BIT(access_bit);
> > > +	for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
> > 
> > size_t i
> 
> This is one of the two places where this didn't work.
> 
> The loop goes from top to bottom here, and the "i >= 0" check would
> always be true for a size_t.
> 
> If there is a more idiomatic way to write that loop, I can switch to
> it, but would otherwise lean towards keeping it as it is?

Indeed.  We can use ssize_t as in get_hierarchy().

> 
> 
> > > +		if (masks->access[i] & *access_request) {
> > > +			*access_request &= masks->access[i];
> > > +			return i;
> > >  		}
> > >  	}
> > >  
> > > -	for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
> > > -		const layer_mask_t mask = (*layer_masks)[access_bit];
> > > +	for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
> > 
> > size_t i
> 
> Ditto, the loop goes from top to bottom here.
> 
> 
> > > +		const access_mask_t denied = masks->access[i] & optional_access;
> > > +		const unsigned long newly_denied = denied & ~all_denied;
> > >  
> 
> 
> > > -static bool
> > > -scope_to_request(const access_mask_t access_request,
> > > -		 layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> > > +static bool scope_to_request(const access_mask_t access_request,
> > > +			     struct layer_access_masks *masks)
> > >  {
> > > -	const unsigned long access_req = access_request;
> > > -	unsigned long access_bit;
> > > +	bool saw_unfulfilled_access = false;
> > >  
> > > -	if (WARN_ON_ONCE(!layer_masks))
> > > +	if (WARN_ON_ONCE(!masks))
> > >  		return true;
> > >  
> > > -	for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> > > -		(*layer_masks)[access_bit] = 0;
> > > -
> > > -	return is_layer_masks_allowed(layer_masks);
> > > +	for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
> > > +		masks->access[i] &= access_request;
> > > +		if (masks->access[i])
> > 
> > {
> > 
> > > +			saw_unfulfilled_access = true;
> > 
> > break;
> > }
> 
> Two lines above, this loop mutates masks->access[...]:
> 
>   masks->access[i] &= access_request
> 
> If we break the loop early, we would not actually scope it down to the
> request entirely?  Is this safe?

You're right, don't add this break.  BTW, would a test catch it?

> 
> > > +	}
> > > +	return !saw_unfulfilled_access;
> > >  }
> 
> –Günther
> 

Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure

[Günther Noack]

On Thu, Jan 29, 2026 at 05:54:01PM +0100, Mickaël Salaün wrote:
> On Thu, Jan 29, 2026 at 08:56:37AM +0100, Günther Noack wrote:
> > On Wed, Jan 28, 2026 at 10:34:02PM +0100, Mickaël Salaün wrote:
> > > > +	for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
> > > 
> > > size_t i
> > 
> > This is one of the two places where this didn't work.
> > 
> > The loop goes from top to bottom here, and the "i >= 0" check would
> > always be true for a size_t.
> > 
> > If there is a more idiomatic way to write that loop, I can switch to
> > it, but would otherwise lean towards keeping it as it is?
> 
> Indeed.  We can use ssize_t as in get_hierarchy().

Good point, done.


> > > > -static bool
> > > > -scope_to_request(const access_mask_t access_request,
> > > > -		 layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> > > > +static bool scope_to_request(const access_mask_t access_request,
> > > > +			     struct layer_access_masks *masks)
> > > >  {
> > > > -	const unsigned long access_req = access_request;
> > > > -	unsigned long access_bit;
> > > > +	bool saw_unfulfilled_access = false;
> > > >  
> > > > -	if (WARN_ON_ONCE(!layer_masks))
> > > > +	if (WARN_ON_ONCE(!masks))
> > > >  		return true;
> > > >  
> > > > -	for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> > > > -		(*layer_masks)[access_bit] = 0;
> > > > -
> > > > -	return is_layer_masks_allowed(layer_masks);
> > > > +	for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
> > > > +		masks->access[i] &= access_request;
> > > > +		if (masks->access[i])
> > > 
> > > {
> > > 
> > > > +			saw_unfulfilled_access = true;
> > > 
> > > break;
> > > }
> > 
> > Two lines above, this loop mutates masks->access[...]:
> > 
> >   masks->access[i] &= access_request
> > 
> > If we break the loop early, we would not actually scope it down to the
> > request entirely?  Is this safe?
> 
> You're right, don't add this break.  BTW, would a test catch it?

Yes, the existing tests already catch that; this happens when we break early:

[08:53:12] ================= landlock_fs (7 subtests) =================
[08:53:12] [PASSED] test_no_more_access
[08:53:12] [PASSED] test_scope_to_request_with_exec_none
[08:53:12] # test_scope_to_request_with_exec_some: EXPECTATION FAILED at security/landlock/fs.c:616
[08:53:12] Expected 0 == masks.access[1], but
[08:53:12]     masks.access[1] == 2 (0x2)
[08:53:12] [FAILED] test_scope_to_request_with_exec_some
[08:53:12] [PASSED] test_scope_to_request_without_access
[08:53:12] [PASSED] test_is_eacces_with_none
[08:53:12] [PASSED] test_is_eacces_with_refer
[08:53:12] [PASSED] test_is_eacces_with_write
[08:53:12]     # module: landlock
[08:53:12] # landlock_fs: pass:6 fail:1 skip:0 total:7
[08:53:12] # Totals: pass:6 fail:1 skip:0 total:7

Good coverage!

–Günther

Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure

[Mickaël Salaün]

On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> The layer masks data structure tracks the requested but unfulfilled
> access rights during an operation's security check.  It stores one bit
> for each combination of access right and layer index.  If the bit is
> set, that access right is not granted (yet) in the given layer and we
> have to traverse the path further upwards to grant it.
> 
> Previously, the layer masks were stored as arrays mapping from access
> right indices to layer_mask_t.  The layer_mask_t value then indicates
> all layers in which the given access right is still (tentatively)
> denied.
> 
> This patch introduces struct layer_access_masks instead: This struct
> contains an array with the access_mask_t of each (tentatively) denied
> access right in that layer.
> 
> The hypothesis of this patch is that this simplifies the code enough
> so that the resulting code will run faster:
> 
> * We can use bitwise operations in multiple places where we previously
>   looped over bits individually with macros.  (Should require less
>   branch speculation and lends itself to better loop unrolling.)
> 
> * Code is ~75 lines smaller.
> 
> Other noteworthy changes:
> 
> * In no_more_access(), call a new helper function may_refer(), which
>   only solves the asymmetric case.  Previously, the code interleaved
>   the checks for the two symmetric cases in RENAME_EXCHANGE.  It feels
>   that the code is clearer when renames without RENAME_EXCHANGE are
>   more obviously the normal case.
> 
> Tradeoffs:
> 
> This change improves performance, at a slight size increase to the
> layer masks data structure.
> 
> At the moment, for the filesystem access rights, the data structure
> has the same size as before, but once we introduce the 17th filesystem
> access right, it will double in size (from 32 to 64 bytes), as
> access_mask_t grows from 16 to 32 bit.  See the link below for
> measurements.
> 
> Link: https://lore.kernel.org/all/20260120.haeCh4li9Vae@digikod.net/

When adding extra links, please add a [1] reference at the end and use
this reference in the commit message.

> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
>  security/landlock/access.h  |  10 +-
>  security/landlock/audit.c   |  84 +++------
>  security/landlock/audit.h   |   3 +-
>  security/landlock/domain.c  |  45 +++--
>  security/landlock/domain.h  |   4 +-
>  security/landlock/fs.c      | 352 ++++++++++++++++--------------------
>  security/landlock/net.c     |  11 +-
>  security/landlock/ruleset.c |  88 ++++-----
>  security/landlock/ruleset.h |  21 ++-
>  9 files changed, 274 insertions(+), 344 deletions(-)

Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure

[Günther Noack]

On Thu, Jan 29, 2026 at 09:28:08PM +0100, Mickaël Salaün wrote:
> On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> > At the moment, for the filesystem access rights, the data structure
> > has the same size as before, but once we introduce the 17th filesystem
> > access right, it will double in size (from 32 to 64 bytes), as
> > access_mask_t grows from 16 to 32 bit.  See the link below for
> > measurements.
> > 
> > Link: https://lore.kernel.org/all/20260120.haeCh4li9Vae@digikod.net/
> 
> When adding extra links, please add a [1] reference at the end and use
> this reference in the commit message.

Ack, done.

–Günther

Re: [PATCH v2 0/3] landlock: Refactor layer masks

[Mickaël Salaün]

On Sun, Jan 25, 2026 at 08:58:50PM +0100, Günther Noack wrote:
> Hello!
> 
> This patch set "transposes" the layer masks matrix, which was
> previously modeled as a access-max-sized array of layer masks, and
> changes it to be a layer-max-sized array of access masks instead.
> (It is a pure refactoring, there are no user-visible changes.)
> 
> This unlocks a few code simplifications and in multiple places it
> removes the need for loops and branches that deal with individual
> bits.  Instead, the changed data structure now lends itself for more
> bitwise operations.  The underlying hypothesis for me was that by
> using more bitwise operations and fewer branches, we would get an
> overall speedup even when the data structure size increases slightly
> in some cases.
> 
> Tentative results with and without this patch set show that the
> hypothesis likely holds true.  The benchmark I used exercises a "worst
> case" scenario that attempts to be bottlenecked on the affected code:
> constructs a large number of nested directories, with one "path
> beneath" rule each and then tries to open the innermost directory many
> times.  The benchmark is intentionally unrealistic to amplify the
> amount of time used for the path walk logic and forces Landlock to
> walk the full path (eventually failing the open syscall).  (I'll send
> the benchmark program in a reply to this mail for full transparency.)
> 
> Measured with the benchmark program, the patch set results in a
> speedup of about -10%.  The benchmark results are only tentative and
> have been produced in Qemu:
> 
> With the patch, the benchmark runs in 6046 clocks (measured with
> times(3)):
> 
> *** Benchmark ***
> 10000 dirs, 100000 iterations, with landlock
> *** Benchmark concluded ***
> System: 6046 clocks
> User  : 1 clocks
> Clocks per second: 1000000
> 
> Without the patch, we get 6713 clocks, which is 11% more
> 
> *** Benchmark ***
> 10000 dirs, 100000 iterations, with landlock
> *** Benchmark concluded ***
> System: 6713 clocks
> User  : 0 clocks
> Clocks per second: 1000000
> 
> The base revision used for benchmarking was commit 7a51784da76d
> ("tools/sched_ext: update scx_show_state.py for scx_aborting change")
> 
> In real-life scenarios, the speed improvement from this patch set will
> be less pronounced than in the artificial benchmark, as people do not
> usually stack directories that deeply and attach so many rules to
> them, and the EACCES error should also be the exception rather than
> the norm.
> 
> I am looking forward to your feedback.
> 
> P.S.: I am open to suggestions on what the "layer masks" variables
> should be called, because the name "layer masks" might be less
> appropriate after this change.  I have not fixed up the name
> everywhere because fixing up the code took priority for now.

Could you please clarify your thoughts and explain why this name might
not be appropriate anymore?  Any list of name proposals?

If we rename the variables, this should be done in the same refactoring
patch.

> 
> ---
> Changes since previous versions:
> 
> V2: (This patch set)
> 
> * Remove the refactoring around the deny_mask_t type,
>   it is better to send that as a separate patch (mic review)

Feel free to include the new dedicated patch in this series.


> * Added the benchmark program to the selftests
> * Fix unused variable report for "access_dom":
>   https://lore.kernel.org/all/202601200900.wonk9M0m-lkp@intel.com/
> * Use size_t and ARRAY_SIZE to loop over the layers (mic review)
> * Documentation
>   * Fixing up and adding back documentaiton (mic review)
>   * Documented landlock_unmask_layers()
>   * Fixed up kernel docs in a place where it was improperly updated
>     (Spotted by Randy Dunlap
>     https://lore.kernel.org/all/20260123025121.3713403-1-rdunlap@infradead.org/)
> * Minor
>   * Const, some newlines (mic review)
> 
> V1: (Initial version)
> 
> https://lore.kernel.org/all/20251230103917.10549-3-gnoack3000@gmail.com/
> 
> Günther Noack (3):
>   selftests/landlock: Add filesystem access benchmark
>   landlock: access_mask_subset() helper
>   landlock: transpose the layer masks data structure
> 
>  security/landlock/access.h                  |  16 +-
>  security/landlock/audit.c                   |  84 ++---
>  security/landlock/audit.h                   |   3 +-
>  security/landlock/domain.c                  |  45 +--
>  security/landlock/domain.h                  |   4 +-
>  security/landlock/fs.c                      | 354 +++++++++-----------
>  security/landlock/net.c                     |  11 +-
>  security/landlock/ruleset.c                 |  88 ++---
>  security/landlock/ruleset.h                 |  21 +-
>  tools/testing/selftests/landlock/.gitignore |   1 +
>  tools/testing/selftests/landlock/Makefile   |   1 +
>  tools/testing/selftests/landlock/fs_bench.c | 161 +++++++++
>  12 files changed, 444 insertions(+), 345 deletions(-)
>  create mode 100644 tools/testing/selftests/landlock/fs_bench.c
> 
> -- 
> 2.52.0
> 
> 

Re: [PATCH v2 0/3] landlock: Refactor layer masks

[Günther Noack]

Hello!

On Wed, Jan 28, 2026 at 10:31:07PM +0100, Mickaël Salaün wrote:
> On Sun, Jan 25, 2026 at 08:58:50PM +0100, Günther Noack wrote:
> > P.S.: I am open to suggestions on what the "layer masks" variables
> > should be called, because the name "layer masks" might be less
> > appropriate after this change.  I have not fixed up the name
> > everywhere because fixing up the code took priority for now.
> 
> Could you please clarify your thoughts and explain why this name might
> not be appropriate anymore?  Any list of name proposals?
> 
> If we rename the variables, this should be done in the same refactoring
> patch.

When this was an array of layer_mask_t, the name layer_masks was a
description of that underlying data type.  Now that we have removed
the layer_mask_t datatype, it is not as obviously true any more.

When trying to name these variables after the "role" that they have in
their declaration context, I think of them as "unfulfilled per-layer
access requests", but that strikes me as a bit long.

For the upcoming patch set, I'm leaning towards naming these variables
just "masks", to keep it short.


> > Changes since previous versions:
> > 
> > V2: (This patch set)
> > 
> > * Remove the refactoring around the deny_mask_t type,
> >   it is better to send that as a separate patch (mic review)
> 
> Feel free to include the new dedicated patch in this series.

I'm afraid that this one did not get any further than what it already
was, and I'll have to leave it out for now.  But I have it on my TODO
list.

–Günther

Re: [PATCH v2 0/3] landlock: Refactor layer masks

[Günther Noack]

On Fri, Feb 06, 2026 at 08:32:06AM +0100, Günther Noack wrote:
> On Wed, Jan 28, 2026 at 10:31:07PM +0100, Mickaël Salaün wrote:
> > On Sun, Jan 25, 2026 at 08:58:50PM +0100, Günther Noack wrote:
> > > P.S.: I am open to suggestions on what the "layer masks" variables
> > > should be called, because the name "layer masks" might be less
> > > appropriate after this change.  I have not fixed up the name
> > > everywhere because fixing up the code took priority for now.
> > 
> > Could you please clarify your thoughts and explain why this name might
> > not be appropriate anymore?  Any list of name proposals?
> > 
> > If we rename the variables, this should be done in the same refactoring
> > patch.
> 
> When this was an array of layer_mask_t, the name layer_masks was a
> description of that underlying data type.  Now that we have removed
> the layer_mask_t datatype, it is not as obviously true any more.
> 
> When trying to name these variables after the "role" that they have in
> their declaration context, I think of them as "unfulfilled per-layer
> access requests", but that strikes me as a bit long.
> 
> For the upcoming patch set, I'm leaning towards naming these variables
> just "masks", to keep it short.

OK, staring at the code a bit longer, I realize that since the type is
now named "struct layer_access_masks", "layer_masks" is actually still
a reasonable shorthand.  I have abbreviated that to "masks" in some
places where it is anyway clear from the context that those are the
layer access masks, but left it as "layer_masks" in places where we
also use other access masks, for disambiguation.

–Günther

Re: [PATCH v2 0/3] landlock: Refactor layer masks

[Mickaël Salaün]

On Fri, Feb 06, 2026 at 08:49:39AM +0100, Günther Noack wrote:
> On Fri, Feb 06, 2026 at 08:32:06AM +0100, Günther Noack wrote:
> > On Wed, Jan 28, 2026 at 10:31:07PM +0100, Mickaël Salaün wrote:
> > > On Sun, Jan 25, 2026 at 08:58:50PM +0100, Günther Noack wrote:
> > > > P.S.: I am open to suggestions on what the "layer masks" variables
> > > > should be called, because the name "layer masks" might be less
> > > > appropriate after this change.  I have not fixed up the name
> > > > everywhere because fixing up the code took priority for now.
> > > 
> > > Could you please clarify your thoughts and explain why this name might
> > > not be appropriate anymore?  Any list of name proposals?
> > > 
> > > If we rename the variables, this should be done in the same refactoring
> > > patch.
> > 
> > When this was an array of layer_mask_t, the name layer_masks was a
> > description of that underlying data type.  Now that we have removed
> > the layer_mask_t datatype, it is not as obviously true any more.
> > 
> > When trying to name these variables after the "role" that they have in
> > their declaration context, I think of them as "unfulfilled per-layer
> > access requests", but that strikes me as a bit long.
> > 
> > For the upcoming patch set, I'm leaning towards naming these variables
> > just "masks", to keep it short.
> 
> OK, staring at the code a bit longer, I realize that since the type is
> now named "struct layer_access_masks", "layer_masks" is actually still
> a reasonable shorthand.  I have abbreviated that to "masks" in some
> places where it is anyway clear from the context that those are the
> layer access masks, but left it as "layer_masks" in places where we
> also use other access masks, for disambiguation.

Looks good