* [RFC PATCH 0/2] landlock: Refactor layer masks
@ 2025-12-30 10:39 Günther Noack
2025-12-30 10:39 ` [RFC PATCH 1/2] landlock: access_mask_subset() helper Günther Noack
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Günther Noack @ 2025-12-30 10:39 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, 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 -8.3%. The benchmark results are only tentative and have
been produced in Qemu:
With the patch, the benchmark runs in 5932 clocks (measured with
times(3)):
*** Benchmark ***
10000 dirs, 100000 iterations, with landlock
*** Benchmark concluded ***
System: 5932 clocks
User : 1 clocks
Clocks per second: 1000000
Without the patch, we get 6472 clocks, which is 9.1% more.
*** Benchmark ***
10000 dirs, 100000 iterations, with landlock
*** Benchmark concluded ***
System: 6472 clocks
User : 1 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.
Günther Noack (2):
landlock: access_mask_subset() helper
landlock: transpose the layer masks data structure
security/landlock/access.h | 10 +-
security/landlock/audit.c | 155 ++++++----------
security/landlock/audit.h | 3 +-
security/landlock/domain.c | 120 +++---------
security/landlock/domain.h | 6 +-
security/landlock/fs.c | 361 +++++++++++++++++-------------------
security/landlock/net.c | 10 +-
security/landlock/ruleset.c | 78 ++------
security/landlock/ruleset.h | 18 +-
9 files changed, 300 insertions(+), 461 deletions(-)
--
2.52.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC PATCH 1/2] landlock: access_mask_subset() helper
2025-12-30 10:39 [RFC PATCH 0/2] landlock: Refactor layer masks Günther Noack
@ 2025-12-30 10:39 ` Günther Noack
2025-12-30 10:39 ` [RFC PATCH 2/2] landlock: transpose the layer masks data structure Günther Noack
2025-12-30 10:48 ` [RFC PATCH 0/2] landlock: Refactor layer masks Günther Noack
2 siblings, 0 replies; 4+ messages in thread
From: Günther Noack @ 2025-12-30 10:39 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, 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/fs.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index fe794875ad461..b4ce03bef4b8e 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -398,6 +398,15 @@ static const struct access_masks any_fs = {
.fs = ~0,
};
+/*
+ * Returns true iff a has a subset of the bits of b.
+ * It helps readability and gets inlined.
+ */
+static bool access_mask_subset(access_mask_t a, access_mask_t b)
+{
+ return (a | b) == b;
+}
+
/*
* Check that a destination file hierarchy has more restrictions than a source
* file hierarchy. This is only used for link and rename actions.
@@ -1696,7 +1705,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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [RFC PATCH 2/2] landlock: transpose the layer masks data structure
2025-12-30 10:39 [RFC PATCH 0/2] landlock: Refactor layer masks Günther Noack
2025-12-30 10:39 ` [RFC PATCH 1/2] landlock: access_mask_subset() helper Günther Noack
@ 2025-12-30 10:39 ` Günther Noack
2025-12-30 10:48 ` [RFC PATCH 0/2] landlock: Refactor layer masks Günther Noack
2 siblings, 0 replies; 4+ messages in thread
From: Günther Noack @ 2025-12-30 10:39 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Günther Noack
The layer masks data structure tracks the requested but unfulfilled
access rights during an operations 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)
* Code is ~160 lines smaller.
Other noteworthy changes:
* Clarify deny_mask_t and the code assembling it.
* Document what that value looks like
* Make writing and reading functions specific to file system rules.
(It only worked for FS rules before as well, but going all the way
simplifies the code logic more.)
* 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.
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
security/landlock/access.h | 10 +-
security/landlock/audit.c | 155 ++++++----------
security/landlock/audit.h | 3 +-
security/landlock/domain.c | 120 +++----------
security/landlock/domain.h | 6 +-
security/landlock/fs.c | 350 ++++++++++++++++--------------------
security/landlock/net.c | 10 +-
security/landlock/ruleset.c | 78 +++-----
security/landlock/ruleset.h | 18 +-
9 files changed, 290 insertions(+), 460 deletions(-)
diff --git a/security/landlock/access.h b/security/landlock/access.h
index 7961c6630a2d7..aa0efa36a37db 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 c52d079cdb77b..650bd7f5cb6be 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -182,36 +182,18 @@ static void test_get_hierarchy(struct kunit *const test)
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 access_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 = LANDLOCK_MAX_NUM_LAYERS - 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,94 +203,82 @@ 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);
}
#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
-static size_t
-get_layer_from_deny_masks(access_mask_t *const access_request,
- const access_mask_t all_existing_optional_access,
- const deny_masks_t deny_masks)
+/*
+ * get_layer_from_fs_deny_masks - get the layer which denied the access request
+ *
+ * As a side effect, stores the denied access rights from that layer(!) in
+ * *access_request.
+ */
+static size_t get_layer_from_fs_deny_masks(access_mask_t *const access_request,
+ const deny_masks_t deny_masks)
{
- const unsigned long access_opt = all_existing_optional_access;
- const unsigned long access_req = *access_request;
- access_mask_t missing = 0;
+ const access_mask_t access_req = *access_request;
size_t youngest_layer = 0;
- size_t access_index = 0;
- unsigned long access_bit;
+ access_mask_t missing = 0;
- /* This will require change with new object types. */
- WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
+ WARN_ON_ONCE((access_req | _LANDLOCK_ACCESS_FS_OPTIONAL) !=
+ _LANDLOCK_ACCESS_FS_OPTIONAL);
- for_each_set_bit(access_bit, &access_opt,
- BITS_PER_TYPE(access_mask_t)) {
- if (access_req & BIT(access_bit)) {
- const size_t layer =
- (deny_masks >> (access_index * 4)) &
- (LANDLOCK_MAX_NUM_LAYERS - 1);
+ if (access_req & LANDLOCK_ACCESS_FS_TRUNCATE) {
+ size_t layer = deny_masks & 0x0f;
- if (layer > youngest_layer) {
- youngest_layer = layer;
- missing = BIT(access_bit);
- } else if (layer == youngest_layer) {
- missing |= BIT(access_bit);
- }
- }
- access_index++;
+ missing |= LANDLOCK_ACCESS_FS_TRUNCATE;
+ youngest_layer = max(youngest_layer, layer);
}
+ if (access_req & LANDLOCK_ACCESS_FS_IOCTL_DEV) {
+ size_t layer = (deny_masks & 0xf0) >> 4;
+ if (layer > youngest_layer)
+ missing = 0;
+
+ missing |= LANDLOCK_ACCESS_FS_IOCTL_DEV;
+ youngest_layer = max(youngest_layer, layer);
+ }
*access_request = missing;
return youngest_layer;
}
#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
-static void test_get_layer_from_deny_masks(struct kunit *const test)
+static void test_get_layer_from_fs_deny_masks(struct kunit *const test)
{
deny_masks_t deny_mask;
access_mask_t access;
@@ -318,16 +288,12 @@ static void test_get_layer_from_deny_masks(struct kunit *const test)
access = LANDLOCK_ACCESS_FS_TRUNCATE;
KUNIT_EXPECT_EQ(test, 0,
- get_layer_from_deny_masks(&access,
- _LANDLOCK_ACCESS_FS_OPTIONAL,
- deny_mask));
+ get_layer_from_fs_deny_masks(&access, deny_mask));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
access = LANDLOCK_ACCESS_FS_TRUNCATE | LANDLOCK_ACCESS_FS_IOCTL_DEV;
KUNIT_EXPECT_EQ(test, 2,
- get_layer_from_deny_masks(&access,
- _LANDLOCK_ACCESS_FS_OPTIONAL,
- deny_mask));
+ get_layer_from_fs_deny_masks(&access, deny_mask));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
/* truncate:15 ioctl_dev:15 */
@@ -335,16 +301,12 @@ static void test_get_layer_from_deny_masks(struct kunit *const test)
access = LANDLOCK_ACCESS_FS_TRUNCATE;
KUNIT_EXPECT_EQ(test, 15,
- get_layer_from_deny_masks(&access,
- _LANDLOCK_ACCESS_FS_OPTIONAL,
- deny_mask));
+ get_layer_from_fs_deny_masks(&access, deny_mask));
KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
access = LANDLOCK_ACCESS_FS_TRUNCATE | LANDLOCK_ACCESS_FS_IOCTL_DEV;
KUNIT_EXPECT_EQ(test, 15,
- get_layer_from_deny_masks(&access,
- _LANDLOCK_ACCESS_FS_OPTIONAL,
- deny_mask));
+ get_layer_from_fs_deny_masks(&access, deny_mask));
KUNIT_EXPECT_EQ(test, access,
LANDLOCK_ACCESS_FS_TRUNCATE |
LANDLOCK_ACCESS_FS_IOCTL_DEV);
@@ -361,18 +323,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,14 +364,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,
- request->deny_masks);
+ youngest_layer = get_layer_from_fs_deny_masks(
+ &missing, request->deny_masks);
}
youngest_denied =
get_hierarchy(subject->domain, youngest_layer);
@@ -507,7 +464,7 @@ static struct kunit_case test_cases[] = {
/* clang-format off */
KUNIT_CASE(test_get_hierarchy),
KUNIT_CASE(test_get_denied_layer),
- KUNIT_CASE(test_get_layer_from_deny_masks),
+ KUNIT_CASE(test_get_layer_from_fs_deny_masks),
{}
/* clang-format on */
};
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 92428b7fc4d80..104472060ef5e 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 a647b68e8d060..e8e4ae5d075fe 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -23,6 +23,7 @@
#include "common.h"
#include "domain.h"
#include "id.h"
+#include "limits.h"
#ifdef CONFIG_AUDIT
@@ -133,111 +134,47 @@ int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy)
return 0;
}
-static deny_masks_t
-get_layer_deny_mask(const access_mask_t all_existing_optional_access,
- const unsigned long access_bit, const size_t layer)
-{
- unsigned long access_weight;
-
- /* This may require change with new object types. */
- WARN_ON_ONCE(all_existing_optional_access !=
- _LANDLOCK_ACCESS_FS_OPTIONAL);
-
- if (WARN_ON_ONCE(layer >= LANDLOCK_MAX_NUM_LAYERS))
- return 0;
-
- access_weight = hweight_long(all_existing_optional_access &
- GENMASK(access_bit, 0));
- if (WARN_ON_ONCE(access_weight < 1))
- return 0;
-
- return layer
- << ((access_weight - 1) * HWEIGHT(LANDLOCK_MAX_NUM_LAYERS - 1));
-}
-
-#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
-
-static void test_get_layer_deny_mask(struct kunit *const test)
-{
- const unsigned long truncate = BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE);
- const unsigned long ioctl_dev = BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV);
-
- KUNIT_EXPECT_EQ(test, 0,
- get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
- truncate, 0));
- KUNIT_EXPECT_EQ(test, 0x3,
- get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
- truncate, 3));
-
- KUNIT_EXPECT_EQ(test, 0,
- get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
- ioctl_dev, 0));
- KUNIT_EXPECT_EQ(test, 0xf0,
- get_layer_deny_mask(_LANDLOCK_ACCESS_FS_OPTIONAL,
- ioctl_dev, 15));
-}
-
-#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_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)
+landlock_get_fs_deny_masks(const access_mask_t optional_access,
+ const struct layer_access_masks *layer_masks)
{
- const unsigned long access_opt = optional_access;
- unsigned long access_bit;
- deny_masks_t deny_masks = 0;
+ u8 truncate_layer = 0;
+ u8 ioctl_dev_layer = 0;
- /* This may require change with new object types. */
- WARN_ON_ONCE(access_opt !=
- (optional_access & all_existing_optional_access));
-
- if (WARN_ON_ONCE(!layer_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];
-
- if (!mask)
- continue;
-
- /* __fls(1) == 0 */
- deny_masks |= get_layer_deny_mask(all_existing_optional_access,
- access_bit, __fls(mask));
+ for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
+ if (layer_masks->access[i] & optional_access &
+ LANDLOCK_ACCESS_FS_TRUNCATE)
+ truncate_layer = i;
+ if (layer_masks->access[i] & optional_access &
+ LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ ioctl_dev_layer = i;
}
- return deny_masks;
+ return ((ioctl_dev_layer << 4) & 0xf0) | (truncate_layer & 0x0f);
}
#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
-static void test_landlock_get_deny_masks(struct kunit *const test)
+static void test_landlock_get_fs_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)));
+ landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_TRUNCATE,
+ &layers1));
KUNIT_EXPECT_EQ(test, 0x20,
- landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
- LANDLOCK_ACCESS_FS_IOCTL_DEV,
- &layers1, ARRAY_SIZE(layers1)));
+ landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_IOCTL_DEV,
+ &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)));
+ landlock_get_fs_deny_masks(LANDLOCK_ACCESS_FS_TRUNCATE |
+ LANDLOCK_ACCESS_FS_IOCTL_DEV,
+ &layers1));
}
#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
@@ -246,8 +183,7 @@ static void test_landlock_get_deny_masks(struct kunit *const test)
static struct kunit_case test_cases[] = {
/* clang-format off */
- KUNIT_CASE(test_get_layer_deny_mask),
- KUNIT_CASE(test_landlock_get_deny_masks),
+ KUNIT_CASE(test_landlock_get_fs_deny_masks),
{}
/* clang-format on */
};
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index 7fb70b25f85a1..39600acb63897 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -120,10 +120,8 @@ struct landlock_hierarchy {
#ifdef CONFIG_AUDIT
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);
+landlock_get_fs_deny_masks(const access_mask_t optional_access,
+ const struct layer_access_masks *layer_masks);
int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy);
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index b4ce03bef4b8e..1e765d22d8d49 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -407,57 +407,55 @@ static bool access_mask_subset(access_mask_t a, access_mask_t b)
return (a | b) == b;
}
+/*
+ * 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 (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; 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__))
@@ -467,25 +465,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);
@@ -573,31 +571,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 (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
+ masks->access[i] &= access_request;
+ if (masks->access[i])
+ saw_unfulfilled_access = true;
+ }
+ return !saw_unfulfilled_access;
}
#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
@@ -605,48 +602,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 */
@@ -655,20 +645,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 (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
+ /* LANDLOCK_ACCESS_FS_REFER alone must return -EXDEV. */
+ if (masks->access[i] & access_request &
+ ~LANDLOCK_ACCESS_FS_REFER)
return true;
}
return false;
@@ -681,37 +667,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 */
@@ -761,26 +747,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;
@@ -820,22 +805,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);
}
@@ -890,16 +873,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)
@@ -953,9 +932,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) {
@@ -963,9 +940,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;
}
return allowed_parent1 && allowed_parent2;
}
@@ -978,7 +953,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)
@@ -1053,10 +1028,10 @@ 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;
@@ -1075,9 +1050,8 @@ static bool collect_domain_accesses(
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.
@@ -1165,8 +1139,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)
@@ -1323,7 +1297,8 @@ static void hook_sb_delete(struct super_block *const sb)
* second call to iput() for the same Landlock object. Also
* checks I_NEW because such inode cannot be tied to an object.
*/
- if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_NEW)) {
+ if (inode_state_read(inode) &
+ (I_FREEING | I_WILL_FREE | I_NEW)) {
spin_unlock(&inode->i_lock);
continue;
}
@@ -1641,7 +1616,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 =
@@ -1676,20 +1651,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 (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++)
+ allowed_access &= ~layer_masks.access[i];
}
/*
@@ -1700,9 +1669,8 @@ 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_file(file)->deny_masks =
+ landlock_get_fs_deny_masks(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 1f3915a90a808..2a5456f4f017e 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,
@@ -178,8 +178,9 @@ 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;
@@ -189,8 +190,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 dfcdc19ea2683..d20e28d38e9c9 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -622,49 +622,24 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
* request are empty).
*/
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;
- /*
- * An access is granted if, for each policy layer, at least one rule
- * encountered on the pathwalk grants the requested access,
- * regardless of its position in the layer stack. We must then check
- * the remaining layers for each inode, from the first added layer to
- * the last one. When there is multiple requested accesses, for each
- * policy layer, the full set of requested accesses may not be granted
- * 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 (int i = 0; i < rule->num_layers; i++) {
+ const struct landlock_layer *l = &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;
+ masks->access[l->level - 1] &= ~l->access;
}
- return false;
+
+ for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
+ if (masks->access[i])
+ return false;
+ }
+ return true;
}
typedef access_mask_t
@@ -679,8 +654,7 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
*
* @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
@@ -689,23 +663,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 *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) */
@@ -714,27 +685,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 (int 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 (int i = domain->num_layers; i < LANDLOCK_MAX_NUM_LAYERS; i++)
+ masks->access[i] = 0;
+
return handled_accesses;
}
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 1a78cba662b24..f7b80b18c2a70 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -301,15 +301,25 @@ landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
return ruleset->access_masks[layer_level].scope;
}
+/**
+ * struct layer_accesses - 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_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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH 0/2] landlock: Refactor layer masks
2025-12-30 10:39 [RFC PATCH 0/2] landlock: Refactor layer masks Günther Noack
2025-12-30 10:39 ` [RFC PATCH 1/2] landlock: access_mask_subset() helper Günther Noack
2025-12-30 10:39 ` [RFC PATCH 2/2] landlock: transpose the layer masks data structure Günther Noack
@ 2025-12-30 10:48 ` Günther Noack
2 siblings, 0 replies; 4+ messages in thread
From: Günther Noack @ 2025-12-30 10:48 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze
On Tue, Dec 30, 2025 at 11:39:17AM +0100, Günther Noack wrote:
> 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.)
Please see the benchmark program below.
To compile it, use:
cc -o benchmark_worsecase benchmark_worsecase.c
Source code:
```
#define _GNU_SOURCE
#include <err.h>
#include <fcntl.h>
#include <linux/landlock.h>
#include <stdbool.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>
/* Flags */
bool use_landlock = true;
size_t num_iterations = 100000;
size_t num_subdirs = 10000;
void usage() { puts("Usage: benchmark_worstcase [-no-landlock]"); }
/*
* 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) {
const char *path = "d"; /* directory name */
if (use_landlock) {
int 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);
}
int ruleset_fd = -1;
if (use_landlock) {
if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0)
err(1, "prctl");
struct landlock_ruleset_attr attr = {
.handled_access_fs = 0xffff, /* All FS access rights as of 2025-12 */
};
ruleset_fd = syscall(SYS_landlock_create_ruleset, &attr, sizeof(attr), 0U);
if (ruleset_fd < 0)
err(1, "landlock_create_ruleset");
}
int 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);
int previous = current;
current = openat(current, path, O_PATH);
if (current < 0)
err(1, "open(%s)", path);
close(previous);
}
if (use_landlock) {
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[]) {
for (int i = 1; i < argc; i++) {
if (!strcmp(argv[i], "-no-landlock")) {
use_landlock = false;
} else if (!strcmp(argv[i], "-d")) {
i++;
if (i < argc)
err(1, "expected number of subdirs after -d");
num_subdirs = atoi(argv[i]);
} else if (!strcmp(argv[i], "-n")) {
i++;
if (i < argc)
err(1, "expected number of iterations after -n");
num_iterations = atoi(argv[i]);
} else {
usage();
errx(1, "unknown argument: %s", argv[i]);
}
}
printf("*** Benchmark ***\n");
printf("%zu dirs, %zu iterations, %s landlock\n", num_subdirs,
num_iterations, use_landlock ? "with" : "without");
struct tms start_time;
if (times(&start_time) == -1)
err(1, "times");
int current = build_directory(num_subdirs);
for (int i = 0; i < num_iterations; i++) {
int fd = openat(current, ".", O_DIRECTORY);
if (fd != -1)
errx(1, "openat succeeded, expected error");
}
struct tms end_time;
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: %d\n", CLOCKS_PER_SEC);
close(current);
}
```
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-12-30 10:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-30 10:39 [RFC PATCH 0/2] landlock: Refactor layer masks Günther Noack
2025-12-30 10:39 ` [RFC PATCH 1/2] landlock: access_mask_subset() helper Günther Noack
2025-12-30 10:39 ` [RFC PATCH 2/2] landlock: transpose the layer masks data structure Günther Noack
2025-12-30 10:48 ` [RFC PATCH 0/2] landlock: Refactor layer masks Günther Noack
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).