* [PATCH v3 1/8] landlock: Add a place for flags to layer rules
From: Tingmao Wang @ 2025-10-26 20:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
In-Reply-To: <cover.1761511023.git.m@maowtm.org>
To avoid unnecessarily increasing the size of struct landlock_layer, we
make the layer level a u8 and use the space to store the flags struct.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v2:
- Comment changes, move local variables, simplify if branch
Changes since v1:
- Comment changes
- Rebased to include disconnected directory handling changes on mic/next
and add backing up of collected_rule_flags.
security/landlock/fs.c | 95 ++++++++++++++++++++++++++-----------
security/landlock/net.c | 3 +-
security/landlock/ruleset.c | 7 ++-
security/landlock/ruleset.h | 26 +++++++++-
4 files changed, 99 insertions(+), 32 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 4ed997f4a663..721cd63a4de8 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -757,10 +757,12 @@ static bool is_access_to_paths_allowed(
const struct path *const path,
const access_mask_t access_request_parent1,
layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
+ struct collected_rule_flags *const rule_flags_parent1,
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 collected_rule_flags *const rule_flags_parent2,
struct landlock_request *const log_request_parent2,
struct dentry *const dentry_child2)
{
@@ -775,6 +777,7 @@ static bool is_access_to_paths_allowed(
_layer_masks_parent2_bkp[LANDLOCK_NUM_ACCESS_FS];
layer_mask_t(*layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS] = NULL,
(*layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS] = NULL;
+ struct collected_rule_flags _rule_flag_parent1_bkp, _rule_flag_parent2_bkp;
if (!access_request_parent1 && !access_request_parent2)
return true;
@@ -800,6 +803,8 @@ static bool is_access_to_paths_allowed(
*/
memcpy(&_layer_masks_parent2_bkp, layer_masks_parent2,
sizeof(_layer_masks_parent2_bkp));
+ memcpy(&_rule_flag_parent2_bkp, rule_flags_parent2,
+ sizeof(_rule_flag_parent2_bkp));
allowed_parent2 = is_layer_masks_allowed(layer_masks_parent2);
/*
@@ -826,26 +831,38 @@ static bool is_access_to_paths_allowed(
*/
memcpy(&_layer_masks_parent1_bkp, layer_masks_parent1,
sizeof(_layer_masks_parent1_bkp));
+ memcpy(&_rule_flag_parent1_bkp, rule_flags_parent1,
+ sizeof(_rule_flag_parent1_bkp));
allowed_parent1 = is_layer_masks_allowed(layer_masks_parent1);
is_dom_check_bkp = is_dom_check;
if (unlikely(dentry_child1)) {
+ /*
+ * Get the layer masks for the child dentries for use by domain
+ * check later. The rule_flags for child1 should have been
+ * included in rule_flags_parent1 already (cf.
+ * collect_domain_accesses), and is not relevant for domain check,
+ * so we don't have to pass it to landlock_unmask_layers.
+ */
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));
+ &_layer_masks_child1, ARRAY_SIZE(_layer_masks_child1),
+ NULL);
layer_masks_child1 = &_layer_masks_child1;
child1_is_directory = d_is_dir(dentry_child1);
}
if (unlikely(dentry_child2)) {
+ /* See above comment for why NULL is passed as rule_flags_masks. */
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));
+ &_layer_masks_child2, ARRAY_SIZE(_layer_masks_child2),
+ NULL);
layer_masks_child2 = &_layer_masks_child2;
child2_is_directory = d_is_dir(dentry_child2);
}
@@ -901,16 +918,18 @@ static bool is_access_to_paths_allowed(
NULL :
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, access_masked_parent1,
+ layer_masks_parent1,
+ ARRAY_SIZE(*layer_masks_parent1),
+ rule_flags_parent1);
+ allowed_parent2 =
+ allowed_parent2 ||
+ landlock_unmask_layers(rule, access_masked_parent2,
+ layer_masks_parent2,
+ ARRAY_SIZE(*layer_masks_parent2),
+ rule_flags_parent2);
/* Stops when a rule from each layer grants access. */
if (allowed_parent1 && allowed_parent2) {
@@ -947,10 +966,16 @@ static bool is_access_to_paths_allowed(
memcpy(&_layer_masks_parent1_bkp,
layer_masks_parent1,
sizeof(_layer_masks_parent1_bkp));
+ memcpy(&_rule_flag_parent1_bkp,
+ rule_flags_parent1,
+ sizeof(_rule_flag_parent1_bkp));
if (layer_masks_parent2) {
memcpy(&_layer_masks_parent2_bkp,
layer_masks_parent2,
sizeof(_layer_masks_parent2_bkp));
+ memcpy(&_rule_flag_parent2_bkp,
+ rule_flags_parent2,
+ sizeof(_rule_flag_parent2_bkp));
is_dom_check_bkp = is_dom_check;
}
@@ -999,11 +1024,15 @@ static bool is_access_to_paths_allowed(
*/
memcpy(layer_masks_parent1, &_layer_masks_parent1_bkp,
sizeof(_layer_masks_parent1_bkp));
+ memcpy(rule_flags_parent1, &_rule_flag_parent1_bkp,
+ sizeof(_rule_flag_parent1_bkp));
allowed_parent1 =
is_layer_masks_allowed(&_layer_masks_parent1_bkp);
if (layer_masks_parent2) {
memcpy(layer_masks_parent2, &_layer_masks_parent2_bkp,
sizeof(_layer_masks_parent2_bkp));
+ memcpy(rule_flags_parent2, &_rule_flag_parent2_bkp,
+ sizeof(_rule_flag_parent2_bkp));
allowed_parent2 = is_layer_masks_allowed(
&_layer_masks_parent2_bkp);
@@ -1063,6 +1092,7 @@ static int current_check_access_path(const struct path *const path,
landlock_get_applicable_subject(current_cred(), masks, NULL);
layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {};
struct landlock_request request = {};
+ struct collected_rule_flags rule_flags = {};
if (!subject)
return 0;
@@ -1071,8 +1101,8 @@ static int current_check_access_path(const struct path *const path,
access_request, &layer_masks,
LANDLOCK_KEY_INODE);
if (is_access_to_paths_allowed(subject->domain, path, access_request,
- &layer_masks, &request, NULL, 0, NULL,
- NULL, NULL))
+ &layer_masks, &rule_flags, &request,
+ NULL, 0, NULL, NULL, NULL, NULL))
return 0;
landlock_log_denial(subject, &request);
@@ -1139,7 +1169,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
static bool collect_domain_accesses(
const struct landlock_ruleset *const domain,
const struct path *const mnt_dir, struct dentry *dir,
- layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS])
+ layer_mask_t (*const layer_masks_dom)[LANDLOCK_NUM_ACCESS_FS],
+ struct collected_rule_flags *const rule_flags)
{
access_mask_t access_dom;
bool ret = false;
@@ -1158,9 +1189,9 @@ 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), access_dom, layer_masks_dom,
+ ARRAY_SIZE(*layer_masks_dom), rule_flags)) {
/*
* Before allowing this side of the access request, checks that the
* walk was not in a disconnected directory.
@@ -1269,6 +1300,8 @@ static int current_check_refer_path(struct dentry *const old_dentry,
layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
struct landlock_request request1 = {}, request2 = {};
+ struct collected_rule_flags rule_flags_parent1 = {},
+ rule_flags_parent2 = {};
if (!subject)
return 0;
@@ -1300,10 +1333,10 @@ static int current_check_refer_path(struct dentry *const old_dentry,
subject->domain,
access_request_parent1 | access_request_parent2,
&layer_masks_parent1, LANDLOCK_KEY_INODE);
- if (is_access_to_paths_allowed(subject->domain, new_dir,
- access_request_parent1,
- &layer_masks_parent1, &request1,
- NULL, 0, NULL, NULL, NULL))
+ if (is_access_to_paths_allowed(
+ subject->domain, new_dir, access_request_parent1,
+ &layer_masks_parent1, &rule_flags_parent1,
+ &request1, NULL, 0, NULL, NULL, NULL, NULL))
return 0;
landlock_log_denial(subject, &request1);
@@ -1327,11 +1360,14 @@ static int current_check_refer_path(struct dentry *const old_dentry,
old_dentry->d_parent;
/* new_dir->dentry is equal to new_dentry->d_parent */
- allow_parent1 = collect_domain_accesses(
- subject->domain, &mnt_dir, old_parent, &layer_masks_parent1);
+ allow_parent1 = collect_domain_accesses(subject->domain, &mnt_dir,
+ old_parent,
+ &layer_masks_parent1,
+ &rule_flags_parent1);
allow_parent2 = collect_domain_accesses(subject->domain, &mnt_dir,
new_dir->dentry,
- &layer_masks_parent2);
+ &layer_masks_parent2,
+ &rule_flags_parent2);
if (allow_parent1 && allow_parent2)
return 0;
@@ -1343,8 +1379,9 @@ static int current_check_refer_path(struct dentry *const old_dentry,
*/
if (is_access_to_paths_allowed(
subject->domain, &mnt_dir, access_request_parent1,
- &layer_masks_parent1, &request1, old_dentry,
- access_request_parent2, &layer_masks_parent2, &request2,
+ &layer_masks_parent1, &rule_flags_parent1, &request1,
+ old_dentry, access_request_parent2, &layer_masks_parent2,
+ &rule_flags_parent2, &request2,
exchange ? new_dentry : NULL))
return 0;
@@ -1747,6 +1784,7 @@ static int hook_file_open(struct file *const file)
const struct landlock_cred_security *const subject =
landlock_get_applicable_subject(file->f_cred, any_fs, NULL);
struct landlock_request request = {};
+ struct collected_rule_flags rule_flags = {};
if (!subject)
return 0;
@@ -1773,7 +1811,8 @@ static int hook_file_open(struct file *const file)
landlock_init_layer_masks(subject->domain,
full_access_request, &layer_masks,
LANDLOCK_KEY_INODE),
- &layer_masks, &request, NULL, 0, NULL, NULL, NULL)) {
+ &layer_masks, &rule_flags, &request, NULL, 0, NULL, NULL,
+ NULL, NULL)) {
allowed_access = full_access_request;
} else {
unsigned long access_bit;
diff --git a/security/landlock/net.c b/security/landlock/net.c
index 1f3915a90a80..fc6369dffa51 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -48,6 +48,7 @@ static int current_check_access_socket(struct socket *const sock,
{
__be16 port;
layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
+ struct collected_rule_flags rule_flags = {};
const struct landlock_rule *rule;
struct landlock_id id = {
.type = LANDLOCK_KEY_NET_PORT,
@@ -179,7 +180,7 @@ static int current_check_access_socket(struct socket *const sock,
access_request, &layer_masks,
LANDLOCK_KEY_NET_PORT);
if (landlock_unmask_layers(rule, access_request, &layer_masks,
- ARRAY_SIZE(layer_masks)))
+ ARRAY_SIZE(layer_masks), &rule_flags))
return 0;
audit_net.family = address->sa_family;
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index dfcdc19ea268..81cdf87d1c79 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -624,7 +624,8 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
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)
+ const size_t masks_array_size,
+ struct collected_rule_flags *const rule_flags)
{
size_t layer_level;
@@ -651,6 +652,10 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
unsigned long access_bit;
bool is_empty;
+ /* Collect rule flags for each layer. */
+ if (rule_flags && layer->flags.quiet)
+ rule_flags->quiet_masks |= layer_bit;
+
/*
* Records in @layer_masks which layer grants access to each requested
* access: bit cleared if the related layer grants access.
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 1a78cba662b2..9790c60c0c00 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -29,7 +29,18 @@ struct landlock_layer {
/**
* @level: Position of this layer in the layer stack. Starts from 1.
*/
- u16 level;
+ u8 level;
+ /**
+ * @flags: Bitfield for special flags attached to this rule.
+ */
+ struct {
+ /**
+ * @quiet: Suppresses denial audit logs for the object covered by
+ * this rule in this domain. For filesystem rules, this inherits
+ * down the file hierarchy.
+ */
+ bool quiet:1;
+ } flags;
/**
* @access: Bitfield of allowed actions on the kernel object. They are
* relative to the object type (e.g. %LANDLOCK_ACTION_FS_READ).
@@ -37,6 +48,16 @@ struct landlock_layer {
access_mask_t access;
};
+/**
+ * struct collected_rule_flags - Hold accumulated flags for each layer.
+ */
+struct collected_rule_flags {
+ /**
+ * @quiet_masks: Layers for which the quiet flag is effective.
+ */
+ layer_mask_t quiet_masks;
+};
+
/**
* union landlock_key - Key of a ruleset's red-black tree
*/
@@ -304,7 +325,8 @@ landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
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);
+ const size_t masks_array_size,
+ struct collected_rule_flags *const rule_flags);
access_mask_t
landlock_init_layer_masks(const struct landlock_ruleset *const domain,
--
2.51.1
^ permalink raw reply related
* [PATCH v3 0/8] Implement LANDLOCK_ADD_RULE_QUIET
From: Tingmao Wang @ 2025-10-26 20:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Tingmao Wang, Günther Noack, Jan Kara, Abhinav Saxena,
linux-security-module
Hi,
This is the v3 of the "quiet flag" series, implementing the feature as
proposed in [1].
v2: https://lore.kernel.org/all/cover.1759686613.git.m@maowtm.org/
v1: https://lore.kernel.org/all/cover.1757376311.git.m@maowtm.org/
Not much has changed in the actual functionality except various comment,
typing, asserts and general style fixes based on feedback. The major new
thing here is tests (a bit of KUnit squashed into the optional access
commit, a lot of selftests especially in fs_tests.c).
This series is still missing:
- Tests for scopes (ptrace, mounts, signals, abstract sockets)
- Some trivial edge cases (quiet_access > handled_access, setting quiet
when quiet_access == 0)
However it turns out the tests I've already finished has grown quite
large, and I expect many feedback on it. As such I thought I would send
this v3 now anyway, as there is probably already a lot to review. The
added fs_tests should exercise code path for optional and non-optional
access, renames, and mountpoint and disconnected directory handling. I
will add the above missing bits to v4.
New patches since v2:
- "selftests/landlock: add tests for quiet flag with net rules"
- "selftests/landlock: add tests for quiet flag with fs rules"
Removed:
- "Implement quiet for optional accesses"
(squashed into "landlock: Suppress logging when quiet flag is present")
Old feature summary below:
The quiet flag allows a sandboxer to suppress audit logs for uninteresting
denials. The flag can be set on objects and inherits downward in the
filesystem hierarchy. On a denial, the youngest denying layer's quiet
flag setting decides whether to audit. The motivation for this feature is
to reduce audit noise, and also prepare for a future supervisor feature
which will use this bit to suppress supervisor notifications.
This patch introduces a new quiet access mask in the ruleset_attr, which
gets eventually stored in the hierarchy. This allows the user to specify
which access should be affected by quiet bits. One can then, for example,
make it such that read accesses to certain files are not audited (but
still denied), but all writes are still audited, regardless of location.
The sandboxer is extended to show example usage of this feature,
supporting quieting filesystem, network and scope accesses.
Demo:
/# LL_FS_RO=/usr LL_FS_RW= LL_FORCE_LOG=1 LL_FS_QUIET=/dev:/tmp:/etc LL_FS_QUIET_ACCESS=r ./sandboxer bash
...
audit: type=1423 audit(1759680175.562:195): domain=15bb25f6b blockers=fs.write_file,fs.read_file path="/dev/tty" dev="devtmpfs" ino=11
^^^^^^^^
# note: because write is not quieted, we see the above line. blockers
# contains read as well since that's the originally requested access.
audit: type=1424 audit(1759680175.562:195): domain=15bb25f6b status=allocated mode=enforcing pid=616 uid=0 exe="/sandboxer" comm="sandboxer"
audit: type=1300 audit(1759680175.562:195): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=5565c86113d1 a2=802 a3=0 items=0 ppid=605 pid=616 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="bash" exe="/usr/bin/bash" key=(null)
audit: type=1327 audit(1759680175.562:195): proctitle="bash"
bash: cannot set terminal process group (605): Inappropriate ioctl for device
bash: no job control in this shell
bash: /etc/bash.bashrc: Permission denied
audit: type=1423 audit(1759680175.570:196): domain=15bb25f6b blockers=fs.read_file path="/.bash_history" dev="virtiofs" ino=36963
^^^^^^^^
# read outside /dev:/tmp:/etc - not quieted
audit: type=1300 audit(1759680175.570:196): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=5565c868e400 a2=0 a3=0 items=0 ppid=605 pid=616 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="bash" exe="/usr/bin/bash" key=(null)
audit: type=1327 audit(1759680175.570:196): proctitle="bash"
audit: type=1423 audit(1759680175.570:197): domain=15bb25f6b blockers=fs.read_file path="/.bash_history" dev="virtiofs" ino=36963
audit: type=1300 audit(1759680175.570:197): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=5565c868e400 a2=0 a3=0 items=0 ppid=605 pid=616 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="bash" exe="/usr/bin/bash" key=(null)
audit: type=1327 audit(1759680175.570:197): proctitle="bash"
bash-5.2# head /etc/passwd
head: cannot open '/etc/passwd' for reading: Permission denied
^^^^^^^^
# reads to /etc are quieted
bash-5.2# echo evil >> /etc/passwd
bash: /etc/passwd: Permission denied
audit: type=1423 audit(1759680227.030:198): domain=15bb25f6b blockers=fs.write_file path="/etc/passwd" dev="virtiofs" ino=790
^^^^^^^^
# writes are not quieted
audit: type=1300 audit(1759680227.030:198): arch=c000003e syscall=257 success=no exit=-13 a0=ffffffffffffff9c a1=5565c86ab030 a2=441 a3=1b6 items=0 ppid=605 pid=616 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="bash" exe="/usr/bin/bash" key=(null)
audit: type=1327 audit(1759680227.030:198): proctitle="bash"
Design:
- The user can set the quiet flag for a layer on any part of the fs
hierarchy (whether it allows any access on it or not), and the flag
inherits down (no support for "cancelling" the inheritance of the flag
in specific subdirectories).
- The youngest layer that denies a request gets to decide whether the
denial is audited or not. This means that a compromised binary, for
example, cannot "turn off" Landlock auditing when it tries to access
files, unless it denies access to the files itself. There is some
debate to be had on whether, if a parent layer sets the quiet flag, but
the request is denied by a deeper layer, whether Landlock should still
audit anyway (since the rule author of the child layer likely did not
expect the denial, so it would be good diagnostic). The current
approach is to ignore the quiet on the parent layer and audit anyway.
[1]: https://github.com/landlock-lsm/linux/issues/44#issuecomment-2876500918
Kind regards,
Tingmao
Tingmao Wang (8):
landlock: Add a place for flags to layer rules
landlock: Add API support and docs for the quiet flags
landlock: Suppress logging when quiet flag is present
landlock: Fix wrong type usage
samples/landlock: Add quiet flag support to sandboxer
selftests/landlock: Replace hard-coded 16 with a constant
selftests/landlock: add tests for quiet flag with fs rules
selftests/landlock: add tests for quiet flag with net rules
include/uapi/linux/landlock.h | 64 +
samples/landlock/sandboxer.c | 133 +-
security/landlock/access.h | 5 +
security/landlock/audit.c | 257 +-
security/landlock/audit.h | 4 +-
security/landlock/domain.c | 33 +
security/landlock/domain.h | 10 +
security/landlock/fs.c | 144 +-
security/landlock/fs.h | 19 +-
security/landlock/net.c | 11 +-
security/landlock/net.h | 3 +-
security/landlock/ruleset.c | 17 +-
security/landlock/ruleset.h | 39 +-
security/landlock/syscalls.c | 72 +-
security/landlock/task.c | 12 +-
tools/testing/selftests/landlock/audit_test.c | 2 +-
tools/testing/selftests/landlock/base_test.c | 4 +-
tools/testing/selftests/landlock/common.h | 2 +
tools/testing/selftests/landlock/fs_test.c | 2238 ++++++++++++++++-
tools/testing/selftests/landlock/net_test.c | 121 +-
20 files changed, 3058 insertions(+), 132 deletions(-)
base-commit: 72fb0170ef1f45addf726319c52a0562b6913707
--
2.51.1
^ permalink raw reply
* [RFC bpf-next] lsm: bpf: Remove lsm_prop_bpf
From: Song Liu @ 2025-10-25 0:10 UTC (permalink / raw)
To: bpf, linux-security-module
Cc: paul, jmorris, serge, casey, kpsingh, mattbobrowski, ast, daniel,
andrii, john.johansen, Song Liu
lsm_prop_bpf is not used in any code. Remove it.
Signed-off-by: Song Liu <song@kernel.org>
---
Or did I miss any user of it?
---
include/linux/lsm/bpf.h | 16 ----------------
include/linux/security.h | 2 --
2 files changed, 18 deletions(-)
delete mode 100644 include/linux/lsm/bpf.h
diff --git a/include/linux/lsm/bpf.h b/include/linux/lsm/bpf.h
deleted file mode 100644
index 8106e206fcef..000000000000
--- a/include/linux/lsm/bpf.h
+++ /dev/null
@@ -1,16 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Linux Security Module interface to other subsystems.
- * BPF may present a single u32 value.
- */
-#ifndef __LINUX_LSM_BPF_H
-#define __LINUX_LSM_BPF_H
-#include <linux/types.h>
-
-struct lsm_prop_bpf {
-#ifdef CONFIG_BPF_LSM
- u32 secid;
-#endif
-};
-
-#endif /* ! __LINUX_LSM_BPF_H */
diff --git a/include/linux/security.h b/include/linux/security.h
index 92ac3f27b973..b6ace332576f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -37,7 +37,6 @@
#include <linux/lsm/selinux.h>
#include <linux/lsm/smack.h>
#include <linux/lsm/apparmor.h>
-#include <linux/lsm/bpf.h>
struct linux_binprm;
struct cred;
@@ -163,7 +162,6 @@ struct lsm_prop {
struct lsm_prop_selinux selinux;
struct lsm_prop_smack smack;
struct lsm_prop_apparmor apparmor;
- struct lsm_prop_bpf bpf;
};
extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v2 1/2] landlock: Multithreading support for landlock_restrict_self()
From: Jann Horn @ 2025-10-24 21:29 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Konstantin Meskhidze, Tingmao Wang,
Paul Moore, linux-security-module
In-Reply-To: <20251020.fohbo6Iecahz@digikod.net>
On Mon, Oct 20, 2025 at 10:12 PM Mickaël Salaün <mic@digikod.net> wrote:
> On Wed, Oct 01, 2025 at 01:18:06PM +0200, Günther Noack wrote:
> > +
> > + /*
> > + * Decrement num_preparing for current, to undo that we
> > + * initialized it to 1 at the beginning of the inner loop.
> > + */
> > + if (atomic_dec_return(&shared_ctx.num_preparing) > 0)
> > + wait_for_completion(&shared_ctx.all_prepared);
> > + } while (found_more_threads &&
> > + !atomic_read(&shared_ctx.preparation_error));
>
> Is it safe to prevent inconsistencies wrt execve? seccomp uses
> cred_guard_mutex (new code should probably use exec_update_lock), why
> should Landlock not do the same?
We don't have to worry about interactions with execve because, unlike
seccomp, we don't directly change properties of another running
thread; we ask other threads to change their credentials _themselves_.
From a locking context, restrict_one_thread() essentially runs in the
same kind of context as a syscall, and doesn't need any more locking
than the existing landlock_restrict_self().
> Why shouldn't we lock sighand as well?
seccomp uses siglock for the following reasons:
1. to protect against concurrent access to one thread's seccomp filter
information from multiple threads; we don't do anything like that
2. to protect the for_each_thread() loop; we use RCU for that (we
could also use siglock but there's no reason to do that, and RCU is
more lightweight than the siglock which requires disabling interrupts)
3. to ensure that threads' seccomp states don't change between its two
loops over other threads (seccomp_can_sync_threads() and
seccomp_sync_threads()); we don't do anything like that
^ permalink raw reply
* Re: [PATCH v2 1/2] landlock: Multithreading support for landlock_restrict_self()
From: Jann Horn @ 2025-10-24 21:18 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Konstantin Meskhidze, Tingmao Wang,
Paul Moore, linux-security-module
In-Reply-To: <20251017.ohthoos9Ogha@digikod.net>
On Fri, Oct 17, 2025 at 5:04 PM Mickaël Salaün <mic@digikod.net> wrote:
> On Wed, Oct 01, 2025 at 01:18:06PM +0200, Günther Noack wrote:
> > + /* If needed, establish enforcement prerequisites. */
> > + if (!ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN))
> > + task_set_no_new_privs(current);
>
> We should always set PR_SET_NO_NEW_PRIVS if it is set on the calling
> thread as done by seccomp. We should just store the result of
> task_no_new_privs() in tsync_shared_context and use it as condition here.
> This should be explained in the documentation.
>
> This also mean that if the calling thread has CAP_SYS_ADMIN but not
> PR_SET_NO_NEW_PRIVS, then a sibling thread could not have CAP_SYS_ADMIN
> nor PR_SET_NO_NEW_PRIVS. This would be a risky state but mainly because
> of the CAP_SYS_ADMIN inconsistency, not really the missing
> PR_SET_NO_NEW_PRIVS.
Agreed, it would be nice to have behavior that is consistent with seccomp.
[...]
> > +/*
> > + * tsync_works_provide - provides a preallocated tsync_work for the given task
> > + *
> > + * This also stores a task pointer in the context and increments the reference
> > + * count of the task.
> > + *
> > + * Returns:
> > + * A pointer to the preallocated context struct, with task filled in.
> > + *
> > + * NULL, if we ran out of preallocated context structs.
> > + */
> > +static struct tsync_work *tsync_works_provide(struct tsync_works *s,
> > + struct task_struct *task)
> > +{
> > + struct tsync_work *ctx;
> > +
> > + if (s->size >= s->capacity)
>
> In which case can this happen? Should we wrap this in a WARN_ON_ONCE()?
No, this can legitimately happen if new sibling threads are created
between the time we pre-allocate memory and the time we loop over them
to call tsync_works_provide().
[...]
> > + return 0;
> > +
> > + works = krealloc_array(s->works, new_capacity, sizeof(s->works[0]),
> > + flags);
> > + if (IS_ERR(works))
> > + return PTR_ERR(works);
> > +
> > + s->works = works;
> > +
> > + for (i = s->capacity; i < new_capacity; i++) {
> > + s->works[i] = kzalloc(sizeof(*s->works[i]), flags);
>
> We should use a local variable to avoid storing an error code in
> s->works[i] and potentially dereferencing it later (e.g. in
> tsync_work_free).
>
> Why can't we avoid this loop entirely and allocate a flat array with
> only one call to krealloc_array()? Why struct tsync_works->works needs
> to be a pointer to a pointer?
Because pointers to some "struct tsync_work" items might already be in
use as task work or such, so we can't move them to a different address
anymore at this point.
^ permalink raw reply
* Re: [PATCH v2 1/2] landlock: Multithreading support for landlock_restrict_self()
From: Jann Horn @ 2025-10-24 21:11 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, Konstantin Meskhidze, Tingmao Wang,
Paul Moore, linux-security-module
In-Reply-To: <20251001111807.18902-2-gnoack@google.com>
On Wed, Oct 1, 2025 at 1:18 PM Günther Noack <gnoack@google.com> wrote:
> Introduce the LANDLOCK_RESTRICT_SELF_TSYNC flag. With this flag, a
> given Landlock ruleset is applied to all threads of the calling
> process, instead of only the current one.
>
> Without this flag, multithreaded userspace programs currently resort
> to using the nptl(7)/libpsx hack for multithreaded policy enforcement,
> which is also used by libcap and for setuid(2). Using this scheme,
> the threads of a process enforce the same Landlock ruleset, but the
> resulting Landlock domains are still separate, which makes a
> difference for Landlock's "scoped" access rights, where the domain
> identity and nesting is used. As a result, when using
> LANLDOCK_SCOPE_SIGNAL, signaling between sibling threads stops
> working. This is a problem for programming languages and frameworks
> which are inherently multithreaded (e.g. Go).
This looks good to me overall, though there are a couple details to fix.
[...]
> +static inline void landlock_cred_copy(struct landlock_cred_security *dst,
> + const struct landlock_cred_security *src)
> +{
> + if (dst->domain)
> + landlock_put_ruleset(dst->domain);
> +
> + *dst = *src;
nit: I would add a short comment at the definition of struct
landlock_cred_security noting that this function memcpy's the entire
struct
> +
> + if (dst->domain)
> + landlock_get_ruleset(src->domain);
> +}
[...]
> +/*
> + * tsync_works_grow_by - preallocates space for n more contexts in s
> + *
> + * Returns:
> + * -ENOMEM if the (re)allocation fails
> + * 0 if the allocation succeeds, partially succeeds, or no reallocation was needed
> + */
> +static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags)
> +{
> + int i;
> + size_t new_capacity = s->capacity + n;
(You only have to grow to `s->size + n` but I guess this works too.)
> + struct tsync_work **works;
> +
> + if (new_capacity <= s->capacity)
> + return 0;
> +
> + works = krealloc_array(s->works, new_capacity, sizeof(s->works[0]),
> + flags);
> + if (IS_ERR(works))
> + return PTR_ERR(works);
The kmalloc function family returns NULL on failure, so you have to
check for NULL here instead of IS_ERR(), and then return -ENOMEM
instead of PTR_ERR().
> +
> + s->works = works;
> +
> + for (i = s->capacity; i < new_capacity; i++) {
> + s->works[i] = kzalloc(sizeof(*s->works[i]), flags);
> + if (IS_ERR(s->works[i])) {
(again, kzalloc() returns NULL on failure)
> + /*
> + * Leave the object in a consistent state,
> + * but return an error.
> + */
> + s->capacity = i;
> + return PTR_ERR(s->works[i]);
> + }
> + }
> + s->capacity = new_capacity;
> + return 0;
> +}
[...]
> +/*
> + * tsync_works_free - free memory held by s and drop all task references
> + */
> +static void tsync_works_free(struct tsync_works *s)
> +{
> + int i;
> +
> + for (i = 0; i < s->size; i++)
> + put_task_struct(s->works[i]->task);
You'll need a NULL check before calling put_task_struct(), since the
task_work_add() failure path can NULL out ->task. (Alternatively you
could leave the task pointer intact in the task_work_add() failure
path, since task_work_add() only fails if the task is already
PF_EXITING. The &work_exited marker which causes task_work_add() to
fail is only put on the task work list when task_work_run() runs on a
PF_EXITING task.)
> + for (i = 0; i < s->capacity; i++)
> + kfree(s->works[i]);
> + kfree(s->works);
> + s->works = NULL;
> + s->size = 0;
> + s->capacity = 0;
> +}
> +
> +/*
> + * restrict_sibling_threads - enables a Landlock policy for all sibling threads
> + */
> +static int restrict_sibling_threads(const struct cred *old_cred,
> + const struct cred *new_cred)
> +{
> + int res;
> + struct task_struct *thread, *caller;
> + struct tsync_shared_context shared_ctx;
> + struct tsync_works works = {};
> + size_t newly_discovered_threads;
> + bool found_more_threads;
> + struct tsync_work *ctx;
> +
> + atomic_set(&shared_ctx.preparation_error, 0);
> + init_completion(&shared_ctx.all_prepared);
> + init_completion(&shared_ctx.ready_to_commit);
> + atomic_set(&shared_ctx.num_unfinished, 0);
I think num_unfinished should be initialized to 1 here and decremented
later on, I think, similar to how num_preparing works. Though it only
matters in the edge case where the first thread we send task work to
immediately fails the memory allocation. (And then you can also remove
that "if (works.size)" check before
"wait_for_completion(&shared_ctx.all_finished)".)
> + init_completion(&shared_ctx.all_finished);
> + shared_ctx.old_cred = old_cred;
> + shared_ctx.new_cred = new_cred;
> +
> + caller = current;
[...]
> + init_task_work(&ctx->work,
> + restrict_one_thread_callback);
> + res = task_work_add(thread, &ctx->work, TWA_SIGNAL);
> + if (res) {
> + /*
> + * Remove the task from ctx so that we will
> + * revisit the task at a later stage, if it
> + * still exists.
> + */
> + put_task_struct_rcu_user(ctx->task);
The complement to get_task_struct() is put_task_struct(), which I see
you also used in tsync_works_free(). put_task_struct_rcu_user() is for
a different, special type of task_struct reference.
> + ctx->task = NULL;
> +
> + atomic_set(&shared_ctx.preparation_error, res);
I think you don't want to set preparation_error here - that would
cause the syscall to return -ESRCH if we happen to race with an
exiting thread. Just remove that line - in the next iteration, we'll
skip this thread even if it still exists, because it has PF_EXITING
set by this point.
> + atomic_dec(&shared_ctx.num_preparing);
> + atomic_dec(&shared_ctx.num_unfinished);
> + }
> + }
> + rcu_read_unlock();
> +
> + /*
> + * Decrement num_preparing for current, to undo that we
> + * initialized it to 1 at the beginning of the inner loop.
> + */
> + if (atomic_dec_return(&shared_ctx.num_preparing) > 0)
> + wait_for_completion(&shared_ctx.all_prepared);
I'm sorry, because this will make the patch a little bit more
complicated, but... I don't think you can use wait_for_completion()
here. Consider the scenario where two userspace threads of the same
process call this functionality (or a kernel subsystem that does
something similar) simultaneously. Each thread will wait for the other
indefinitely, and userspace won't even be able to resolve the deadlock
by killing the processes.
Similar issues would probably apply if, for example, GDB tried to
attach to the process with bad timing - if GDB ptrace-stops another
thread before you schedule task work for it, and then tries to
ptrace-stop this thread, I think this thread could essentially be in a
deadlock with GDB.
You'll have to do something else here. I think the best solution would
be to use wait_for_completion_interruptible() instead; then if that
fails, tear down all the task work stuff that was already scheduled,
and return with error -ERESTARTNOINTR. Something like (entirely
untested):
/* interruptible wait to avoid deadlocks while waiting for other tasks
to enter our task work */
if (wait_for_completion_interruptible(&shared_ctx.all_prepared)) {
atomic_set(&shared_ctx.preparation_error, -ERESTARTNOINTR);
for (int i=0; i<works.size; i++) {
if (task_work_cancel(works.works[i]->task, &works.works[i]->work))
if (atomic_dec_return(&shared_ctx.num_preparing))
complete_all(&shared_ctx.all_prepared);
}
/* at this point we're only waiting for tasks that are already
executing the task work */
wait_for_completion(&shared_ctx.all_prepared);
}
Note that if the syscall returns -ERESTARTNOINTR, that won't be
visible to userspace (except for debugging tools like strace/gdb); the
kernel ensures that the syscall will transparently re-execute
immediately. (It literally decrements the saved userspace instruction
pointer by the size of a syscall instruction, so that when the kernel
returns to userspace, the next instruction that executes will redo the
syscall.) This allows us to break the deadlock without having to write
any ugly retry logic or throwing userspace-visible errors.
> + } while (found_more_threads &&
> + !atomic_read(&shared_ctx.preparation_error));
> +
> + /*
> + * We now have all sibling threads blocking and in "prepared" state in
> + * the task work. Ask all threads to commit.
> + */
> + complete_all(&shared_ctx.ready_to_commit);
> +
> + if (works.size)
> + wait_for_completion(&shared_ctx.all_finished);
> +
> + tsync_works_free(&works);
> +
> + return atomic_read(&shared_ctx.preparation_error);
> +}
[...]
> @@ -566,5 +987,13 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> new_llcred->domain_exec |= BIT(new_dom->num_layers - 1);
> #endif /* CONFIG_AUDIT */
>
> + if (flags & LANDLOCK_RESTRICT_SELF_TSYNC) {
> + res = restrict_sibling_threads(current_cred(), new_cred);
> + if (res != 0) {
> + abort_creds(new_cred);
> + return res;
> + }
> + }
Annoyingly, there is a special-case path above for the case where
LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF is set without actually
applying any ruleset. In that case you won't reach this point, and so
LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF would only affect the
current thread in that case. I doubt it'd be very noticeable, but
still, it might be a good idea to rearrange things here a bit... maybe
instead of the current `if (!ruleset) return commit_creds(new_cred);`,
put some of the subsequent stuff in a `if (ruleset) {` block?
^ permalink raw reply
* Re: [GIT PULL] Block fixes for 6.18-rc3
From: Linus Torvalds @ 2025-10-24 20:31 UTC (permalink / raw)
To: Jens Axboe, Paul Moore, Serge Hallyn, Christian Brauner
Cc: linux-block@vger.kernel.org, LSM List
In-Reply-To: <37fb8720-bee9-43b7-b0ff-0214a8ad33a2@kernel.dk>
[ Adding LSM people. Also Christian, because he did the cred refcount
cleanup with override_creds() and friends last year, and I'm
suggesting taking that one step further ]
On Fri, 24 Oct 2025 at 06:58, Jens Axboe <axboe@kernel.dk> wrote:
>
> Ondrej Mosnacek (1):
> nbd: override creds to kernel when calling sock_{send,recv}msg()
I've pulled this, but looking at the patch, I note that more than half
the patch - 75% to be exact - is just boilerplate for "I need to
allocate the kernel cred and deal with error handling there".
It literally has three lines of new actual useful code (two statements
and one local variable declaration), and then nine lines of the "setup
dance".
Which isn't wrong, but when the infrastructure boilerplate is three
times more than the actual code, it makes me think we should maybe
just get rid of the
my_kernel_cred = prepare_kernel_cred(&init_task);
pattern for this use-case, and just let people use "init_cred"
directly for things like this.
Because that's essentially what that prepare_kernel_cred() thing
returns, except it allocates a new copy of said thing, so now you have
error handling and you have to free it after-the-fact.
And I'm not seeing that the extra error handling and freeing dance
actually buys us anything at all.
Now, some *other* users actually go on to change the creds: they want
that prepare_kernel_cred() dance because they then actually do
something else like using their own keyring or whatever (eg the NFS
idmap code or some other filesystem stuff).
So it's not like prepare_kernel_cred() is wrong, but in this kind of
case where people just go "I'm a driver with hardware access, I want
to do something with kernel privileges not user privileges", it
actually seems counterproductive to have extra code just to complicate
things.
Now, my gut feel is that if we just let people use 'init_cred'
directly, we should also make sure that it's always exposed as a
'const struct cred' , but wouldn't that be a whole lot simpler and
more straightforward?
This is *not* the only use case of that.
We now have at least four use-cases of this "raw kernel cred" pattern:
core-dumping over unix domain socket, nbd, firmware loading and SCSI
target all do this exact thing as far as I can tell.
So they all just want that bare kernel cred, and this interface then
forces it to do extra work instead of just doing
old_cred = override_creds(&init_cred);
...
revert_creds(old_cred);
and it ends up being extra code for allocating and freeing that copy
of a cred that we already *had* and could just have used directly.
I did just check that making 'init_cred' be const
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -28 +28 @@ extern struct nsproxy init_nsproxy;
-extern struct cred init_cred;
+extern const struct cred init_cred;
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -44 +44 @@ static struct group_info init_groups = { .usage =
REFCOUNT_INIT(2) };
-struct cred init_cred = {
+const struct cred init_cred = {
seems to build just fine and would seem to be the right thing to do
even if we *don't* expect people to use it. And override_creds() is
perfectly happy with a
Maybe there's some reason for that extra work that I'm not seeing and
thinking of? But it all smells like make-believe work to me that
probably has a historical reason for it, but doesn't seem to make a
lot of sense any more.
Hmm?
Linus
^ permalink raw reply
* Re: [PATCH v6 10/10] tpm-buf: Enable managed and stack allocations.
From: Jarkko Sakkinen @ 2025-10-24 18:49 UTC (permalink / raw)
To: Stefano Garzarella
Cc: linux-integrity, keyring, dpsmith, ross.philipson,
Jonathan McDowell, Roberto Sassu, Jarkko Sakkinen, Stefan Berger,
Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
linux-kernel, keyrings, linux-security-module
In-Reply-To: <yynqxoqux5whcbsnticikhwmupqh57xfbll5egzkn42kj7gkaf@s4kwxfmto5ia>
On Mon, Oct 20, 2025 at 11:04:51AM +0200, Stefano Garzarella wrote:
> On Sat, Oct 18, 2025 at 02:17:25PM +0300, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> >
> > Decouple kzalloc from buffer creation, so that a managed allocation can be
> > done trivially:
> >
> > struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUFSIZE,
> > GFP_KERNEL);
> > if (!buf)
> > return -ENOMEM;
> > tpm_buf_init(buf, TPM_BUFSIZE);
> >
> > Alternatively, stack allocations are also possible:
> >
> > u8 buf_data[512];
> > struct tpm_buf *buf = (struct tpm_buf *)buf_data;
> > tpm_buf_init(buf, sizeof(buf_data));
> >
> > Given that every single tpm_transmit_cmd() call site needs to be changed,
> > place command names from TCG 1.2 and 2.0 specifications to the @dest
> > parameter, which will e.g., help tracing bugs.
>
> Perhaps my previous message fell through the cracks, but I still have a
> couple of comments (perhaps trivial, sorry in that case) that have not been
> answered about this patch:
I think what happened is that there was enough time that I forgot what
I had or hadn't done :-) I'll address your comments.
>
> >
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> > v6
> > - Update commit message.
> > v5:
> > - There was a spurious change in tpm2_seal_trusted() error
> > code handling introduce by this patch.
> > v4:
> > - Since every single tpm_transmit_cmd() call site needs to be
> > changed anyhow, use 'dest' parameter more structured and
> > actually useful way, and pick the string TCG 1.2 and 2.0
> > specifications.
> > - tpm1-cmd: Remove useless rc declarations and repliace them
> > with trivial "return tpm_transmit_cmd" statement.
> > - Reverted spurious changes in include/linux/tpm.h.
> > - Use concisely TPM_BUFSIZE instead of PAGE_SIZE.
> > v3:
> > - A new patch from the earlier series with more scoped changes and
> > less abstract commit message.
> > ---
> > drivers/char/tpm/tpm-buf.c | 122 +++++----
> > drivers/char/tpm/tpm-sysfs.c | 21 +-
> > drivers/char/tpm/tpm.h | 1 -
> > drivers/char/tpm/tpm1-cmd.c | 162 +++++-------
> > drivers/char/tpm/tpm2-cmd.c | 299 ++++++++++------------
> > drivers/char/tpm/tpm2-sessions.c | 122 +++++----
> > drivers/char/tpm/tpm2-space.c | 44 ++--
> > drivers/char/tpm/tpm_vtpm_proxy.c | 30 +--
> > include/linux/tpm.h | 18 +-
> > security/keys/trusted-keys/trusted_tpm1.c | 34 ++-
> > security/keys/trusted-keys/trusted_tpm2.c | 175 ++++++-------
> > 11 files changed, 484 insertions(+), 544 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> > index 1b9dee0d0681..a3bf3c3d0c48 100644
> > --- a/drivers/char/tpm/tpm-buf.c
> > +++ b/drivers/char/tpm/tpm-buf.c
>
> [...]
>
> > @@ -92,6 +119,9 @@ EXPORT_SYMBOL_GPL(tpm_buf_destroy);
> > */
> > u32 tpm_buf_length(struct tpm_buf *buf)
>
> Should we update the return value to u16?
Ack.
>
>
> > {
> > + if (buf->flags & TPM_BUF_INVALID)
> > + return 0;
> > +
> > return buf->length;
> > }
> > EXPORT_SYMBOL_GPL(tpm_buf_length);
>
> [...]
>
> > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > index 636acb66a4f6..3ac204a902de 100644
> > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > @@ -310,9 +310,8 @@ static int TSS_checkhmac2(unsigned char *buffer,
> > * For key specific tpm requests, we will generate and send our
> > * own TPM command packets using the drivers send function.
> > */
> > -static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> > +static int trusted_tpm_send(void *cmd, size_t buflen)
> > {
> > - struct tpm_buf buf;
> > int rc;
> >
> > if (!chip)
> > @@ -322,15 +321,12 @@ static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> > if (rc)
> > return rc;
> >
> > - buf.flags = 0;
> > - buf.length = buflen;
> > - buf.data = cmd;
> > dump_tpm_buf(cmd);
> > - rc = tpm_transmit_cmd(chip, &buf, 4, "sending data");
> > + rc = tpm_transmit_cmd(chip, cmd, 4, "sending data");
>
> Is it fine here to remove the intermediate tpm_buf ?
>
> IIUC tpm_transmit_cmd() needs a tpm_buf, while here we are passing just
> the "data", or in some way it's a nested tpm_buf?
This does not look right at all. I'll fix it and add automated test
suite for TPM 1.x to my standard test shenanigans so that I can
continuously test its correctness [1]. It anyhow should have TPM 1.2
tests (especially for trusted keys). As it becomes more rare
accidental bugs could easily hover in.
Thanks, this was really good catch. I'll address both bug and make
sure that I don't do the same mistake twice :-)
>
> > dump_tpm_buf(cmd);
> >
> > + /* Convert TPM error to -EPERM. */
> > if (rc > 0)
> > - /* TPM error */
> > rc = -EPERM;
> >
> > tpm_put_ops(chip);
>
> Thanks,
> Stefano
>
[1] https://codeberg.org/jarkko/linux-tpmdd-test
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] ima: Fall back to default kernel module signature verification
From: Mimi Zohar @ 2025-10-24 15:16 UTC (permalink / raw)
To: Coiby Xu
Cc: linux-integrity, Dmitry Torokhov, Karel Srot, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <559f6ebf4a19da321fffc2a3ca180dc3d6216a22.camel@linux.ibm.com>
On Mon, 2025-10-20 at 08:21 -0400, Mimi Zohar wrote:
> On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
> > > > > 2. Instead of defining an additional process_measurement() argument to identify
> > > > > compressed kernel modules, to simplify the code it might be possible to define a
> > > > > new "func" named COMPRESSED_MODULE_CHECK.
> > > > >
> > > > > + [READING_COMPRESSED_MODULE] = MODULE_CHECK, -> COMPRESSED_MODULE_CHECK
> > > >
> > > > I also thought about this approach. But IMA rule maps kernel module
> > > > loading to MODULE_CHECK. If we define a new rule and ask users to use
> > > > this new rule, ima_policy=secure_boot still won't work.
> > >
> > > I don't have a problem with extending the "secure-boot" policy to support
> > > uncompressed kernel modules appended signatures, based on whether
> > > CONFIG_MODULE_SIG is enabled. The new rule would be in addition to the existing
> > > MODULE_CHECK rule.
> >
> > I assume once the new rule get added, we can't remove it for userspace
> > backward compatibility, right? And with CPIO xattr supported, it seems
> > there is no need to keep this rule. So if this concern is valid, do you
> > think we shall switch to another approach i.e. to make IMA support
> > verifying decompressed module and then make "secure-boot" to allow
> > appended module signature?
>
> Yes, once the rule is added, it wouldn't be removed. As for "to make IMA
> support verifying decompressed module", yes that might be a better solution,
> than relying on "sig_enforce" being enabled. IMA already supports verifying the
> appended signatures. A new IMA specific or LSM hook would need to be defined
> after module_decompress().
Looking at the code further, decompressing the kernel module in IMA is
redundant. Instead I think the best approach would be to:
- define DECOMPRESSED_MODULE, in addition to COMPRESSED_MODULE.
id(COMPRESSED_MODULE, compressed-kernel-module) \
id(DECOMPRESSED_MODULE, decompressed-kernel-module) \
- instead of passing a boolean indicating whether the module is compressed, pass
the kernel_read_file_id enumeration to differentiate between the compressed and
decompressed module.
- define a new IMA hook, probably LSM hook as well, named
ima_decompressed_module().
- call the new ima_decompressed_module() from init_module_from_file()
immediately after decompressing the kernel module. Something along the lines
of:
err = ima_decompressed_module(f, (char *)info.hdr, info.len,
READING_DECOMPRESSED_MODULE);
For testing purposes to see the decompressed appended signature in the
measurement list, modify the MODULE_CHECK measure rule to include "template=ima-
modsig" in ima_efi.c.
--
Mimi
^ permalink raw reply
* Re: [PATCH] KEYS: fix compilation warnings in the dump_options() function
From: Ahmad Fatoum @ 2025-10-24 8:10 UTC (permalink / raw)
To: yebin, kernel, James.Bottomley, jarkko, zohar, dhowells, paul,
jmorris, serge, linux-integrity, keyrings, linux-security-module,
yebin10
In-Reply-To: <68FB2470.4000206@huaweicloud.com>
Hello,
On 10/24/25 9:02 AM, yebin wrote:
> Ignore this patch as 275a9a3f9b6a（“KEYS: trusted: Pass argument by
> pointer in dump_options”）already fix this issue.
What tree are you looking at? I can't find this commit in my git and the
code you are purportedly patching never existed upstream.
If you run into issues exclusive to a vendor fork, you need to submit
your patches to the vendor. The upstream mailing lists are for upstream.
Thanks,
Ahmad
>
> On 2025/10/24 14:11, Ye Bin wrote:
>> From: Ye Bin <yebin10@huawei.com>
>>
>> There's issue as follows:
>> security/keys/trusted-keys/trusted_caam.c: In function ‘dump_options’:
>> security/keys/trusted-keys/trusted_caam.c:37:20: note: the ABI of
>> passing struct with a flexible array member has changed in GCC 4.4
>> 37 | static inline void dump_options(struct caam_pkey_info pkey_info)
>> | ^~~~~~~~~~~~
>>
>> To solve the above problem, pass 'struct caam_pkey_info*' type parameter
>> to the dump_options() function.
>>
>> Fixes: 9eb25ca6c973 ("KEYS: trusted: caam based protected key")
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>> security/keys/trusted-keys/trusted_caam.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/
>> keys/trusted-keys/trusted_caam.c
>> index 090099d1b04d..dd7a69bcf6a3 100644
>> --- a/security/keys/trusted-keys/trusted_caam.c
>> +++ b/security/keys/trusted-keys/trusted_caam.c
>> @@ -29,12 +29,12 @@ static const match_table_t key_tokens = {
>> };
>>
>> #ifdef CAAM_DEBUG
>> -static inline void dump_options(struct caam_pkey_info pkey_info)
>> +static inline void dump_options(struct caam_pkey_info *pkey_info)
>> {
>> - pr_info("key encryption algo %d\n", pkey_info.key_enc_algo);
>> + pr_info("key encryption algo %d\n", pkey_info->key_enc_algo);
>> }
>> #else
>> -static inline void dump_options(struct caam_pkey_info pkey_info)
>> +static inline void dump_options(struct caam_pkey_info *pkey_info)
>> {
>> }
>> #endif
>> @@ -108,7 +108,7 @@ static int trusted_caam_seal(struct
>> trusted_key_payload *p, char *datablob)
>> ret = get_pkey_options(datablob, &info.pkey_info);
>> if (ret < 0)
>> return 0;
>> - dump_options(info.pkey_info);
>> + dump_options(&info.pkey_info);
>> }
>>
>> ret = caam_encap_blob(blobifier, &info);
>> @@ -140,7 +140,7 @@ static int trusted_caam_unseal(struct
>> trusted_key_payload *p, char *datablob)
>> ret = get_pkey_options(datablob, &info.pkey_info);
>> if (ret < 0)
>> return 0;
>> - dump_options(info.pkey_info);
>> + dump_options(&info.pkey_info);
>>
>> p->key_len = p->blob_len + sizeof(struct caam_pkey_info);
>> memcpy(p->key, &info.pkey_info, sizeof(struct caam_pkey_info));
>>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH] KEYS: fix compilation warnings in the dump_options() function
From: yebin @ 2025-10-24 7:02 UTC (permalink / raw)
To: a.fatoum, kernel, James.Bottomley, jarkko, zohar, dhowells, paul,
jmorris, serge, linux-integrity, keyrings, linux-security-module,
yebin10
In-Reply-To: <20251024061153.61470-1-yebin@huaweicloud.com>
Ignore this patch as 275a9a3f9b6a（“KEYS: trusted: Pass argument by
pointer in dump_options”）already fix this issue.
On 2025/10/24 14:11, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> There's issue as follows:
> security/keys/trusted-keys/trusted_caam.c: In function ‘dump_options’:
> security/keys/trusted-keys/trusted_caam.c:37:20: note: the ABI of passing struct with a flexible array member has changed in GCC 4.4
> 37 | static inline void dump_options(struct caam_pkey_info pkey_info)
> | ^~~~~~~~~~~~
>
> To solve the above problem, pass 'struct caam_pkey_info*' type parameter
> to the dump_options() function.
>
> Fixes: 9eb25ca6c973 ("KEYS: trusted: caam based protected key")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
> security/keys/trusted-keys/trusted_caam.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
> index 090099d1b04d..dd7a69bcf6a3 100644
> --- a/security/keys/trusted-keys/trusted_caam.c
> +++ b/security/keys/trusted-keys/trusted_caam.c
> @@ -29,12 +29,12 @@ static const match_table_t key_tokens = {
> };
>
> #ifdef CAAM_DEBUG
> -static inline void dump_options(struct caam_pkey_info pkey_info)
> +static inline void dump_options(struct caam_pkey_info *pkey_info)
> {
> - pr_info("key encryption algo %d\n", pkey_info.key_enc_algo);
> + pr_info("key encryption algo %d\n", pkey_info->key_enc_algo);
> }
> #else
> -static inline void dump_options(struct caam_pkey_info pkey_info)
> +static inline void dump_options(struct caam_pkey_info *pkey_info)
> {
> }
> #endif
> @@ -108,7 +108,7 @@ static int trusted_caam_seal(struct trusted_key_payload *p, char *datablob)
> ret = get_pkey_options(datablob, &info.pkey_info);
> if (ret < 0)
> return 0;
> - dump_options(info.pkey_info);
> + dump_options(&info.pkey_info);
> }
>
> ret = caam_encap_blob(blobifier, &info);
> @@ -140,7 +140,7 @@ static int trusted_caam_unseal(struct trusted_key_payload *p, char *datablob)
> ret = get_pkey_options(datablob, &info.pkey_info);
> if (ret < 0)
> return 0;
> - dump_options(info.pkey_info);
> + dump_options(&info.pkey_info);
>
> p->key_len = p->blob_len + sizeof(struct caam_pkey_info);
> memcpy(p->key, &info.pkey_info, sizeof(struct caam_pkey_info));
>
^ permalink raw reply
* [PATCH] KEYS: fix compilation warnings in the dump_options() function
From: Ye Bin @ 2025-10-24 6:11 UTC (permalink / raw)
To: a.fatoum, kernel, James.Bottomley, jarkko, zohar, dhowells, paul,
jmorris, serge, linux-integrity, keyrings, linux-security-module,
yebin, yebin10
From: Ye Bin <yebin10@huawei.com>
There's issue as follows:
security/keys/trusted-keys/trusted_caam.c: In function ‘dump_options’:
security/keys/trusted-keys/trusted_caam.c:37:20: note: the ABI of passing struct with a flexible array member has changed in GCC 4.4
37 | static inline void dump_options(struct caam_pkey_info pkey_info)
| ^~~~~~~~~~~~
To solve the above problem, pass 'struct caam_pkey_info*' type parameter
to the dump_options() function.
Fixes: 9eb25ca6c973 ("KEYS: trusted: caam based protected key")
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
security/keys/trusted-keys/trusted_caam.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
index 090099d1b04d..dd7a69bcf6a3 100644
--- a/security/keys/trusted-keys/trusted_caam.c
+++ b/security/keys/trusted-keys/trusted_caam.c
@@ -29,12 +29,12 @@ static const match_table_t key_tokens = {
};
#ifdef CAAM_DEBUG
-static inline void dump_options(struct caam_pkey_info pkey_info)
+static inline void dump_options(struct caam_pkey_info *pkey_info)
{
- pr_info("key encryption algo %d\n", pkey_info.key_enc_algo);
+ pr_info("key encryption algo %d\n", pkey_info->key_enc_algo);
}
#else
-static inline void dump_options(struct caam_pkey_info pkey_info)
+static inline void dump_options(struct caam_pkey_info *pkey_info)
{
}
#endif
@@ -108,7 +108,7 @@ static int trusted_caam_seal(struct trusted_key_payload *p, char *datablob)
ret = get_pkey_options(datablob, &info.pkey_info);
if (ret < 0)
return 0;
- dump_options(info.pkey_info);
+ dump_options(&info.pkey_info);
}
ret = caam_encap_blob(blobifier, &info);
@@ -140,7 +140,7 @@ static int trusted_caam_unseal(struct trusted_key_payload *p, char *datablob)
ret = get_pkey_options(datablob, &info.pkey_info);
if (ret < 0)
return 0;
- dump_options(info.pkey_info);
+ dump_options(&info.pkey_info);
p->key_len = p->blob_len + sizeof(struct caam_pkey_info);
memcpy(p->key, &info.pkey_info, sizeof(struct caam_pkey_info));
--
2.34.1
^ permalink raw reply related
* [PATCH 2/2] ipe: Update documentation for script enforcement
From: Yanzhu Huang @ 2025-10-23 23:36 UTC (permalink / raw)
To: wufan, paul, mic
Cc: jmorris, serge, corbet, yanzhuhuang, linux-security-module,
linux-doc, linux-kernel
In-Reply-To: <20251023233656.661344-1-yanzhuhuang@linux.microsoft.com>
This patch adds explanation of script enforcement mechanism in admin
guide documentation. Describes how IPE supports integrity enforcement
for indirectly executed scripts through the AT_EXECVE_CHECK flag, and
how this differs from kernel enforcement for compiled executables.
Signed-off-by: Yanzhu Huang <yanzhuhuang@linux.microsoft.com>
---
Documentation/admin-guide/LSM/ipe.rst | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/LSM/ipe.rst b/Documentation/admin-guide/LSM/ipe.rst
index dc7088451f9d..1063256559a8 100644
--- a/Documentation/admin-guide/LSM/ipe.rst
+++ b/Documentation/admin-guide/LSM/ipe.rst
@@ -95,7 +95,20 @@ languages when these scripts are invoked by passing these program files
to the interpreter. This is because the way interpreters execute these
files; the scripts themselves are not evaluated as executable code
through one of IPE's hooks, but they are merely text files that are read
-(as opposed to compiled executables) [#interpreters]_.
+(as opposed to compiled executables) [#interpreters]_. However, with the
+introduction of the ``AT_EXECVE_CHECK`` flag, interpreters can use it to
+signal the kernel that a script file will be executed, and request the
+kernel to perform LSM security checks on it.
+
+IPE's EXECUTE operation enforcement differs between compiled executables and
+interpreted scripts: For compiled executables, enforcement is triggered
+automatically by the kernel during ``execve()``, ``execveat()``, ``mmap()``
+and ``mprotect()`` syscalls when loading executable content. For interpreted
+scripts, enforcement requires explicit interpreter integration using
+``execveat()`` with ``AT_EXECVE_CHECK`` flag. Unlike exec syscalls that IPE
+intercepts during the execution process, this mechanism needs the interpreter
+to take the initiative, and existing interpreters won't be automatically
+supported unless the signal call is added.
Threat Model
------------
--
2.43.0
^ permalink raw reply related
* [PATCH 1/2] ipe: Add AT_EXECVE_CHECK support for script enforcement
From: Yanzhu Huang @ 2025-10-23 23:36 UTC (permalink / raw)
To: wufan, paul, mic
Cc: jmorris, serge, corbet, yanzhuhuang, linux-security-module,
linux-doc, linux-kernel
In-Reply-To: <20251023233656.661344-1-yanzhuhuang@linux.microsoft.com>
This patch adds a new ipe_bprm_creds_for_exec() hook that integrates
with the AT_EXECVE_CHECK mechanism. To enable script enforcement,
interpreters need to incorporate the AT_EXECVE_CHECK flag when
calling execveat() on script files before execuation.
When a userspace interpreter calls execveat() with the AT_EXECVE_CHECK
flag, this hook triggers IPE policy evaluation on the script file. The
hook only triggers IPE when bprm->is_check is true, ensuring it's
being called from an AT_EXECVE_CHECK context. It then builds an
evaluation context for an IPE_OP_EXEC operation and invokes IPE policy.
The kernel returns the policy decision to the interpreter, which can
then decide whether to proceed with script execution.
This extends IPE enforcement to indirectly executed scripts, permitting
trusted scripts to execute while denying untrusted ones.
Signed-off-by: Yanzhu Huang <yanzhuhuang@linux.microsoft.com>
---
security/ipe/audit.c | 1 +
security/ipe/hooks.c | 27 +++++++++++++++++++++++++++
security/ipe/hooks.h | 3 +++
security/ipe/ipe.c | 1 +
4 files changed, 32 insertions(+)
diff --git a/security/ipe/audit.c b/security/ipe/audit.c
index de5fed62592e..3f0deeb54912 100644
--- a/security/ipe/audit.c
+++ b/security/ipe/audit.c
@@ -46,6 +46,7 @@ static const char *const audit_op_names[__IPE_OP_MAX + 1] = {
static const char *const audit_hook_names[__IPE_HOOK_MAX] = {
"BPRM_CHECK",
+ "BPRM_CREDS_FOR_EXEC",
"MMAP",
"MPROTECT",
"KERNEL_READ",
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index d0323b81cd8f..32dd99abd4de 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -35,6 +35,33 @@ int ipe_bprm_check_security(struct linux_binprm *bprm)
return ipe_evaluate_event(&ctx);
}
+/**
+ * ipe_bprm_creds_for_exec() - ipe security hook function for bprm creds check.
+ * @bprm: Supplies a pointer to a linux_binprm structure to source the file
+ * being evaluated.
+ *
+ * This LSM hook is called when userspace signals the kernel to check a file
+ * for execution through the execveat syscall with the AT_EXECVE_CHECK flag.
+ * The hook triggers IPE policy evaluation on the script file and returns
+ * the policy decision to userspace. The userspace program receives the
+ * return code and can decide whether to proceed with script execution.
+ *
+ * Return:
+ * * %0 - Success
+ * * %-EACCES - Did not pass IPE policy
+ */
+int ipe_bprm_creds_for_exec(struct linux_binprm *bprm)
+{
+ struct ipe_eval_ctx ctx = IPE_EVAL_CTX_INIT;
+
+ if (!bprm->is_check)
+ return 0;
+
+ ipe_build_eval_ctx(&ctx, bprm->file, IPE_OP_EXEC,
+ IPE_HOOK_BPRM_CREDS_FOR_EXEC);
+ return ipe_evaluate_event(&ctx);
+}
+
/**
* ipe_mmap_file() - ipe security hook function for mmap check.
* @f: File being mmap'd. Can be NULL in the case of anonymous memory.
diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
index 38d4a387d039..07db37332740 100644
--- a/security/ipe/hooks.h
+++ b/security/ipe/hooks.h
@@ -13,6 +13,7 @@
enum ipe_hook_type {
IPE_HOOK_BPRM_CHECK = 0,
+ IPE_HOOK_BPRM_CREDS_FOR_EXEC,
IPE_HOOK_MMAP,
IPE_HOOK_MPROTECT,
IPE_HOOK_KERNEL_READ,
@@ -24,6 +25,8 @@ enum ipe_hook_type {
int ipe_bprm_check_security(struct linux_binprm *bprm);
+int ipe_bprm_creds_for_exec(struct linux_binprm *bprm);
+
int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
unsigned long flags);
diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
index 4317134cb0da..845e3fd7a345 100644
--- a/security/ipe/ipe.c
+++ b/security/ipe/ipe.c
@@ -47,6 +47,7 @@ struct ipe_inode *ipe_inode(const struct inode *inode)
static struct security_hook_list ipe_hooks[] __ro_after_init = {
LSM_HOOK_INIT(bprm_check_security, ipe_bprm_check_security),
+ LSM_HOOK_INIT(bprm_creds_for_exec, ipe_bprm_creds_for_exec),
LSM_HOOK_INIT(mmap_file, ipe_mmap_file),
LSM_HOOK_INIT(file_mprotect, ipe_file_mprotect),
LSM_HOOK_INIT(kernel_read_file, ipe_kernel_read_file),
--
2.43.0
^ permalink raw reply related
* [PATCH 0/2] ipe: add script enforcement mechanism with AT_EXECVE_CHECK
From: Yanzhu Huang @ 2025-10-23 23:36 UTC (permalink / raw)
To: wufan, paul, mic
Cc: jmorris, serge, corbet, yanzhuhuang, linux-security-module,
linux-doc, linux-kernel
Indirect file execution through interpreters (e.g. python script.py, sh
script.sh) should have integrity policy enforced by IPE based on the
rules. Currently, IPE can only enforce policy on the interpreter binary
itself, but has no visibility into the scripts that the interpreter
executes.
Overview
--------
This patch series introduces script enforcement for IPE, allowing integrity
evaluation of indirectly executed scripts through the AT_EXECVE_CHECK flag.
Patch 1 adds the core implementation with ipe_bprm_creds_for_exec() hook
that integrates with the AT_EXECVE_CHECK mechanism.
Patch 2 updates admin guide documentation to explain the script enforcement
mechanism.
The IPE test suite has been updated to include script enforcement tests:
https://github.com/microsoft/ipe/pull/6
Yanzhu Huang (2):
ipe: Add AT_EXECVE_CHECK support for script enforcement
ipe: Update documentation for script enforcement
Documentation/admin-guide/LSM/ipe.rst | 15 ++++++++++++++-
security/ipe/audit.c | 1 +
security/ipe/hooks.c | 27 +++++++++++++++++++++++++++
security/ipe/hooks.h | 3 +++
security/ipe/ipe.c | 1 +
5 files changed, 46 insertions(+), 1 deletion(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: Paul Moore @ 2025-10-23 17:53 UTC (permalink / raw)
To: KP Singh
Cc: James Bottomley, Alexei Starovoitov, Linus Torvalds,
Alexei Starovoitov, Blaise Boscaccy, bpf, LSM List,
K. Y. Srinivasan, Daniel Borkmann, Andrii Nakryiko, wufan,
Quentin Monnet
In-Reply-To: <CACYkzJ4z4vzVEjtOmFHuC9tpDmWp0N-EH-xDK7Bs6YJ-x0W3Sw@mail.gmail.com>
On Thu, Oct 23, 2025 at 11:39 AM KP Singh <kpsingh@kernel.org> wrote:
> On Wed, Oct 22, 2025 at 11:10 PM James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Mon, 2025-10-20 at 18:25 -0700, Alexei Starovoitov wrote:
> > > On Mon, Oct 20, 2025 at 4:13 PM James Bottomley
> > > <James.Bottomley@hansenpartnership.com> wrote:
> > [...]
> > > > The point, for me, is when doing integrity tests both patch sets
> > > > produce identical results and correctly detect when integrity of a
> > > > light skeleton is compromised (in mathematical terms that means
> > > > they're functionally equivalent). The only difference is that with
> > > > Blaise's patch set verification completes before the LSM load hook
> > > > is called and with KP's it completes after ... and the security
> > > > problem with the latter case is that there's no LSM hook to collect
> > > > the verification result.
> > >
> > > the security problem with KP's approach? wtf.
> > > I'm going to add "depends on !microsoft" to kconfig bpf_syscall
> > > and be done with it.
> > > Don't use it since it's so insecure.
> >
> > Most Linux installations use LSMs to enforce and manage policies for
> > system integrity (they don't all use the same set of LSMs, but that's
> > not relevant to the argument). So while Meta may not use LSMs for
> > system integrity the fact that practically everyone else does makes not
> > having a correctly functioning LSM hook for BPF signature verification
> > a problem for a huge set of users that goes way beyond just Microsoft.
>
> The core tenet of your claim is that you need "LSM observability" but
> without any description of a security policy
> that cannot not be currently implemented. The responses I have
> received are generic statements that the loader verification is
> "unsafe"
As we've discussed this many times across various threads over the
past several months, I don't see much point in revisiting the
argument. Instead I'll refer you back to my last response to this,
taken from earlier in this thread; the relevant portion is the last
paragraph:
https://lore.kernel.org/linux-security-module/CAHC9VhSDkwGgPfrBUh7EgBKEJj_JjnY68c0YAmuuLT_i--GskQ@mail.gmail.com/
> If you really consider this unsafe, then you can deny loading programs
> with relocations ...
This would require a LSM to inspect BPF programs and maps at load
time, something Alexei has previously rejected, are you now saying
that this would be acceptable?
https://lore.kernel.org/linux-security-module/CAADnVQJyNRZVLPj_nzegCyo+BzM1-whbnajotCXu+GW+5-=P6w@mail.gmail.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: KP Singh @ 2025-10-23 15:39 UTC (permalink / raw)
To: James Bottomley
Cc: Alexei Starovoitov, Paul Moore, Linus Torvalds,
Alexei Starovoitov, Blaise Boscaccy, bpf, LSM List,
K. Y. Srinivasan, Daniel Borkmann, Andrii Nakryiko, wufan,
Quentin Monnet
In-Reply-To: <b21284e338846166804bd99bfc37186cf80f1b38.camel@HansenPartnership.com>
On Wed, Oct 22, 2025 at 11:10 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Mon, 2025-10-20 at 18:25 -0700, Alexei Starovoitov wrote:
> > On Mon, Oct 20, 2025 at 4:13 PM James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:
> [...]
> > > The point, for me, is when doing integrity tests both patch sets
> > > produce identical results and correctly detect when integrity of a
> > > light skeleton is compromised (in mathematical terms that means
> > > they're functionally equivalent). The only difference is that with
> > > Blaise's patch set verification completes before the LSM load hook
> > > is called and with KP's it completes after ... and the security
> > > problem with the latter case is that there's no LSM hook to collect
> > > the verification result.
> >
> > the security problem with KP's approach? wtf.
> > I'm going to add "depends on !microsoft" to kconfig bpf_syscall
> > and be done with it.
> > Don't use it since it's so insecure.
>
> Most Linux installations use LSMs to enforce and manage policies for
> system integrity (they don't all use the same set of LSMs, but that's
> not relevant to the argument). So while Meta may not use LSMs for
> system integrity the fact that practically everyone else does makes not
> having a correctly functioning LSM hook for BPF signature verification
> a problem for a huge set of users that goes way beyond just Microsoft.
>
The core tenet of your claim is that you need "LSM observability" but
without any description of a security policy
that cannot not be currently implemented. The responses I have
received are generic statements that the loader verification is
"unsafe"
If you really consider this unsafe, then you can deny loading programs
with relocations and re-enable them when / if we achieve stable
instruction buffers. To be honest, with this restriction of all
signature verification happening in the kernel you also need to deny
key real-world BPF use-cases like Cilium, bpftrace which generate eBPF
programs on the target host which also shows me how out of touch you
are with the eBPF eco-system and users.
- KP
> Regards,
>
> James
>
^ permalink raw reply
* [PATCH 2/2] keys: Remove unnecessary local variable from proc_keys_show
From: Thorsten Blum @ 2025-10-23 14:32 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn
Cc: Thorsten Blum, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251023143231.2086-2-thorsten.blum@linux.dev>
The local variable 'rc' is only used to temporary store the result of
calling key_task_permission(). Use the result directly and remove the
local variable.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/keys/proc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/security/keys/proc.c b/security/keys/proc.c
index 4f4e2c1824f1..39af57af2aad 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -160,7 +160,6 @@ static int proc_keys_show(struct seq_file *m, void *v)
char xbuf[16];
short state;
u64 timo;
- int rc;
struct keyring_search_context ctx = {
.index_key = key->index_key,
@@ -188,8 +187,7 @@ static int proc_keys_show(struct seq_file *m, void *v)
}
/* check whether the current task is allowed to view the key */
- rc = key_task_permission(key_ref, ctx.cred, KEY_NEED_VIEW);
- if (rc < 0)
+ if (key_task_permission(key_ref, ctx.cred, KEY_NEED_VIEW) < 0)
return 0;
now = ktime_get_real_seconds();
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] KEYS: encrypted: Use designated initializers for match_table_t structs
From: Roberto Sassu @ 2025-10-23 13:35 UTC (permalink / raw)
To: James Bottomley, Thorsten Blum
Cc: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, linux-integrity, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <e60f6a07d00c1fd87b4509947e8738ecab9560b4.camel@HansenPartnership.com>
On Thu, 2025-10-09 at 09:51 -0400, James Bottomley wrote:
> On Thu, 2025-10-09 at 15:30 +0200, Thorsten Blum wrote:
> > On 9. Oct 2025, at 14:44, James Bottomley wrote:
> > > On Thu, 2025-10-09 at 13:58 +0200, Thorsten Blum wrote:
> > > > Use designated initializers for 'key_format_tokens' and
> > > > 'key_tokens' to allow struct fields to be reordered more easily
> > >
> > > How does it improve that? The key,value pairs are surrounded by
> > > braces so we just cut and paste the lot anyway.
> >
> > Using designated initializers (especially for global structs) allows
> > the fields of struct match_token from linux/parser.h to be reordered
> > or extended more easily, improving overall maintainability.
>
> Why would we ever want to reorder them? The reason the ordering is
> {token, parser} string is because that's the nicest order to read them
> in.
I also join James regarding this. I find it fine as it is.
Consider also that there might be patches depending on this change that
cannot be automatically ported to stable kernels. Then, extra work is
required to find which dependencies are needed and backport them as
well.
Thanks
Roberto
> > > > and to improve readability.
> > >
> > > I don't think I agree with this when looking through the code,
> > > especially because this is the way it's done for *every* option in
> > > the entire key subsystem. So firstly I really don't think it's
> > > helpful for only encrypted keys to be different from everything
> > > else and secondly when I read the code (as I often do to figure out
> > > what the options mean), the additional .token and .pattern just get
> > > in the way of what I'm looking for.
> >
> > I just stumbled upon this and didn't check any other files.
>
> jejb@lingrow:~/git/linux> git grep 'match_table_t'|wc -l
> 49
>
> I'll leave it as an exercise to you to figure out how many use the
> style you're proposing.
>
> There's definite advantage in uniformity and even if I accepted the
> readability argument, which I don't, it's too small a reason to churn
> nearly 50 files one at a time.
>
> Regards,
>
> James
>
^ permalink raw reply
* Re: [PATCH v5 0/34] Rework the LSM initialization
From: Paul Moore @ 2025-10-22 23:34 UTC (permalink / raw)
To: linux-security-module, linux-integrity, selinux
Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
Mickaël Salaün, Günther Noack, Kees Cook,
Micah Morton, Casey Schaufler, Tetsuo Handa, Nicolas Bouchinet,
Xiu Jianfeng
In-Reply-To: <20251017202456.484010-36-paul@paul-moore.com>
On Fri, Oct 17, 2025 at 4:28 PM Paul Moore <paul@paul-moore.com> wrote:
>
> This is the fifth, and likely final, revision of the LSM rework patchset.
> The number of changes in this revision are very minor and barring any
> surprises I expect to merge this into the lsm/dev branch next week; I'll
> send a notice when I do.
Here is that notice. This patchset is now merged into lsm/dev and
should be in the next linux-next release; if anyone notices anything
odd, please let me know.
As a FYI, I also moved the base of lsm/dev up to v6.18-rc2 to grab the
fix below (it was affecting testing).
https://lore.kernel.org/netdev/20251015052715.4140493-1-edumazet@google.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: James Bottomley @ 2025-10-22 21:10 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Paul Moore, Linus Torvalds, Alexei Starovoitov, KP Singh,
Blaise Boscaccy, bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann,
Andrii Nakryiko, wufan, Quentin Monnet
In-Reply-To: <CAADnVQKcOS8iu0Nq5aYg+Lg_EAO8fFde0H3w8t0m_SXUy4iKAA@mail.gmail.com>
On Mon, 2025-10-20 at 18:25 -0700, Alexei Starovoitov wrote:
> On Mon, Oct 20, 2025 at 4:13 PM James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
[...]
> > The point, for me, is when doing integrity tests both patch sets
> > produce identical results and correctly detect when integrity of a
> > light skeleton is compromised (in mathematical terms that means
> > they're functionally equivalent). The only difference is that with
> > Blaise's patch set verification completes before the LSM load hook
> > is called and with KP's it completes after ... and the security
> > problem with the latter case is that there's no LSM hook to collect
> > the verification result.
>
> the security problem with KP's approach? wtf.
> I'm going to add "depends on !microsoft" to kconfig bpf_syscall
> and be done with it.
> Don't use it since it's so insecure.
Most Linux installations use LSMs to enforce and manage policies for
system integrity (they don't all use the same set of LSMs, but that's
not relevant to the argument). So while Meta may not use LSMs for
system integrity the fact that practically everyone else does makes not
having a correctly functioning LSM hook for BPF signature verification
a problem for a huge set of users that goes way beyond just Microsoft.
Regards,
James
^ permalink raw reply
* [PATCH] apparmor: replace sprintf with snprintf in aa_new_learning_profile
From: Thorsten Blum @ 2025-10-22 9:37 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel
Replace unbounded sprintf() calls with snprintf() to prevent potential
buffer overflows in aa_new_learning_profile(). While the current code
works correctly, snprintf() is safer and follows secure coding best
practices. No functional changes.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/policy.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 50d5345ff5cb..b09323867fea 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -697,24 +697,27 @@ struct aa_profile *aa_new_learning_profile(struct aa_profile *parent, bool hat,
struct aa_profile *p, *profile;
const char *bname;
char *name = NULL;
+ size_t name_sz;
AA_BUG(!parent);
if (base) {
- name = kmalloc(strlen(parent->base.hname) + 8 + strlen(base),
- gfp);
+ name_sz = strlen(parent->base.hname) + 8 + strlen(base);
+ name = kmalloc(name_sz, gfp);
if (name) {
- sprintf(name, "%s//null-%s", parent->base.hname, base);
+ snprintf(name, name_sz, "%s//null-%s",
+ parent->base.hname, base);
goto name;
}
/* fall through to try shorter uniq */
}
- name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, gfp);
+ name_sz = strlen(parent->base.hname) + 2 + 7 + 8;
+ name = kmalloc(name_sz, gfp);
if (!name)
return NULL;
- sprintf(name, "%s//null-%x", parent->base.hname,
- atomic_inc_return(&parent->ns->uniq_null));
+ snprintf(name, name_sz, "%s//null-%x", parent->base.hname,
+ atomic_inc_return(&parent->ns->uniq_null));
name:
/* lookup to see if this is a dup creation */
--
2.51.0
^ permalink raw reply related
* Zwrot
From: Eryk Wawrzyn @ 2025-10-22 8:11 UTC (permalink / raw)
To: linux-security-module
Dzień dobry,
kontaktuję się w imieniu kancelarii specjalizującej się w zarządzaniu wierzytelnościami.
Od lat wspieramy firmy w odzyskiwaniu należności. Prowadzimy kompleksową obsługę na etapach: przedsądowym, sądowym i egzekucyjnym, dostosowując działania do branży Klienta.
Kiedy możemy porozmawiać?
Pozdrawiam
Eryk Wawrzyn
^ permalink raw reply
* Re: [PATCH v2] x86/bpf: do not audit capability check in do_jit()
From: patchwork-bot+netdevbpf @ 2025-10-22 1:30 UTC (permalink / raw)
To: Ondrej Mosnacek; +Cc: ast, daniel, bpf, selinux, linux-security-module, serge
In-Reply-To: <20251021122758.2659513-1-omosnace@redhat.com>
Hello:
This patch was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Tue, 21 Oct 2025 14:27:58 +0200 you wrote:
> The failure of this check only results in a security mitigation being
> applied, slightly affecting performance of the compiled BPF program. It
> doesn't result in a failed syscall, an thus auditing a failed LSM
> permission check for it is unwanted. For example with SELinux, it causes
> a denial to be reported for confined processes running as root, which
> tends to be flagged as a problem to be fixed in the policy. Yet
> dontauditing or allowing CAP_SYS_ADMIN to the domain may not be
> desirable, as it would allow/silence also other checks - either going
> against the principle of least privilege or making debugging potentially
> harder.
>
> [...]
Here is the summary with links:
- [v2] x86/bpf: do not audit capability check in do_jit()
https://git.kernel.org/bpf/bpf/c/881a9c9cb785
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v2] x86/bpf: do not audit capability check in do_jit()
From: Paul Moore @ 2025-10-21 16:22 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, selinux,
linux-security-module, Serge E . Hallyn
In-Reply-To: <20251021122758.2659513-1-omosnace@redhat.com>
On Tue, Oct 21, 2025 at 8:28 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> The failure of this check only results in a security mitigation being
> applied, slightly affecting performance of the compiled BPF program. It
> doesn't result in a failed syscall, an thus auditing a failed LSM
> permission check for it is unwanted. For example with SELinux, it causes
> a denial to be reported for confined processes running as root, which
> tends to be flagged as a problem to be fixed in the policy. Yet
> dontauditing or allowing CAP_SYS_ADMIN to the domain may not be
> desirable, as it would allow/silence also other checks - either going
> against the principle of least privilege or making debugging potentially
> harder.
>
> Fix it by changing it from capable() to ns_capable_noaudit(), which
> instructs the LSMs to not audit the resulting denials.
>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2369326
> Fixes: d4e89d212d40 ("x86/bpf: Call branch history clearing sequence on exit")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>
> v1: https://lore.kernel.org/selinux/20250806143105.915748-1-omosnace@redhat.com/
> Changes in v2:
> - just silence the audit records instead of switching to bpf_capable()
>
> arch/x86/net/bpf_jit_comp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox