* Re: [PATCH RFC 3/15] Audit: Add record for multiple task security contexts
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-4-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Replace the single skb pointer in an audit_buffer with a list of
> skb pointers. Add the audit_stamp information to the audit_buffer as
> there's no guarantee that there will be an audit_context containing
> the stamp associated with the event. At audit_log_end() time create
> auxiliary records as have been added to the list. Functions are
> created to manage the skb list in the audit_buffer.
>
> Create a new audit record AUDIT_MAC_TASK_CONTEXTS.
> An example of the MAC_TASK_CONTEXTS record is:
>
> type=MAC_TASK_CONTEXTS
> msg=audit(1600880931.832:113)
> subj_apparmor=unconfined
> subj_smack=_
>
> When an audit event includes a AUDIT_MAC_TASK_CONTEXTS record the
> "subj=" field in other records in the event will be "subj=?".
> An AUDIT_MAC_TASK_CONTEXTS record is supplied when the system has
> multiple security modules that may make access decisions based on a
> subject security context.
>
> Refactor audit_log_task_context(), creating a new audit_log_subj_ctx().
> This is used in netlabel auditing to provide multiple subject security
> contexts as necessary.
>
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/audit.h | 16 +++
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 207 +++++++++++++++++++++++++++++------
> net/netlabel/netlabel_user.c | 9 +-
> security/apparmor/lsm.c | 3 +
> security/lsm.h | 4 -
> security/lsm_init.c | 5 -
> security/security.c | 3 -
> security/selinux/hooks.c | 3 +
> security/smack/smack_lsm.c | 3 +
> 10 files changed, 202 insertions(+), 52 deletions(-)
Similar to patch 1/15, dropped due to this already being in Linus'
tree.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 4/15] Audit: Add record for multiple object contexts
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-5-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Create a new audit record AUDIT_MAC_OBJ_CONTEXTS.
> An example of the MAC_OBJ_CONTEXTS record is:
>
> type=MAC_OBJ_CONTEXTS
> msg=audit(1601152467.009:1050):
> obj_selinux=unconfined_u:object_r:user_home_t:s0
>
> When an audit event includes a AUDIT_MAC_OBJ_CONTEXTS record
> the "obj=" field in other records in the event will be "obj=?".
> An AUDIT_MAC_OBJ_CONTEXTS record is supplied when the system has
> multiple security modules that may make access decisions based
> on an object security context.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/audit.h | 7 +++++
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 58 +++++++++++++++++++++++++++++++++++++-
> kernel/auditsc.c | 45 ++++++++---------------------
> security/selinux/hooks.c | 3 +-
> security/smack/smack_lsm.c | 3 +-
> 6 files changed, 80 insertions(+), 37 deletions(-)
Similar to patch 1/15, dropped due to this already being in Linus'
tree.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 5/15] LSM: Single calls in secid hooks
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-6-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> security_socket_getpeersec_stream(), security_socket_getpeersec_dgram()
> and security_secctx_to_secid() can only provide a single security context
> or secid to their callers. Open code these hooks to return the first
> hook provided. Because only one "major" LSM is allowed there will only
> be one hook in the list, with the excepton being BPF. BPF is not expected
> to be using these interfaces.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/security.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index db85006d2fd5..2286285f8aea 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -3806,8 +3806,13 @@ EXPORT_SYMBOL(security_lsmprop_to_secctx);
> */
> int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
> {
> + struct lsm_static_call *scall;
> +
> *secid = 0;
> - return call_int_hook(secctx_to_secid, secdata, seclen, secid);
> + lsm_for_each_hook(scall, secctx_to_secid) {
> + return scall->hl->hook.secctx_to_secid(secdata, seclen, secid);
> + }
> + return LSM_RET_DEFAULT(secctx_to_secid);
> }
> EXPORT_SYMBOL(security_secctx_to_secid);
Two thoughts come to mind:
If we are relying on BPF not using these hooks we should remove the BPF
callback. It looks like the secctx_to_secid and socket_getpeersec_stream
callbacks are already absent from the BPF LSM, so it's just a matter of
working with the BPF folks to see if socket_getpeersec_dgram can be
removed. If it can't be removed, you'll need to find another solution.
Instead of opening up the call_int_hook() wrapper here, what would it
look like if we enforced the single callback rule at LSM registration
time?
> @@ -4268,8 +4273,13 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
> int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
> sockptr_t optlen, unsigned int len)
> {
> - return call_int_hook(socket_getpeersec_stream, sock, optval, optlen,
> - len);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, socket_getpeersec_stream) {
> + return scall->hl->hook.socket_getpeersec_stream(sock, optval,
> + optlen, len);
> + }
> + return LSM_RET_DEFAULT(socket_getpeersec_stream);
> }
>
> /**
> @@ -4289,7 +4299,13 @@ int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
> int security_socket_getpeersec_dgram(struct socket *sock,
> struct sk_buff *skb, u32 *secid)
> {
> - return call_int_hook(socket_getpeersec_dgram, sock, skb, secid);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, socket_getpeersec_dgram) {
> + return scall->hl->hook.socket_getpeersec_dgram(sock, skb,
> + secid);
> + }
> + return LSM_RET_DEFAULT(socket_getpeersec_dgram);
> }
> EXPORT_SYMBOL(security_socket_getpeersec_dgram);
>
> --
> 2.47.0
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 6/15] LSM: Exclusive secmark usage
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-7-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> The network secmark can only be used by one security module
> at a time. Establish mechanism to identify to security modules
> whether they have access to the secmark. SELinux already
> incorparates mechanism, but it has to be added to Smack and
> AppArmor.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/apparmor/include/net.h | 5 +++++
> security/apparmor/lsm.c | 7 ++++---
> security/lsm_init.c | 6 ++++++
> security/selinux/hooks.c | 4 +++-
> security/smack/smack.h | 5 +++++
> security/smack/smack_lsm.c | 3 ++-
> security/smack/smack_netfilter.c | 10 ++++++++--
> 8 files changed, 34 insertions(+), 7 deletions(-)
We discussed this patch in a separate patchset, lore link below.
https://lore.kernel.org/linux-security-module/20251001215643.31465-1-casey@schaufler-ca.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 7/15] Audit: Call only the first of the audit rule hooks
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-8-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> The audit system is not (yet) capable for distinguishing
> between audit rules specified for multiple security modules.
> Call only the first registered of the audit rule hooks.
> The order of registration, which can be specified with the
> lsm= boot parameter, is hence an important consideration.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/security.c | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index 2286285f8aea..93d4ac39fe9f 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5056,7 +5056,13 @@ void security_key_post_create_or_update(struct key *keyring, struct key *key,
> int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule,
> gfp_t gfp)
> {
> - return call_int_hook(audit_rule_init, field, op, rulestr, lsmrule, gfp);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, audit_rule_init) {
> + return scall->hl->hook.audit_rule_init(field, op, rulestr,
> + lsmrule, gfp);
> + }
> + return LSM_RET_DEFAULT(audit_rule_init);
> }
Similar to the comments in patch 5/15, what would it look like to do the
enforcement of this callback restriction at LSM registration time?
> /**
> @@ -5070,7 +5076,12 @@ int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule,
> */
> int security_audit_rule_known(struct audit_krule *krule)
> {
> - return call_int_hook(audit_rule_known, krule);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, audit_rule_known) {
> + return scall->hl->hook.audit_rule_known(krule);
> + }
> + return LSM_RET_DEFAULT(audit_rule_known);
> }
>
> /**
> @@ -5082,7 +5093,12 @@ int security_audit_rule_known(struct audit_krule *krule)
> */
> void security_audit_rule_free(void *lsmrule)
> {
> - call_void_hook(audit_rule_free, lsmrule);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, audit_rule_free) {
> + scall->hl->hook.audit_rule_free(lsmrule);
> + return;
> + }
> }
>
> /**
> @@ -5101,7 +5117,13 @@ void security_audit_rule_free(void *lsmrule)
> int security_audit_rule_match(struct lsm_prop *prop, u32 field, u32 op,
> void *lsmrule)
> {
> - return call_int_hook(audit_rule_match, prop, field, op, lsmrule);
> + struct lsm_static_call *scall;
> +
> + lsm_for_each_hook(scall, audit_rule_match) {
> + return scall->hl->hook.audit_rule_match(prop, field, op,
> + lsmrule);
> + }
> + return LSM_RET_DEFAULT(audit_rule_match);
> }
> #endif /* CONFIG_AUDIT */
>
> --
> 2.47.0
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 9/15] LSM: Add mount opts blob size tracking
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-10-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Add mount option data to the blob size accounting in anticipation
> of using a shared mnt_opts blob.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/lsm_init.c | 2 ++
> security/selinux/hooks.c | 1 +
> security/smack/smack_lsm.c | 1 +
> 4 files changed, 5 insertions(+)
We discussed this patch in a separate patchset, lore link below.
https://lore.kernel.org/linux-security-module/20250925171208.5997-1-casey@schaufler-ca.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 10/15] LSM: allocate mnt_opts blobs instead of module specific data
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-11-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Replace allocations of LSM specific mount data with the
> shared mnt_opts blob.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/security.c | 12 ++++++++++++
> security/selinux/hooks.c | 10 +++++++---
> security/smack/smack_lsm.c | 4 ++--
> 4 files changed, 22 insertions(+), 5 deletions(-)
We discussed the concepts in this patch in a separate patchset, lore link below.
https://lore.kernel.org/linux-security-module/20250925171208.5997-1-casey@schaufler-ca.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 11/15] LSM: Infrastructure management of the mnt_opts security blob
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-12-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Move management of the mnt_opts->security blob out of the
> individual security modules and into the security
> infrastructure. Blobs are still allocated within the modules
> as they are only required when mount options are present.
> The modules tell the infrastructure how much space is required,
> and the space is allocated if needed. Modules can no longer
> count on the presence of a blob implying that mount options
> specific to that module are present, so flags are added
> to the module specific blobs to indicate that this module
> has options.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/security.c | 14 ++++-----
> security/selinux/hooks.c | 58 +++++++++++++++++++++++-------------
> security/smack/smack_lsm.c | 61 ++++++++++++++++++++++++++------------
> 3 files changed, 85 insertions(+), 48 deletions(-)
We discussed this patch in a separate patchset, lore link below.
https://lore.kernel.org/linux-security-module/20250925171208.5997-1-casey@schaufler-ca.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 12/15] LSM: Allow reservation of netlabel
From: Paul Moore @ 2025-10-14 23:12 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-13-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Allow LSMs to request exclusive access to the netlabel facility.
> Provide mechanism for LSMs to determine if they have access to
> netlabel. Update the current users of netlabel, SELinux and Smack,
> to use and respect the exclusive use of netlabel.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hooks.h | 1 +
> security/lsm_init.c | 6 +++++
> security/selinux/hooks.c | 7 +++---
> security/selinux/include/netlabel.h | 5 ++++
> security/selinux/netlabel.c | 4 ++--
> security/smack/smack.h | 5 ++++
> security/smack/smack_lsm.c | 36 +++++++++++++++++++++--------
> security/smack/smackfs.c | 20 +++++++++++++++-
> 8 files changed, 69 insertions(+), 15 deletions(-)
We discussed this patch in a separate patchset, lore link below.
https://lore.kernel.org/linux-security-module/20251001215643.31465-1-casey@schaufler-ca.com/
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 13/15] LSM: restrict security_cred_getsecid() to a single LSM
From: Paul Moore @ 2025-10-14 23:13 UTC (permalink / raw)
To: Casey Schaufler, casey, eparis, linux-security-module, audit
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, linux-kernel, selinux
In-Reply-To: <20250621171851.5869-14-casey@schaufler-ca.com>
On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> The LSM hook security_cred_getsecid() provides a single secid
> that is only used by the binder driver. Provide the first value
> available, and abandon any other hooks.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> security/security.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/security/security.c b/security/security.c
> index dd167a872248..409b1cb10d35 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2740,8 +2740,13 @@ void security_transfer_creds(struct cred *new, const struct cred *old)
> */
> void security_cred_getsecid(const struct cred *c, u32 *secid)
> {
> + struct lsm_static_call *scall;
> +
> *secid = 0;
> - call_void_hook(cred_getsecid, c, secid);
> + lsm_for_each_hook(scall, cred_getsecid) {
> + scall->hl->hook.cred_getsecid(c, secid);
> + break;
> + }
> }
> EXPORT_SYMBOL(security_cred_getsecid);
Another case of "would this be painful to do at registration time?"
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH RFC 1/15] Audit: Create audit_stamp structure
From: Casey Schaufler @ 2025-10-15 5:18 UTC (permalink / raw)
To: Paul Moore; +Cc: LSM List
In-Reply-To: <846555fc86ec02df31f55935e747a71f@paul-moore.com>
On 10/14/2025 4:12 PM, Paul Moore wrote:
> On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Replace the timestamp and serial number pair used in audit records
>> with a structure containing the two elements.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> kernel/audit.c | 17 +++++++++--------
>> kernel/audit.h | 13 +++++++++----
>> kernel/auditsc.c | 22 +++++++++-------------
>> 3 files changed, 27 insertions(+), 25 deletions(-)
> Dropped as this patch was merged into Linus' tree during the v6.18
> merge window via another patchset. To be clear, I generally don't have
> a problem with multiple patchsets including a few common patches, it
> helps prevent cross-dependencies between patchsets which is a good
> thing.
>
> --
> paul-moore.com
I'm off-grid for the rest of the month. Will respond to these many things
upon my return.
^ permalink raw reply
* Re: [PATCH] keys: Remove redundant less-than-zero checks
From: Jarkko Sakkinen @ 2025-10-15 11:02 UTC (permalink / raw)
To: Thorsten Blum, David Howells
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, James Bottomley, keyrings, linux-security-module,
linux-kernel, linux-integrity
In-Reply-To: <20251011144824.1257-2-thorsten.blum@linux.dev>
On Sat, Oct 11, 2025 at 04:48:24PM +0200, Thorsten Blum wrote:
> The local variables 'size_t datalen' are unsigned and cannot be less
> than zero. Remove the redundant conditions.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
David, I've applied this.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v3] keys: Replace deprecated strncpy in ecryptfs_fill_auth_tok
From: Jarkko Sakkinen @ 2025-10-15 11:06 UTC (permalink / raw)
To: Paul Menzel, David Howells
Cc: Thorsten Blum, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, linux-hardening, Kees Cook,
linux-integrity, keyrings, linux-security-module, linux-kernel
In-Reply-To: <b9246396-c9d9-4452-a16c-f2c8166a32ee@molgen.mpg.de>
On Mon, Oct 13, 2025 at 06:11:38PM +0200, Paul Menzel wrote:
> Dear Thorsten,
>
>
> Thank you for the patch.
>
> Am 13.10.25 um 17:26 schrieb Thorsten Blum:
> > strncpy() is deprecated for NUL-terminated destination buffers; use
> > strscpy_pad() instead to retain the NUL-padding behavior of strncpy().
> >
> > The destination buffer is initialized using kzalloc() with a 'signature'
> > size of ECRYPTFS_PASSWORD_SIG_SIZE + 1. strncpy() then copies up to
> > ECRYPTFS_PASSWORD_SIG_SIZE bytes from 'key_desc', NUL-padding any
> > remaining bytes if needed, but expects the last byte to be zero.
> >
> > strscpy_pad() also copies the source string to 'signature', and NUL-pads
> > the destination buffer if needed, but ensures it's always NUL-terminated
> > without relying on it being zero-initialized.
> >
> > strscpy_pad() automatically determines the size of the fixed-length
> > destination buffer via sizeof() when the optional size argument is
> > omitted, making an explicit size unnecessary.
> >
> > In encrypted_init(), the source string 'key_desc' is validated by
> > valid_ecryptfs_desc() before calling ecryptfs_fill_auth_tok(), and is
> > therefore NUL-terminated and satisfies the __must_be_cstr() requirement
> > of strscpy_pad().
> >
> > Link: https://github.com/KSPP/linux/issues/90
> > Reviewed-by: Kees Cook <kees@kernel.org>
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> > Changes in v3:
> > - Improve commit message
> > - Link to v2: https://lore.kernel.org/lkml/20251010161340.458707-2-thorsten.blum@linux.dev/
> >
> > Changes in v2:
> > - Improve commit message as suggested by Jarkko and Kees
> > - Link to v1: https://lore.kernel.org/lkml/20251009180316.394708-3-thorsten.blum@linux.dev/
> > ---
> > security/keys/encrypted-keys/ecryptfs_format.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/security/keys/encrypted-keys/ecryptfs_format.c b/security/keys/encrypted-keys/ecryptfs_format.c
> > index 8fdd76105ce3..2fc6f3a66135 100644
> > --- a/security/keys/encrypted-keys/ecryptfs_format.c
> > +++ b/security/keys/encrypted-keys/ecryptfs_format.c
> > @@ -54,8 +54,7 @@ int ecryptfs_fill_auth_tok(struct ecryptfs_auth_tok *auth_tok,
> > auth_tok->version = (((uint16_t)(major << 8) & 0xFF00)
> > | ((uint16_t)minor & 0x00FF));
> > auth_tok->token_type = ECRYPTFS_PASSWORD;
> > - strncpy((char *)auth_tok->token.password.signature, key_desc,
> > - ECRYPTFS_PASSWORD_SIG_SIZE);
> > + strscpy_pad(auth_tok->token.password.signature, key_desc);
> > auth_tok->token.password.session_key_encryption_key_bytes =
> > ECRYPTFS_MAX_KEY_BYTES;
> > /*
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Thanks for the review Paul.
David: and also this is in my tree now.
>
>
> Kind regards,
>
> Paul
BR, Jarkko
^ permalink raw reply
* Re: [PATCH RFC 1/15] Audit: Create audit_stamp structure
From: Paul Moore @ 2025-10-15 15:01 UTC (permalink / raw)
To: Casey Schaufler; +Cc: LSM List
In-Reply-To: <1dad4179-d133-41ea-a76c-569a2f92fcbe@schaufler-ca.com>
On Wed, Oct 15, 2025 at 1:20 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 10/14/2025 4:12 PM, Paul Moore wrote:
> > On Jun 21, 2025 Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> Replace the timestamp and serial number pair used in audit records
> >> with a structure containing the two elements.
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >> kernel/audit.c | 17 +++++++++--------
> >> kernel/audit.h | 13 +++++++++----
> >> kernel/auditsc.c | 22 +++++++++-------------
> >> 3 files changed, 27 insertions(+), 25 deletions(-)
> >
> > Dropped as this patch was merged into Linus' tree during the v6.18
> > merge window via another patchset. To be clear, I generally don't have
> > a problem with multiple patchsets including a few common patches, it
> > helps prevent cross-dependencies between patchsets which is a good
> > thing.
>
> I'm off-grid for the rest of the month. Will respond to these many things
> upon my return.
No worries, enjoy your time away and thanks for letting us know.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH kvm-next V11 0/7] Add NUMA mempolicy support for KVM guest-memfd
From: Sean Christopherson @ 2025-10-15 18:02 UTC (permalink / raw)
To: Sean Christopherson, willy, akpm, david, pbonzini, shuah, vbabka,
Shivank Garg
Cc: brauner, viro, dsterba, xiang, chao, jaegeuk, clm, josef,
kent.overstreet, zbestahu, jefflexu, dhavale, lihongbo22,
lorenzo.stoakes, Liam.Howlett, rppt, surenb, mhocko, ziy,
matthew.brost, joshua.hahnjy, rakie.kim, byungchul, gourry,
ying.huang, apopple, tabba, ackerleytng, paul, jmorris, serge,
pvorel, bfoster, vannapurve, chao.gao, bharata, nikunj,
michael.day, shdhiman, yan.y.zhao, Neeraj.Upadhyay,
thomas.lendacky, michael.roth, aik, kalyazin, peterx, jack, hch,
cgzones, ira.weiny, rientjes, roypat, chao.p.peng, amit, ddutile,
dan.j.williams, ashish.kalra, gshan, jgowans, pankaj.gupta,
papaluri, yuzhao, suzuki.poulose, quic_eberman, linux-bcachefs,
linux-btrfs, linux-erofs, linux-f2fs-devel, linux-fsdevel,
linux-mm, linux-kernel, linux-security-module, kvm,
linux-kselftest, linux-coco, Jason Gunthorpe
In-Reply-To: <20250827175247.83322-2-shivankg@amd.com>
On Wed, 27 Aug 2025 17:52:41 +0000, Shivank Garg wrote:
> This series introduces NUMA-aware memory placement support for KVM guests
> with guest_memfd memory backends. It builds upon Fuad Tabba's work (V17)
> that enabled host-mapping for guest_memfd memory [1] and can be applied
> directly applied on KVM tree [2] (branch kvm-next, base commit: a6ad5413,
> Merge branch 'guest-memfd-mmap' into HEAD)
>
> == Background ==
> KVM's guest-memfd memory backend currently lacks support for NUMA policy
> enforcement, causing guest memory allocations to be distributed across host
> nodes according to kernel's default behavior, irrespective of any policy
> specified by the VMM. This limitation arises because conventional userspace
> NUMA control mechanisms like mbind(2) don't work since the memory isn't
> directly mapped to userspace when allocations occur.
> Fuad's work [1] provides the necessary mmap capability, and this series
> leverages it to enable mbind(2).
>
> [...]
Applied the non-KVM change to kvm-x86 gmem. We're still tweaking and iterating
on the KVM changes, but I fully expect them to land in 6.19.
Holler if you object to taking these through the kvm tree.
[1/7] mm/filemap: Add NUMA mempolicy support to filemap_alloc_folio()
https://github.com/kvm-x86/linux/commit/601aa29f762f
[2/7] mm/filemap: Extend __filemap_get_folio() to support NUMA memory policies
https://github.com/kvm-x86/linux/commit/2bb25703e5bd
[3/7] mm/mempolicy: Export memory policy symbols
https://github.com/kvm-x86/linux/commit/e1b4cf7d6be3
--
https://github.com/kvm-x86/linux/tree/next
^ permalink raw reply
* Re: [PATCH v2 1/6] landlock: Add a place for flags to layer rules
From: Mickaël Salaün @ 2025-10-15 19:07 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <feccc42da1c32b81e07ccd7387564d4bba8a06c1.1759686613.git.m@maowtm.org>
On Sun, Oct 05, 2025 at 06:55:24PM +0100, Tingmao Wang wrote:
> 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>
> ---
>
> Note: conflict with 9a868cdbe6 ("landlock: Fix handling of disconnected
> directories") on mic/next but is not hard to fix - need to backup the
> collected rule flags too.
>
> Changes since v1:
> - Comment changes
>
> security/landlock/fs.c | 75 ++++++++++++++++++++++++-------------
> security/landlock/net.c | 3 +-
> security/landlock/ruleset.c | 9 ++++-
> security/landlock/ruleset.h | 26 ++++++++++++-
> 4 files changed, 82 insertions(+), 31 deletions(-)
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index c04f8879ad03..e7eaf55093e9 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -756,10 +756,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)
> {
> @@ -810,22 +812,30 @@ static bool is_access_to_paths_allowed(
> }
>
> if (unlikely(dentry_child1)) {
> + /*
> + * The rule_flags for child1 should have been included in
> + * rule_flags_masks_parent1 already. We do not bother about it
rule_flags_parent1
> + * for domain check.
The main point about not setting rule_flags_parent* here and below is
because the purpose of this few lines is to get the layer masks for the
potential child dentries and compare them. Please improve the comment
with this explanation too.
> + */
> 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);
> }
> @@ -881,16 +891,18 @@ 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, 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)
> @@ -958,6 +970,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 collected_rule_flags rule_flags = {};
Why add it after the following variable?
> struct landlock_request request = {};
>
> if (!subject)
> @@ -967,8 +980,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);
> @@ -1032,7 +1045,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 dentry *const mnt_root, 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)
> {
> unsigned long access_dom;
> bool ret = false;
> @@ -1051,9 +1065,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)) {
> /*
> * Stops when all handled accesses are allowed by at
> * least one rule in each layer.
> @@ -1140,6 +1154,8 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> struct dentry *old_parent;
> layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {},
> layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {};
> + struct collected_rule_flags rule_flags_parent1 = {},
> + rule_flags_parent2 = {};
> struct landlock_request request1 = {}, request2 = {};
>
> if (!subject)
> @@ -1172,10 +1188,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);
> @@ -1201,10 +1217,12 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> /* new_dir->dentry is equal to new_dentry->d_parent */
> allow_parent1 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
> old_parent,
> - &layer_masks_parent1);
> + &layer_masks_parent1,
> + &rule_flags_parent1);
> allow_parent2 = collect_domain_accesses(subject->domain, mnt_dir.dentry,
> new_dir->dentry,
> - &layer_masks_parent2);
> + &layer_masks_parent2,
> + &rule_flags_parent2);
>
> if (allow_parent1 && allow_parent2)
> return 0;
> @@ -1217,8 +1235,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;
>
> @@ -1616,6 +1635,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 collected_rule_flags rule_flags = {};
Ditto, please move after the other local variables.
> access_mask_t open_access_request, full_access_request, allowed_access,
> optional_access;
> const struct landlock_cred_security *const subject =
> @@ -1647,7 +1667,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 ce7940efea51..3aa4e33ac95b 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -616,7 +616,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;
>
> @@ -643,6 +644,12 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
> unsigned long access_bit;
> bool is_empty;
>
> + if (rule_flags) {
> + /* Collect rule flags for each layer */
> + if (layer->flags.quiet)
> + rule_flags->quiet_masks |= layer_bit;
> + }
> +
> /*
> * Records in @layer_masks which layer grants access to each
> * requested access.
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 5da9a64f5af7..eeee287a9508 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.
> */
> - 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.0
>
^ permalink raw reply
* Re: [PATCH v2 2/6] landlock: Add API support and docs for the quiet flags
From: Mickaël Salaün @ 2025-10-15 19:08 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, Jan Kara, linux-security-module,
Abhinav Saxena
In-Reply-To: <e32cef442528bd99e3348e3149cec71b9e399e8c.1759686613.git.m@maowtm.org>
On Sun, Oct 05, 2025 at 06:55:25PM +0100, Tingmao Wang wrote:
> As for kselftests, for now we just change add_rule_checks_ordering to use
> a different invalid flag. I will add tests for the quiet flag in a later
> version.
>
> According to pahole, even after adding the struct access_masks quiet_masks
> in struct landlock_hierarchy, the u32 log_* bitfield still only has a size
> of 2 bytes, so there's minimal wasted space.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> Changes since v1:
> - Per suggestion, added support for quieting only certain access bits,
> controlled by extra quiet_access_* fields in the ruleset_attr.
> - Added docs for the extra fields and made updates to doc changes in v1.
> In particular, call out that the effect of LANDLOCK_ADD_RULE_QUIET is
> independent from the access bits passed in rule_attr
> - landlock_add_rule will return -EINVAL when LANDLOCK_ADD_RULE_QUIET is
> used but the ruleset does not have any quiet access bits set for the
> given rule type.
> - ABI version bump to v8
> - Syntactic and comment changes per suggestion.
>
> include/uapi/linux/landlock.h | 64 +++++++++++++++++
> security/landlock/domain.h | 5 ++
> security/landlock/fs.c | 4 +-
> security/landlock/fs.h | 2 +-
> security/landlock/net.c | 5 +-
> security/landlock/net.h | 3 +-
> security/landlock/ruleset.c | 10 ++-
> security/landlock/ruleset.h | 8 ++-
> security/landlock/syscalls.c | 72 +++++++++++++++-----
> tools/testing/selftests/landlock/base_test.c | 4 +-
> 10 files changed, 150 insertions(+), 27 deletions(-)
>
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f030adc462ee..0e8555c1f0d3 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -32,6 +32,19 @@
> * *handle* a wide range or all access rights that they know about at build time
> * (and that they have tested with a kernel that supported them all).
> *
> + * quiet_access_fs and quiet_access_net are bitmasks of actions for which
@quiet_access_fs and @quiet_access_fs
> + * a denial by this layer will not trigger an audit log if the
> + * corresponding object (or its children, for filesystem rules) is marked
> + * with the "quiet" bit via LANDLOCK_ADD_RULE_QUIET, even if audit logging
Why do you explicitly use "audit" before log? Wouldn't log be enough?
The first "audit log" is fine but we can then just use "log".
> + * would normally take place per landlock_restrict_self() flags.
> + * quiet_scoped is similar, except that it does not require marking any
> + * objects as quiet - if the ruleset is created with any bits set in
> + * quiet_scoped, then denial of such scoped resources will not trigger any
> + * audit log. These 3 fields are available since Landlock ABI version 8.
> + *
> + * quiet_access_fs, quiet_access_net and quiet_scoped must be a subset of
ditto
> + * handled_access_fs, handled_access_net and scoped respectively.
@handled_access_fs...
> + *
> * This structure can grow in future Landlock versions.
> */
> struct landlock_ruleset_attr {
> @@ -51,6 +64,24 @@ struct landlock_ruleset_attr {
> * resources (e.g. IPCs).
> */
> __u64 scoped;
> +
> + /* Since ABI 8: */
> +
> + /**
> + * @quiet_access_fs: Bitmask of filesystem actions which should not be
> + * audit logged if per-object quiet flag is set.
> + */
> + __u64 quiet_access_fs;
> + /**
> + * @quiet_access_net: Bitmask of network actions which should not be
> + * audit logged if per-object quiet flag is set.
> + */
> + __u64 quiet_access_net;
> + /**
> + * @quiet_scoped: Bitmask of scoped actions which should not be audit
> + * logged.
> + */
> + __u64 quiet_scoped;
For consistency, it would also make sense to be able to quiet implicit
restrictions: ptrace and FS topology changes. I'm not sure this is worth
it for now wrt the existing landlock_restrict_self()'s log flags though.
We could add a ptrace scoped flag and a fs_change_topology "denied" flag
(see https://lore.kernel.org/all/20250918.io7too8ain7A@digikod.net/)
that would be set by default. Then a new quiet_denied field could also
be added to landlock_ruleset_attr, and we'll get a consistent way to
quiet any kind of audit log. But for now, let's stick with the current
patch series.
> };
>
> /**
> @@ -69,6 +100,39 @@ struct landlock_ruleset_attr {
> #define LANDLOCK_CREATE_RULESET_ERRATA (1U << 1)
> /* clang-format on */
>
> +/**
> + * DOC: landlock_add_rule_flags
> + *
> + * **Flags**
> + *
> + * %LANDLOCK_ADD_RULE_QUIET
> + * Together with the quiet_* fields in struct landlock_ruleset_attr,
> + * this flag controls whether Landlock will log audit messages when
> + * access to the objects covered by this rule is denied by this layer.
> + *
> + * If audit logging is enabled, when Landlock denies an access, it will
> + * suppress the audit log if all of the following are true:
> + *
> + * - This layer is the innermost layer that denied the access;
> + * - The access is part of the quiet_* fields in the initial
> + * landlock_ruleset_attr;
I would say that *all* requested accesses are part of the quiet_*
fields in the related struct landlock_ruleset_attr.
> + * - The object (or one of its parents, for filesystem rules) is
> + * marked as "quiet" via LANDLOCK_ADD_RULE_QUIET.
Please consistently use %LANDLOCK_ADD_RULE_QUIET everywhere in the doc.
> + *
> + * Because logging is only suppressed by a layer if the layer denies
> + * access, a sandboxed program cannot use this flag to "hide" access
> + * denials, without denying itself the access in the first place.
> + *
> + * The effect of this flag does not depend on the value of
> + * allowed_access in the passed in rule_attr. When this flag is
> + * present, the caller is also allowed to pass in an empty
> + * allowed_access.
> + */
Excellent!
> +
> +/* clang-format off */
> +#define LANDLOCK_ADD_RULE_QUIET (1U << 0)
> +/* clang-format on */
> +
> /**
> * DOC: landlock_restrict_self_flags
> *
> diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> index 7fb70b25f85a..aadbf53505c0 100644
> --- a/security/landlock/domain.h
> +++ b/security/landlock/domain.h
> @@ -114,6 +114,11 @@ struct landlock_hierarchy {
> * %LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON. Set to false by default.
> */
> log_new_exec : 1;
> + /**
> + * @quiet_masks: Bitmasks of access that should be quieted (i.e. not
> + * logged) if the related object is marked as quiet.
> + */
> + struct access_masks quiet_masks;
> #endif /* CONFIG_AUDIT */
> };
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index e7eaf55093e9..b566ae498df5 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -322,7 +322,7 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> */
> int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> const struct path *const path,
> - access_mask_t access_rights)
> + access_mask_t access_rights, const int flags)
> {
> int err;
> struct landlock_id id = {
> @@ -343,7 +343,7 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> if (IS_ERR(id.key.object))
> return PTR_ERR(id.key.object);
> mutex_lock(&ruleset->lock);
> - err = landlock_insert_rule(ruleset, id, access_rights);
> + err = landlock_insert_rule(ruleset, id, access_rights, flags);
> mutex_unlock(&ruleset->lock);
> /*
> * No need to check for an error because landlock_insert_rule()
> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> index bf9948941f2f..cb7e654933ac 100644
> --- a/security/landlock/fs.h
> +++ b/security/landlock/fs.h
> @@ -126,6 +126,6 @@ __init void landlock_add_fs_hooks(void);
>
> int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> const struct path *const path,
> - access_mask_t access_hierarchy);
> + access_mask_t access_hierarchy, const int flags);
>
> #endif /* _SECURITY_LANDLOCK_FS_H */
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index fc6369dffa51..bddbe93d69fd 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -20,7 +20,8 @@
> #include "ruleset.h"
>
> int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
> - const u16 port, access_mask_t access_rights)
> + const u16 port, access_mask_t access_rights,
> + const int flags)
> {
> int err;
> const struct landlock_id id = {
> @@ -35,7 +36,7 @@ int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
> ~landlock_get_net_access_mask(ruleset, 0);
>
> mutex_lock(&ruleset->lock);
> - err = landlock_insert_rule(ruleset, id, access_rights);
> + err = landlock_insert_rule(ruleset, id, access_rights, flags);
> mutex_unlock(&ruleset->lock);
>
> return err;
> diff --git a/security/landlock/net.h b/security/landlock/net.h
> index 09960c237a13..799cedd5d0b7 100644
> --- a/security/landlock/net.h
> +++ b/security/landlock/net.h
> @@ -16,7 +16,8 @@
> __init void landlock_add_net_hooks(void);
>
> int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
> - const u16 port, access_mask_t access_rights);
> + const u16 port, access_mask_t access_rights,
> + const int flags);
> #else /* IS_ENABLED(CONFIG_INET) */
> static inline void landlock_add_net_hooks(void)
> {
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index 3aa4e33ac95b..5f551a7a7485 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -21,6 +21,7 @@
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> #include <linux/workqueue.h>
> +#include <uapi/linux/landlock.h>
>
> #include "access.h"
> #include "audit.h"
> @@ -251,6 +252,7 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
> if (WARN_ON_ONCE(this->layers[0].level != 0))
> return -EINVAL;
> this->layers[0].access |= (*layers)[0].access;
> + this->layers[0].flags.quiet |= (*layers)[0].flags.quiet;
> return 0;
> }
>
> @@ -297,12 +299,15 @@ static void build_check_layer(void)
> /* @ruleset must be locked by the caller. */
> int landlock_insert_rule(struct landlock_ruleset *const ruleset,
> const struct landlock_id id,
> - const access_mask_t access)
> + const access_mask_t access, const int flags)
> {
> struct landlock_layer layers[] = { {
> .access = access,
> /* When @level is zero, insert_rule() extends @ruleset. */
> .level = 0,
> + .flags = {
> + .quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
> + }
> } };
>
> build_check_layer();
> @@ -343,6 +348,7 @@ static int merge_tree(struct landlock_ruleset *const dst,
> return -EINVAL;
>
> layers[0].access = walker_rule->layers[0].access;
> + layers[0].flags = walker_rule->layers[0].flags;
>
> err = insert_rule(dst, id, &layers, ARRAY_SIZE(layers));
> if (err)
> @@ -573,6 +579,8 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
> if (err)
> return ERR_PTR(err);
>
> + new_dom->hierarchy->quiet_masks = ruleset->quiet_masks;
> +
> return no_free_ptr(new_dom);
> }
>
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index eeee287a9508..43d59c7116e5 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -193,6 +193,12 @@ struct landlock_ruleset {
> * non-merged ruleset (i.e. not a domain).
> */
> u32 num_layers;
> + /**
> + * @quiet_masks: Stores the quiet flags for an unmerged
> + * ruleset. For a merged domain, this is stored in each
> + * layer's struct landlock_hierarchy instead.
> + */
> + struct access_masks quiet_masks;
> /**
> * @access_masks: Contains the subset of filesystem and
> * network actions that are restricted by a ruleset.
> @@ -223,7 +229,7 @@ DEFINE_FREE(landlock_put_ruleset, struct landlock_ruleset *,
>
> int landlock_insert_rule(struct landlock_ruleset *const ruleset,
> const struct landlock_id id,
> - const access_mask_t access);
> + const access_mask_t access, const int flags);
>
> struct landlock_ruleset *
> landlock_merge_ruleset(struct landlock_ruleset *const parent,
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 0116e9f93ffe..8288bf914c8b 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -102,8 +102,11 @@ static void build_check_abi(void)
> ruleset_size = sizeof(ruleset_attr.handled_access_fs);
> ruleset_size += sizeof(ruleset_attr.handled_access_net);
> ruleset_size += sizeof(ruleset_attr.scoped);
> + ruleset_size += sizeof(ruleset_attr.quiet_access_fs);
> + ruleset_size += sizeof(ruleset_attr.quiet_access_net);
> + ruleset_size += sizeof(ruleset_attr.quiet_scoped);
> BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size);
> - BUILD_BUG_ON(sizeof(ruleset_attr) != 24);
> + BUILD_BUG_ON(sizeof(ruleset_attr) != 48);
>
> path_beneath_size = sizeof(path_beneath_attr.allowed_access);
> path_beneath_size += sizeof(path_beneath_attr.parent_fd);
> @@ -161,7 +164,7 @@ static const struct file_operations ruleset_fops = {
> * Documentation/userspace-api/landlock.rst should be updated to reflect the
> * UAPI change.
> */
> -const int landlock_abi_version = 7;
> +const int landlock_abi_version = 8;
>
> /**
> * sys_landlock_create_ruleset - Create a new ruleset
> @@ -185,6 +188,8 @@ const int landlock_abi_version = 7;
> *
> * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> * - %EINVAL: unknown @flags, or unknown access, or unknown scope, or too small @size;
> + * - %EINVAL: quiet_access_fs or quiet_fs_net is not a subset of the
> + * corresponding handled_access_fs or handled_access_net;
> * - %E2BIG: @attr or @size inconsistencies;
> * - %EFAULT: @attr or @size inconsistencies;
> * - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
> @@ -241,6 +246,21 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
> if ((ruleset_attr.scoped | LANDLOCK_MASK_SCOPE) != LANDLOCK_MASK_SCOPE)
> return -EINVAL;
>
> + /*
> + * Check that quiet masks are subsets of the respective handled masks.
> + * Because of the checks above this is sufficient to also ensure that
> + * the quiet masks are valid access masks.
> + */
> + if ((ruleset_attr.quiet_access_fs | ruleset_attr.handled_access_fs) !=
> + ruleset_attr.handled_access_fs)
> + return -EINVAL;
> + if ((ruleset_attr.quiet_access_net | ruleset_attr.handled_access_net) !=
> + ruleset_attr.handled_access_net)
> + return -EINVAL;
> + if ((ruleset_attr.quiet_scoped | ruleset_attr.scoped) !=
> + ruleset_attr.scoped)
> + return -EINVAL;
> +
> /* Checks arguments and transforms to kernel struct. */
> ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs,
> ruleset_attr.handled_access_net,
> @@ -248,6 +268,10 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
> if (IS_ERR(ruleset))
> return PTR_ERR(ruleset);
>
> + ruleset->quiet_masks.fs = ruleset_attr.quiet_access_fs;
> + ruleset->quiet_masks.net = ruleset_attr.quiet_access_net;
> + ruleset->quiet_masks.scope = ruleset_attr.quiet_scoped;
> +
> /* Creates anonymous FD referring to the ruleset. */
> ruleset_fd = anon_inode_getfd("[landlock-ruleset]", &ruleset_fops,
> ruleset, O_RDWR | O_CLOEXEC);
> @@ -312,7 +336,7 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
> }
>
> static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
> - const void __user *const rule_attr)
> + const void __user *const rule_attr, int flags)
> {
> struct landlock_path_beneath_attr path_beneath_attr;
> struct path path;
> @@ -327,9 +351,10 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
>
> /*
> * Informs about useless rule: empty allowed_access (i.e. deny rules)
> - * are ignored in path walks.
> + * are ignored in path walks. However, the rule is not useless if it
> + * is there to hold a quiet flag
> */
> - if (!path_beneath_attr.allowed_access)
> + if (!flags && !path_beneath_attr.allowed_access)
> return -ENOMSG;
>
> /* Checks that allowed_access matches the @ruleset constraints. */
> @@ -337,6 +362,10 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
> if ((path_beneath_attr.allowed_access | mask) != mask)
> return -EINVAL;
>
> + /* Check for useless quiet flag */
> + if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_masks.fs)
> + return -EINVAL;
> +
> /* Gets and checks the new rule. */
> err = get_path_from_fd(path_beneath_attr.parent_fd, &path);
> if (err)
> @@ -344,13 +373,13 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
>
> /* Imports the new rule. */
> err = landlock_append_fs_rule(ruleset, &path,
> - path_beneath_attr.allowed_access);
> + path_beneath_attr.allowed_access, flags);
> path_put(&path);
> return err;
> }
>
> static int add_rule_net_port(struct landlock_ruleset *ruleset,
> - const void __user *const rule_attr)
> + const void __user *const rule_attr, int flags)
> {
> struct landlock_net_port_attr net_port_attr;
> int res;
> @@ -363,9 +392,10 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
>
> /*
> * Informs about useless rule: empty allowed_access (i.e. deny rules)
> - * are ignored by network actions.
> + * are ignored by network actions. However, the rule is not useless
> + * if it is there to hold a quiet flag
> */
> - if (!net_port_attr.allowed_access)
> + if (!flags && !net_port_attr.allowed_access)
> return -ENOMSG;
>
> /* Checks that allowed_access matches the @ruleset constraints. */
> @@ -373,13 +403,17 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
> if ((net_port_attr.allowed_access | mask) != mask)
> return -EINVAL;
>
> + /* Check for useless quiet flag */
> + if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_masks.net)
> + return -EINVAL;
> +
> /* Denies inserting a rule with port greater than 65535. */
> if (net_port_attr.port > U16_MAX)
> return -EINVAL;
>
> /* Imports the new rule. */
> return landlock_append_net_rule(ruleset, net_port_attr.port,
> - net_port_attr.allowed_access);
> + net_port_attr.allowed_access, flags);
> }
>
> /**
> @@ -390,7 +424,7 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
> * @rule_type: Identify the structure type pointed to by @rule_attr:
> * %LANDLOCK_RULE_PATH_BENEATH or %LANDLOCK_RULE_NET_PORT.
> * @rule_attr: Pointer to a rule (matching the @rule_type).
> - * @flags: Must be 0.
> + * @flags: Must be 0 or %LANDLOCK_ADD_RULE_QUIET.
> *
> * This system call enables to define a new rule and add it to an existing
> * ruleset.
> @@ -400,20 +434,25 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
> * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> * - %EAFNOSUPPORT: @rule_type is %LANDLOCK_RULE_NET_PORT but TCP/IP is not
> * supported by the running kernel;
> - * - %EINVAL: @flags is not 0;
> + * - %EINVAL: @flags is not valid;
> * - %EINVAL: The rule accesses are inconsistent (i.e.
> * &landlock_path_beneath_attr.allowed_access or
> * &landlock_net_port_attr.allowed_access is not a subset of the ruleset
> * handled accesses)
> * - %EINVAL: &landlock_net_port_attr.port is greater than 65535;
> + * - %EINVAL: LANDLOCK_ADD_RULE_QUIET is passed but the ruleset has no
> + * quiet access bits set for the corresponding rule type.
> * - %ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access is
> - * 0);
> + * 0) and no flags;
> * - %EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
> * member of @rule_attr is not a file descriptor as expected;
> * - %EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
> * @rule_attr is not the expected file descriptor type;
> * - %EPERM: @ruleset_fd has no write access to the underlying ruleset;
> * - %EFAULT: @rule_attr was not a valid address.
> + *
> + * .. kernel-doc:: include/uapi/linux/landlock.h
> + * :identifiers: landlock_add_rule_flags
> */
> SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
> const enum landlock_rule_type, rule_type,
> @@ -424,8 +463,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
> if (!is_initialized())
> return -EOPNOTSUPP;
>
> - /* No flag for now. */
> - if (flags)
> + if (flags && flags != LANDLOCK_ADD_RULE_QUIET)
> return -EINVAL;
>
> /* Gets and checks the ruleset. */
> @@ -435,9 +473,9 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
>
> switch (rule_type) {
> case LANDLOCK_RULE_PATH_BENEATH:
> - return add_rule_path_beneath(ruleset, rule_attr);
> + return add_rule_path_beneath(ruleset, rule_attr, flags);
> case LANDLOCK_RULE_NET_PORT:
> - return add_rule_net_port(ruleset, rule_attr);
> + return add_rule_net_port(ruleset, rule_attr, flags);
> default:
> return -EINVAL;
> }
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 7b69002239d7..b34b340c52a5 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -76,7 +76,7 @@ TEST(abi_version)
> const struct landlock_ruleset_attr ruleset_attr = {
> .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - ASSERT_EQ(7, landlock_create_ruleset(NULL, 0,
> + ASSERT_EQ(8, landlock_create_ruleset(NULL, 0,
> LANDLOCK_CREATE_RULESET_VERSION));
>
> ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
> @@ -201,7 +201,7 @@ TEST(add_rule_checks_ordering)
> ASSERT_LE(0, ruleset_fd);
>
> /* Checks invalid flags. */
> - ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 1));
> + ASSERT_EQ(-1, landlock_add_rule(-1, 0, NULL, 100));
> ASSERT_EQ(EINVAL, errno);
>
> /* Checks invalid ruleset FD. */
> --
> 2.51.0
>
^ permalink raw reply
* Re: [PATCH v2 3/6] landlock/audit: Check for quiet flag in landlock_log_denial
From: Mickaël Salaün @ 2025-10-15 19:09 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <730434d416100f6a72b12fb31eb7253bc8b6fcc0.1759686613.git.m@maowtm.org>
Just use "landlock: " as subject prefix.
On Sun, Oct 05, 2025 at 06:55:26PM +0100, Tingmao Wang wrote:
> Suppresses logging if the flag is effective on the youngest layer.
>
> This does not handle optional access logging yet - to do that correctly we
> will need to expand deny_masks to support representing "don't log
> anything" in a later commit.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> Changes since v1:
> - Supports the new quiet access masks.
> - Support quieting scope requests (but not ptrace and attempted mounting
> for now)
>
> security/landlock/audit.c | 70 +++++++++++++++++++++++++++++++++++--
> security/landlock/audit.h | 3 +-
> security/landlock/fs.c | 18 +++++-----
> security/landlock/net.c | 3 +-
> security/landlock/ruleset.h | 5 +++
> security/landlock/task.c | 12 +++----
> 6 files changed, 92 insertions(+), 19 deletions(-)
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index c52d079cdb77..ec00b7dd00c5 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -381,19 +381,39 @@ static bool is_valid_request(const struct landlock_request *const request)
> return true;
> }
>
> +static access_mask_t
> +pick_access_mask_for_req_type(const enum landlock_request_type type,
pick_access_mask_for_request_type
> + const struct access_masks access_masks)
> +{
> + switch (type) {
> + case LANDLOCK_REQUEST_FS_ACCESS:
> + return access_masks.fs;
> + case LANDLOCK_REQUEST_NET_ACCESS:
> + return access_masks.net;
> + default:
> + WARN_ONCE(1, "Invalid request type %d passed to %s", type,
> + __func__);
> + return 0;
> + }
> +}
> +
> /**
> * landlock_log_denial - Create audit records related to a denial
> *
> * @subject: The Landlock subject's credential denying an action.
> * @request: Detail of the user space request.
> + * @rule_flags: The flags for the matched rule, or no_rule_flags (zero) if
> + * this is a scope request (no particular object involved).
> */
> void landlock_log_denial(const struct landlock_cred_security *const subject,
> - const struct landlock_request *const request)
> + const struct landlock_request *const request,
> + const struct collected_rule_flags rule_flags)
> {
> struct audit_buffer *ab;
> struct landlock_hierarchy *youngest_denied;
> size_t youngest_layer;
> - access_mask_t missing;
> + access_mask_t missing, quiet_mask;
> + bool quiet_flag_on_rule = false, quiet_applicable_to_access = false;
>
> if (WARN_ON_ONCE(!subject || !subject->domain ||
> !subject->domain->hierarchy || !request))
> @@ -436,6 +456,52 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
> if (!audit_enabled)
> return;
>
> + /*
> + * Checks if the object is marked quiet by the layer that denied the
> + * request. If it's a different layer that marked it as quiet, but
> + * that layer is not the one that denied the request, we should still
> + * audit log the denial.
> + */
> + quiet_flag_on_rule = !!(rule_flags.quiet_masks & BIT(youngest_layer));
> +
> + if (quiet_flag_on_rule) {
> + /*
> + * This is not a scope request, since rule_flags is not zero. We
> + * now check if the denied requests are all covered by the layer's
> + * quiet access bits.
> + */
> + quiet_mask = pick_access_mask_for_req_type(
> + request->type, youngest_denied->quiet_masks);
> + quiet_applicable_to_access = (quiet_mask & missing) == missing;
> +
> + if (quiet_applicable_to_access)
> + return;
> + } else {
> + quiet_mask = youngest_denied->quiet_masks.scope;
> + switch (request->type) {
> + case LANDLOCK_REQUEST_SCOPE_SIGNAL:
> + quiet_applicable_to_access =
> + !!(quiet_mask & LANDLOCK_SCOPE_SIGNAL);
> + break;
> + case LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET:
> + quiet_applicable_to_access =
> + !!(quiet_mask &
> + LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
> + break;
> + /*
> + * Leave LANDLOCK_REQUEST_PTRACE and
> + * LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY unhandled for now - they are
> + * never quiet
> + */
This also covers the case where the object is not quiet.
> + default:
> + break;
> + }
I find this if/else block a bit verbose but I didn't find a better
way...
> +
> + if (quiet_applicable_to_access) {
> + return;
> + }
We can still move this quiet_applicable_to_access check after the block
(and without the curly braces).
> + }
> +
> /* Checks if the current exec was restricting itself. */
> if (subject->domain_exec & BIT(youngest_layer)) {
> /* Ignores denials for the same execution. */
This domain_exec block would be better before the quiet_flag_on_rule
use.
> diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> index 92428b7fc4d8..80cf085465e3 100644
> --- a/security/landlock/audit.h
> +++ b/security/landlock/audit.h
> @@ -56,7 +56,8 @@ struct landlock_request {
> void landlock_log_drop_domain(const struct landlock_hierarchy *const hierarchy);
>
> void landlock_log_denial(const struct landlock_cred_security *const subject,
> - const struct landlock_request *const request);
> + const struct landlock_request *const request,
> + const struct collected_rule_flags rule_flags);
>
> #else /* CONFIG_AUDIT */
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index b566ae498df5..1ccef1c2959f 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -984,7 +984,7 @@ static int current_check_access_path(const struct path *const path,
> NULL, 0, NULL, NULL, NULL, NULL))
> return 0;
>
> - landlock_log_denial(subject, &request);
> + landlock_log_denial(subject, &request, rule_flags);
> return -EACCES;
> }
>
> @@ -1194,7 +1194,7 @@ static int current_check_refer_path(struct dentry *const old_dentry,
> &request1, NULL, 0, NULL, NULL, NULL, NULL))
> return 0;
>
> - landlock_log_denial(subject, &request1);
> + landlock_log_denial(subject, &request1, rule_flags_parent1);
> return -EACCES;
> }
>
> @@ -1243,11 +1243,11 @@ static int current_check_refer_path(struct dentry *const old_dentry,
>
> if (request1.access) {
> request1.audit.u.path.dentry = old_parent;
> - landlock_log_denial(subject, &request1);
> + landlock_log_denial(subject, &request1, rule_flags_parent1);
> }
> if (request2.access) {
> request2.audit.u.path.dentry = new_dir->dentry;
> - landlock_log_denial(subject, &request2);
> + landlock_log_denial(subject, &request2, rule_flags_parent2);
> }
>
> /*
> @@ -1403,7 +1403,7 @@ log_fs_change_topology_path(const struct landlock_cred_security *const subject,
> .u.path = *path,
> },
> .layer_plus_one = handle_layer + 1,
> - });
> + }, no_rule_flags);
> }
>
> static void log_fs_change_topology_dentry(
> @@ -1417,7 +1417,7 @@ static void log_fs_change_topology_dentry(
> .u.dentry = dentry,
> },
> .layer_plus_one = handle_layer + 1,
> - });
> + }, no_rule_flags);
> }
>
> /*
> @@ -1705,7 +1705,7 @@ static int hook_file_open(struct file *const file)
>
> /* Sets access to reflect the actual request. */
> request.access = open_access_request;
> - landlock_log_denial(subject, &request);
> + landlock_log_denial(subject, &request, rule_flags);
> return -EACCES;
> }
>
> @@ -1735,7 +1735,7 @@ static int hook_file_truncate(struct file *const file)
> #ifdef CONFIG_AUDIT
> .deny_masks = landlock_file(file)->deny_masks,
> #endif /* CONFIG_AUDIT */
> - });
> + }, no_rule_flags);
> return -EACCES;
> }
>
> @@ -1774,7 +1774,7 @@ static int hook_file_ioctl_common(const struct file *const file,
> #ifdef CONFIG_AUDIT
> .deny_masks = landlock_file(file)->deny_masks,
> #endif /* CONFIG_AUDIT */
> - });
> + }, no_rule_flags);
> return -EACCES;
> }
>
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index bddbe93d69fd..0587aa3d6d0f 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -193,7 +193,8 @@ static int current_check_access_socket(struct socket *const sock,
> .access = access_request,
> .layer_masks = &layer_masks,
> .layer_masks_size = ARRAY_SIZE(layer_masks),
> - });
> + },
> + rule_flags);
> return -EACCES;
> }
>
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 43d59c7116e5..6f44804c2c9b 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -58,6 +58,11 @@ struct collected_rule_flags {
> layer_mask_t quiet_masks;
> };
>
> +/**
> + * no_rule_flags - Convenience constant for an empty collected_rule_flags
> + */
> +static const struct collected_rule_flags no_rule_flags = { 0 };
You can remove the "0" for consistency.
> +
> /**
> * union landlock_key - Key of a ruleset's red-black tree
> */
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 2385017418ca..d5bd9a1b8467 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -115,7 +115,7 @@ static int hook_ptrace_access_check(struct task_struct *const child,
> .u.tsk = child,
> },
> .layer_plus_one = parent_subject->domain->num_layers,
> - });
> + }, no_rule_flags);
>
> return err;
> }
> @@ -161,7 +161,7 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
> .u.tsk = current,
> },
> .layer_plus_one = parent_subject->domain->num_layers,
> - });
> + }, no_rule_flags);
> return err;
> }
>
> @@ -290,7 +290,7 @@ static int hook_unix_stream_connect(struct sock *const sock,
> },
> },
> .layer_plus_one = handle_layer + 1,
> - });
> + }, no_rule_flags);
> return -EPERM;
> }
>
> @@ -327,7 +327,7 @@ static int hook_unix_may_send(struct socket *const sock,
> },
> },
> .layer_plus_one = handle_layer + 1,
> - });
> + }, no_rule_flags);
> return -EPERM;
> }
>
> @@ -383,7 +383,7 @@ static int hook_task_kill(struct task_struct *const p,
> .u.tsk = p,
> },
> .layer_plus_one = handle_layer + 1,
> - });
> + }, no_rule_flags);
> return -EPERM;
> }
>
> @@ -426,7 +426,7 @@ static int hook_file_send_sigiotask(struct task_struct *tsk,
> #ifdef CONFIG_AUDIT
> .layer_plus_one = landlock_file(fown->file)->fown_layer + 1,
> #endif /* CONFIG_AUDIT */
> - });
> + }, no_rule_flags);
> return -EPERM;
> }
>
> --
> 2.51.0
>
^ permalink raw reply
* Re: [PATCH v2 6/6] Implement quiet for optional accesses
From: Mickaël Salaün @ 2025-10-15 19:09 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <d9a05ea8fe3b825087351f22c550854dcad02555.1759686613.git.m@maowtm.org>
This extra patch makes the review easier, but it should be squashed into
the others if possible.
On Sun, Oct 05, 2025 at 06:55:29PM +0100, Tingmao Wang wrote:
> Since the existing deny_masks can only store 2x4bit of layer index, with
> no way to represent "no layer", we need to either expand it or have
> another field. This commit uses the latter approach - we add another
> field to store which optional access (of the 2) are covered by quiet rules
> in their respective layers as stored in deny_masks.
>
> We can avoid making struct landlock_file_security larger by converting the
> existing fown_layer to a 4bit field. This commit does that, and adds test
> to ensure that it is large enough for LANDLOCK_MAX_NUM_LAYERS-1.
>
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> Changes since v1:
> - New patch
>
> security/landlock/audit.c | 55 +++++++++++++++++++++++---------------
> security/landlock/audit.h | 1 +
> security/landlock/domain.c | 23 ++++++++++++++++
> security/landlock/domain.h | 5 ++++
> security/landlock/fs.c | 6 +++++
> security/landlock/fs.h | 34 ++++++++++++++++++-----
> 6 files changed, 96 insertions(+), 28 deletions(-)
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index 4ba44fb1dccb..f183124755a4 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -273,7 +273,7 @@ static void test_get_denied_layer(struct kunit *const 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)
> + const deny_masks_t deny_masks, u8 quiet_optional_accesses, bool *quiet)
> {
> const unsigned long access_opt = all_existing_optional_access;
> const unsigned long access_req = *access_request;
> @@ -285,6 +285,7 @@ get_layer_from_deny_masks(access_mask_t *const access_request,
> /* This will require change with new object types. */
> WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
>
> + *quiet = false;
> for_each_set_bit(access_bit, &access_opt,
> BITS_PER_TYPE(access_mask_t)) {
> if (access_req & BIT(access_bit)) {
> @@ -298,6 +299,11 @@ get_layer_from_deny_masks(access_mask_t *const access_request,
> } else if (layer == youngest_layer) {
> missing |= BIT(access_bit);
> }
> +
> + /* Make sure we set *quiet even if this is the first layer */
Missing final dot.
> + if (layer >= youngest_layer)
> + *quiet = !!(quiet_optional_accesses &
> + BIT(access_index));
This code is good but a bit confusing at first, especially without more
context than this patch provides, where we don't see the relation
between layer and youngest_layer. Anyway, please extend the comment to
say that quiet is always overridden for the youngest layer.
> }
> access_index++;
> }
> @@ -312,42 +318,49 @@ static void test_get_layer_from_deny_masks(struct kunit *const test)
> {
> deny_masks_t deny_mask;
> access_mask_t access;
> + u8 quiet_optional_accesses = 0;
> + bool quiet;
> + bool expected_quiet = false;
>
> /* truncate:0 ioctl_dev:2 */
> deny_mask = 0x20;
>
> 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_deny_masks(
> + &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
> + deny_mask, quiet_optional_accesses, &quiet));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
> + KUNIT_EXPECT_EQ(test, quiet, expected_quiet);
>
> 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_deny_masks(
> + &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
> + deny_mask, quiet_optional_accesses, &quiet));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_IOCTL_DEV);
> + KUNIT_EXPECT_EQ(test, quiet, expected_quiet);
>
> /* truncate:15 ioctl_dev:15 */
> deny_mask = 0xff;
>
> 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_deny_masks(
> + &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
> + deny_mask, quiet_optional_accesses, &quiet));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_TRUNCATE);
> + KUNIT_EXPECT_EQ(test, quiet, expected_quiet);
>
> 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_deny_masks(
> + &access, _LANDLOCK_ACCESS_FS_OPTIONAL,
> + deny_mask, quiet_optional_accesses, &quiet));
> KUNIT_EXPECT_EQ(test, access,
> LANDLOCK_ACCESS_FS_TRUNCATE |
> LANDLOCK_ACCESS_FS_IOCTL_DEV);
> + KUNIT_EXPECT_EQ(test, quiet, expected_quiet);
> }
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> @@ -413,7 +426,7 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
> struct landlock_hierarchy *youngest_denied;
> size_t youngest_layer;
> access_mask_t missing, quiet_mask;
> - bool quiet_flag_on_rule = false, quiet_applicable_to_access = false;
> + bool object_quiet_flag = false, quiet_applicable_to_access = false;
>
> if (WARN_ON_ONCE(!subject || !subject->domain ||
> !subject->domain->hierarchy || !request))
> @@ -429,10 +442,13 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
> youngest_layer = get_denied_layer(
> subject->domain, &missing, request->layer_masks,
> request->layer_masks_size);
> + object_quiet_flag = !!(rule_flags.quiet_masks & BIT(youngest_layer));
> } else {
> youngest_layer = get_layer_from_deny_masks(
> &missing, request->all_existing_optional_access,
> - request->deny_masks);
> + request->deny_masks,
> + request->quiet_optional_accesses,
> + &object_quiet_flag);
> }
> youngest_denied =
> get_hierarchy(subject->domain, youngest_layer);
> @@ -462,13 +478,10 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
> * that layer is not the one that denied the request, we should still
> * audit log the denial.
> */
> - quiet_flag_on_rule = !!(rule_flags.quiet_masks & BIT(youngest_layer));
> -
> - if (quiet_flag_on_rule) {
> + if (object_quiet_flag) {
> /*
> - * This is not a scope request, since rule_flags is not zero. We
> - * now check if the denied requests are all covered by the layer's
> - * quiet access bits.
> + * We now check if the denied requests are all covered by the
> + * layer's quiet access bits.
> */
> quiet_mask = pick_access_mask_for_req_type(
> request->type, youngest_denied->quiet_masks);
> diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> index 80cf085465e3..950365cd223d 100644
> --- a/security/landlock/audit.h
> +++ b/security/landlock/audit.h
> @@ -49,6 +49,7 @@ struct landlock_request {
> /* Required fields for requests with deny masks. */
> const access_mask_t all_existing_optional_access;
> deny_masks_t deny_masks;
> + u8 quiet_optional_accesses;
> };
>
> #ifdef CONFIG_AUDIT
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index a647b68e8d06..0f611ad516be 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -212,6 +212,29 @@ landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
> return deny_masks;
> }
>
Just using u8 is confusing. Please document what is the "type" of the
returned value, and use a dedicated typedef instead of u8 (see my other
comment about static_assert). This typedef should probably be named
optional_access_t and have a size less or equal to access_t's one.
> +u8 landlock_get_quiet_optional_accesses(
> + const access_mask_t all_existing_optional_access,
> + const deny_masks_t deny_masks,
> + const struct collected_rule_flags rule_flags)
> +{
> + const unsigned long access_opt = all_existing_optional_access;
> + size_t access_index = 0;
> + unsigned long access_bit;
> + u8 quiet_optional_accesses = 0;
As for deny_masks_t, we should define an "optional_access_t" type with
appropriate safeguard to guarantee that it can always hold all optional
access rights (see static_assert for deny_masks_t in access.h).
We should also copy the WARN_ON_ONCE() check from
get_layer_from_deny_masks().
> +
> + for_each_set_bit(access_bit, &access_opt,
> + BITS_PER_TYPE(access_mask_t)) {
> + const u8 layer = (deny_masks >> (access_index * 4)) &
> + (LANDLOCK_MAX_NUM_LAYERS - 1);
> + const bool is_quiet = !!(rule_flags.quiet_masks & BIT(layer));
> +
> + if (is_quiet)
> + quiet_optional_accesses |= BIT(access_index);
> + access_index++;
> + }
> + return quiet_optional_accesses;
> +}
> +
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
>
> static void test_landlock_get_deny_masks(struct kunit *const test)
> diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> index aadbf53505c0..ab9e5898776d 100644
> --- a/security/landlock/domain.h
> +++ b/security/landlock/domain.h
> @@ -130,6 +130,11 @@ landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
> const layer_mask_t (*const layer_masks)[],
> size_t layer_masks_size);
>
> +u8 landlock_get_quiet_optional_accesses(
> + const access_mask_t all_existing_optional_access,
> + const deny_masks_t deny_masks,
> + const struct collected_rule_flags rule_flags);
> +
> int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy);
>
> static inline void
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 1ccef1c2959f..4a71b792c4e7 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1698,6 +1698,10 @@ static int hook_file_open(struct file *const file)
> landlock_file(file)->deny_masks = landlock_get_deny_masks(
> _LANDLOCK_ACCESS_FS_OPTIONAL, optional_access, &layer_masks,
> ARRAY_SIZE(layer_masks));
> + landlock_file(file)->quiet_optional_accesses =
> + landlock_get_quiet_optional_accesses(
> + _LANDLOCK_ACCESS_FS_OPTIONAL,
> + landlock_file(file)->deny_masks, rule_flags);
> #endif /* CONFIG_AUDIT */
>
> if ((open_access_request & allowed_access) == open_access_request)
> @@ -1734,6 +1738,7 @@ static int hook_file_truncate(struct file *const file)
> .access = LANDLOCK_ACCESS_FS_TRUNCATE,
> #ifdef CONFIG_AUDIT
> .deny_masks = landlock_file(file)->deny_masks,
> + .quiet_optional_accesses = landlock_file(file)->quiet_optional_accesses,
> #endif /* CONFIG_AUDIT */
> }, no_rule_flags);
> return -EACCES;
> @@ -1773,6 +1778,7 @@ static int hook_file_ioctl_common(const struct file *const file,
> .access = LANDLOCK_ACCESS_FS_IOCTL_DEV,
> #ifdef CONFIG_AUDIT
> .deny_masks = landlock_file(file)->deny_masks,
> + .quiet_optional_accesses = landlock_file(file)->quiet_optional_accesses,
> #endif /* CONFIG_AUDIT */
> }, no_rule_flags);
> return -EACCES;
> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> index cb7e654933ac..04708cf4ec0f 100644
> --- a/security/landlock/fs.h
> +++ b/security/landlock/fs.h
> @@ -63,11 +63,19 @@ struct landlock_file_security {
> * _LANDLOCK_ACCESS_FS_OPTIONAL).
> */
> deny_masks_t deny_masks;
> + /**
> + * @quiet_optional_accesses: Stores which optional accesses are
> + * covered by quiet rules within the layer referred to in deny_masks,
> + * one access per bit. Does not take into account whether the quiet
> + * access bits are actually set in the layer's corresponding
> + * landlock_hierarchy.
> + */
> + u8 quiet_optional_accesses:2;
> /**
> * @fown_layer: Layer level of @fown_subject->domain with
> * LANDLOCK_SCOPE_SIGNAL.
> */
> - u8 fown_layer;
> + u8 fown_layer:4;
> #endif /* CONFIG_AUDIT */
>
> /**
> @@ -80,13 +88,24 @@ struct landlock_file_security {
> struct landlock_cred_security fown_subject;
> };
>
> -#ifdef CONFIG_AUDIT
> +static void build_check_file_security(void)
You can move this function to fs.c and call it in
hook_file_alloc_security() instead.
> +{
> + const struct landlock_file_security file_sec = {
> + .quiet_optional_accesses = ~0,
> + .fown_layer = ~0,
> + };
> +
> + /*
> + * Make sure quiet_optional_accesses has enough bits to cover all
> + * optional accesses
> + */
> + BUILD_BUG_ON(__const_hweight8(file_sec.quiet_optional_accesses) <
We should be able to use HWEIGHT() instead.
> + __const_hweight64(_LANDLOCK_ACCESS_FS_OPTIONAL));
> + /* Makes sure all layers can be identified. */
> + BUILD_BUG_ON(file_sec.fown_layer < LANDLOCK_MAX_NUM_LAYERS - 1);
> +}
>
> -/* Makes sure all layers can be identified. */
> -/* clang-format off */
> -static_assert((typeof_member(struct landlock_file_security, fown_layer))~0 >=
> - LANDLOCK_MAX_NUM_LAYERS);
> -/* clang-format off */
> +#ifdef CONFIG_AUDIT
>
> #endif /* CONFIG_AUDIT */
>
> @@ -107,6 +126,7 @@ struct landlock_superblock_security {
> static inline struct landlock_file_security *
> landlock_file(const struct file *const file)
> {
> + build_check_file_security();
> return file->f_security + landlock_blob_sizes.lbs_file;
> }
>
> --
> 2.51.0
^ permalink raw reply
* Re: [PATCH v2 0/6] Implement LANDLOCK_ADD_RULE_QUIET
From: Mickaël Salaün @ 2025-10-15 19:06 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <cover.1759686613.git.m@maowtm.org>
This patch series is not simple but really good, thanks! I mostly have
nicking comments. I'm looking forward the kselftests.
On Sun, Oct 05, 2025 at 06:55:23PM +0100, Tingmao Wang wrote:
> Hi,
>
> This is the v2 of the "quiet flag" series, implementing the feature as
> proposed in [1].
>
> v1: https://lore.kernel.org/all/cover.1757376311.git.m@maowtm.org/
>
> 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.
>
> In this version, the most significant change is that we now have a 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.
>
> This version also implements quiet support for optional accesses (truncate
> and ioctl), scope denials (signal, abstract unix socket), addresses
> suggestions from v1 review, and further enhances sandboxer to allow full
> customization of which access to quiet. Network and scope access quieting
> are now also supported by the sandboxer via additional environment
> variables.
>
> I still haven't added any selftests yet but did some testing with
> sandboxer. I would like this to be reviewed as it stands, before
> finishing up the tests which I will hopefully add in v3.
>
> Patches removed since v1:
> - landlock/access: Improve explanation on the deny_masks_t
>
> 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.
>
> All existing kselftests pass.
>
> [1]: https://github.com/landlock-lsm/linux/issues/44#issuecomment-2876500918
>
> Kind regards,
> Tingmao
>
> Tingmao Wang (6):
> landlock: Add a place for flags to layer rules
> landlock: Add API support and docs for the quiet flags
> landlock/audit: Check for quiet flag in landlock_log_denial
> landlock/audit: Fix wrong type usage
> samples/landlock: Add quiet flag support to sandboxer
> Implement quiet for optional accesses
>
> include/uapi/linux/landlock.h | 64 +++++++++
> samples/landlock/sandboxer.c | 133 +++++++++++++++++--
> security/landlock/audit.c | 113 +++++++++++++---
> security/landlock/audit.h | 4 +-
> security/landlock/domain.c | 23 ++++
> security/landlock/domain.h | 10 ++
> security/landlock/fs.c | 103 ++++++++------
> security/landlock/fs.h | 36 +++--
> security/landlock/net.c | 11 +-
> security/landlock/net.h | 3 +-
> security/landlock/ruleset.c | 19 ++-
> security/landlock/ruleset.h | 39 +++++-
> security/landlock/syscalls.c | 72 +++++++---
> security/landlock/task.c | 12 +-
> tools/testing/selftests/landlock/base_test.c | 4 +-
> 15 files changed, 538 insertions(+), 108 deletions(-)
>
>
> base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
> --
> 2.51.0
>
>
^ permalink raw reply
* Re: [f2fs-dev] [PATCH kvm-next V11 6/7] KVM: guest_memfd: Enforce NUMA mempolicy using shared policy
From: Gregory Price @ 2025-10-15 21:45 UTC (permalink / raw)
To: Sean Christopherson
Cc: Shivank Garg, jgowans, mhocko, jack, kvm, david, linux-btrfs, aik,
papaluri, kalyazin, peterx, linux-mm, clm, ddutile,
linux-kselftest, shdhiman, gshan, ying.huang, shuah, roypat,
matthew.brost, linux-coco, zbestahu, lorenzo.stoakes,
linux-bcachefs, ira.weiny, dhavale, jmorris, willy, hch, chao.gao,
tabba, ziy, rientjes, yuzhao, xiang, nikunj, serge, amit,
thomas.lendacky, ashish.kalra, chao.p.peng, yan.y.zhao, byungchul,
michael.day, Neeraj.Upadhyay, michael.roth, bfoster, bharata,
josef, Liam.Howlett, ackerleytng, dsterba, viro, jefflexu,
jaegeuk, dan.j.williams, surenb, vbabka, paul, joshua.hahnjy,
apopple, brauner, quic_eberman, rakie.kim, cgzones, pvorel,
linux-erofs, kent.overstreet, linux-kernel, linux-f2fs-devel,
pankaj.gupta, linux-security-module, lihongbo22, linux-fsdevel,
pbonzini, akpm, vannapurve, suzuki.poulose, rppt, jgg
In-Reply-To: <aNbrO7A7fSjb4W84@google.com>
On Fri, Sep 26, 2025 at 12:36:27PM -0700, Sean Christopherson via Linux-f2fs-devel wrote:
> >
> > static struct mempolicy *kvm_gmem_get_policy(struct vm_area_struct *vma,
> > unsigned long addr, pgoff_t *pgoff)
> > {
> > *pgoff = vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT);
> >
> > return __kvm_gmem_get_policy(GMEM_I(file_inode(vma->vm_file)), *pgoff);
>
> Argh!!!!! This breaks the selftest because do_get_mempolicy() very specifically
> falls back to the default_policy, NOT to the current task's policy. That is
> *exactly* the type of subtle detail that needs to be commented, because there's
> no way some random KVM developer is going to know that returning NULL here is
> important with respect to get_mempolicy() ABI.
>
Do_get_mempolicy was designed to be accessed by the syscall, not as an in-kernel ABI.
get_task_policy also returns the default policy if there's nothing
there, because that's what applies.
I have dangerous questions:
why is __kvm_gmem_get_policy using
mpol_shared_policy_lookup()
instead of
get_vma_policy()
get_vma_policy does this all for you
struct mempolicy *get_vma_policy(struct vm_area_struct *vma,
unsigned long addr, int order, pgoff_t *ilx)
{
struct mempolicy *pol;
pol = __get_vma_policy(vma, addr, ilx);
if (!pol)
pol = get_task_policy(current);
if (pol->mode == MPOL_INTERLEAVE ||
pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
*ilx += vma->vm_pgoff >> order;
*ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
}
return pol;
}
Of course you still have the same issue: get_task_policy will return the
default, because that's what applies.
do_get_mempolicy just seems like the completely incorrect interface to
be using here.
~Gregory
^ permalink raw reply
* Re: [f2fs-dev] [PATCH kvm-next V11 6/7] KVM: guest_memfd: Enforce NUMA mempolicy using shared policy
From: Sean Christopherson @ 2025-10-15 22:48 UTC (permalink / raw)
To: Gregory Price
Cc: Shivank Garg, jgowans, mhocko, jack, kvm, david, linux-btrfs, aik,
papaluri, kalyazin, peterx, linux-mm, clm, ddutile,
linux-kselftest, shdhiman, gshan, ying.huang, shuah, roypat,
matthew.brost, linux-coco, zbestahu, lorenzo.stoakes,
linux-bcachefs, ira.weiny, dhavale, jmorris, willy, hch, chao.gao,
tabba, ziy, rientjes, yuzhao, xiang, nikunj, serge, amit,
thomas.lendacky, ashish.kalra, chao.p.peng, yan.y.zhao, byungchul,
michael.day, Neeraj.Upadhyay, michael.roth, bfoster, bharata,
josef, Liam.Howlett, ackerleytng, dsterba, viro, jefflexu,
jaegeuk, dan.j.williams, surenb, vbabka, paul, joshua.hahnjy,
apopple, brauner, quic_eberman, rakie.kim, cgzones, pvorel,
linux-erofs, kent.overstreet, linux-kernel, linux-f2fs-devel,
pankaj.gupta, linux-security-module, lihongbo22, linux-fsdevel,
pbonzini, akpm, vannapurve, suzuki.poulose, rppt, jgg
In-Reply-To: <aPAWFQyFLK4EKWVK@gourry-fedora-PF4VCD3F>
On Wed, Oct 15, 2025, Gregory Price wrote:
> On Fri, Sep 26, 2025 at 12:36:27PM -0700, Sean Christopherson via Linux-f2fs-devel wrote:
> > >
> > > static struct mempolicy *kvm_gmem_get_policy(struct vm_area_struct *vma,
> > > unsigned long addr, pgoff_t *pgoff)
> > > {
> > > *pgoff = vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT);
> > >
> > > return __kvm_gmem_get_policy(GMEM_I(file_inode(vma->vm_file)), *pgoff);
> >
> > Argh!!!!! This breaks the selftest because do_get_mempolicy() very specifically
> > falls back to the default_policy, NOT to the current task's policy. That is
> > *exactly* the type of subtle detail that needs to be commented, because there's
> > no way some random KVM developer is going to know that returning NULL here is
> > important with respect to get_mempolicy() ABI.
> >
>
> Do_get_mempolicy was designed to be accessed by the syscall, not as an
> in-kernel ABI.
Ya, by "get_mempolicy() ABI" I meant the uABI for the get_mempolicy syscall.
> get_task_policy also returns the default policy if there's nothing
> there, because that's what applies.
>
> I have dangerous questions:
Not dangerous at all, I find them very helpful!
> why is __kvm_gmem_get_policy using
> mpol_shared_policy_lookup()
> instead of
> get_vma_policy()
With the disclaimer that I haven't followed the gory details of this series super
closely, my understanding is...
Because the VMA is a means to an end, and we want the policy to persist even if
the VMA goes away.
With guest_memfd, KVM effectively inverts the standard MMU model. Instead of mm/
being the primary MMU and KVM being a secondary MMU, guest_memfd is the primary
MMU and any VMAs are secondary (mostly; it's probably more like 1a and 1b). This
allows KVM to map guest_memfd memory into a guest without a VMA, or with more
permissions than are granted to host userspace, e.g. guest_memfd memory could be
writable by the guest, but read-only for userspace.
But we still want to support things like mbind() so that userspace can ensure
guest_memfd allocations align with the vNUMA topology presented to the guest,
or are bound to the NUMA node where the VM will run. We considered adding equivalent
file-based syscalls, e.g. fbind(), but IIRC the consensus was that doing so was
unnecessary (and potentially messy?) since we were planning on eventually adding
mmap() support to guest_memfd anyways.
> get_vma_policy does this all for you
I assume that doesn't work if the intent is for new VMAs to pick up the existing
policy from guest_memfd? And more importantly, guest_memfd needs to hook
->set_policy so that changes through e.g. mbind() persist beyond the lifetime of
the VMA.
> struct mempolicy *get_vma_policy(struct vm_area_struct *vma,
> unsigned long addr, int order, pgoff_t *ilx)
> {
> struct mempolicy *pol;
>
> pol = __get_vma_policy(vma, addr, ilx);
> if (!pol)
> pol = get_task_policy(current);
> if (pol->mode == MPOL_INTERLEAVE ||
> pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
> *ilx += vma->vm_pgoff >> order;
> *ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> }
> return pol;
> }
>
> Of course you still have the same issue: get_task_policy will return the
> default, because that's what applies.
>
> do_get_mempolicy just seems like the completely incorrect interface to
> be using here.
^ permalink raw reply
* [PATCH] KEYS: encrypted: Simplify determining 'format_len'
From: Thorsten Blum @ 2025-10-15 23:21 UTC (permalink / raw)
To: Mimi Zohar, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn
Cc: Thorsten Blum, linux-integrity, keyrings, linux-security-module,
linux-kernel
Don't unnecessarily negate 'format' and simplify the calculation of
'format_len' in encrypted_key_alloc() and __ekey_init().
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/keys/encrypted-keys/encrypted.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index aef438d18da8..d70f71d37f5f 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -581,7 +581,7 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
if (ret < 0 || dlen < MIN_DATA_SIZE || dlen > MAX_DATA_SIZE)
return ERR_PTR(-EINVAL);
- format_len = (!format) ? strlen(key_format_default) : strlen(format);
+ format_len = strlen(format ?: key_format_default);
decrypted_datalen = dlen;
payload_datalen = decrypted_datalen;
@@ -704,7 +704,7 @@ static void __ekey_init(struct encrypted_key_payload *epayload,
{
unsigned int format_len;
- format_len = (!format) ? strlen(key_format_default) : strlen(format);
+ format_len = strlen(format ?: key_format_default);
epayload->format = epayload->payload_data + epayload->payload_datalen;
epayload->master_desc = epayload->format + format_len + 1;
epayload->datalen = epayload->master_desc + strlen(master_desc) + 1;
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] ima: Fall back to default kernel module signature verification
From: Coiby Xu @ 2025-10-16 3:46 UTC (permalink / raw)
To: Mimi Zohar
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: <bcd1f7b48311aff55711cdff4a6cdbb72aae1d04.camel@linux.ibm.com>
On Tue, Sep 30, 2025 at 04:28:14PM -0400, Mimi Zohar wrote:
>On Tue, 2025-09-30 at 09:57 -0400, Mimi Zohar wrote:
>> On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
>> > Currently, for any IMA policy that requires appraisal for kernel modules
>> > e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
>> > booting will fail because IMA will reject a kernel module which will
>> > be decompressed in the kernel space and then have its signature
>> > verified.
>> >
>> > This happens because when in-kernel module decompression
>> > (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
>> > syscall instead of init_module to load a module. And IMA mandates IMA
>> > xattr verification for finit_module unless appraise_type=imasig|modsig
>> > is specified in the rule. However currently initramfs doesn't support
>> > xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
>> > doesn't work either because IMA will treat to-be-decompressed kernel
>> > module as not having module signature as it can't decompress kernel
>> > module to check if signature exists.
>> >
>> > So fall back to default kernel module signature verification when we have
>> > no way to verify IMA xattr.
>> >
>> > Reported-by: Karel Srot <ksrot@redhat.com>
>> > Signed-off-by: Coiby Xu <coxu@redhat.com>
>> > ---
>> > Another approach will be to make IMA decompress the kernel module to
>> > check the signature. This requires refactoring kernel module code to
>> > make the in-kernel module decompressing feature modular and seemingly
>> > more efforts are needed. A second disadvantage is it feels
>> > counter-intuitive to verify the same kernel module signature twice. And
>> > we still need to make ima_policy=secure_boot allow verifying appended
>> > module signature.
>> >
>> > Anyways, I'm open to suggestions and can try the latter approach if
>> > there are some benefits I'm not aware of or a better approach.
>>
>> Coiby, there are multiple issues being discussed here. Before deciding on an
>> appropriate solution, let's frame the issues(s) properly.
Hi Mimi,
Thanks for listing and framing the issues! Sorry, it took me a while to
go through your feedback as I also had a 8-day holiday.
>>
>> 1. The finit_module syscall eventually calls init_module_from_file() to read the
>> module into memory and then decompress it. The problem is that the kernel
>> module signature verification occurs during the kernel_read_file(), before the
>> kernel module is decompressed. Thus, the appended kernel module signature
>> cannot be verified.
Since IMA only accesses a kernel module as a fd or struct file*, even if
IMA hook is triggerd after kernel module is decompressed, IMA still
doesn't know the default verification has passed or can't access the
decompressed kernel buffer [2] to do the verification by itself.
[2] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v6.17#n3689
>>
>> 2. CPIO doesn't have xattr support. There were multiple attempts at including
>> xattrs in CPIO, but none were upstreamed [1]. If file signatures stored in
>> security.ima were available in the initramfs, then finit_module() could verify
>> them, as opposed to the appended kernel module signature.
Thanks you for pointing me to the work [1]. I'll take a more careful
look at [1]. I think CPIO supporting xattr can be a long-term solution
and also close a important security gap.
>>
>> 3. The issues described above are generic, not limited to Power. When
>> CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
>> include an "appraise func=MODULE_CHECK".
Yes, the issue is not limited to Power. And thanks for correcting me
that Power arch specific IMA policy rules don't have this problem! Sorry
I misread the code.
>>
>> 4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
>> policy, specified on the boot command line as "ima_policy=secure_boot", always
>> enforces the IMA signature stored in security.ima.
>>
>> Partial solutions without kernel changes:
>> - Enable CONFIG_MODULE_SIG (Doesn't solve 4)
>> - Disable kernel module compression.
>>
>> Complete solution:
>> - Pick up and upstream Roberto Sassu's last version of initramfs support [1].
>> - Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
>> enumeration is READING_MODULE and the kernel module is compressed. The change
>> might be limited to ima_post_read_file().
>
>or perhaps not totally.
>
>init_module_from_file() doesn't pass the flags variable to kernel_read_file().
>You might want to consider defining a new kernel_read_file_id enumeration named
>READING_COMPRESSED_MODULE.
Thanks for suggesting the solutions! I like the solution of CPIO
supporting xattr but it seems it won't land in upstream soon. So I
prefer the last approach. I've implemented one [3] by defining a new
kernel_read_file_id enumeration, would you like me to post the patches
to the mailing list directly?
[3] https://github.com/coiby/linux/tree/in_kernel_decompression_ima
>
>Mimi
>
>>
>> [1] [PATCH v4 0/3] initramfs: add support for xattrs in the initial ram disk
>> https://lore.kernel.org/linux-fsdevel/20190523121803.21638-1-roberto.sassu@huawei.com/
>>
>>
>
>
--
Best regards,
Coiby
^ permalink raw reply
* Re: [PATCH] ima: Fall back to default kernel module signature verification
From: Coiby Xu @ 2025-10-16 3:51 UTC (permalink / raw)
To: kernel test robot
Cc: linux-integrity, oe-kbuild-all, Dmitry Torokhov, Karel Srot,
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, linux-security-module,
linux-kernel
In-Reply-To: <202510030029.VRKgik99-lkp@intel.com>
On Fri, Oct 03, 2025 at 01:17:30AM +0800, kernel test robot wrote:
>Hi Coiby,
Hi,
>
>kernel test robot noticed the following build errors:
>
>[auto build test ERROR on cec1e6e5d1ab33403b809f79cd20d6aff124ccfe]
>
>url: https://github.com/intel-lab-lkp/linux/commits/Coiby-Xu/ima-Fall-back-to-default-kernel-module-signature-verification/20250928-110501
>base: cec1e6e5d1ab33403b809f79cd20d6aff124ccfe
>patch link: https://lore.kernel.org/r/20250928030358.3873311-1-coxu%40redhat.com
>patch subject: [PATCH] ima: Fall back to default kernel module signature verification
>config: i386-randconfig-012-20251002 (https://download.01.org/0day-ci/archive/20251003/202510030029.VRKgik99-lkp@intel.com/config)
>compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
>reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251003/202510030029.VRKgik99-lkp@intel.com/reproduce)
>
>If you fix the issue in a separate patch/commit (i.e. not just a new version of
>the same patch/commit), kindly add following tags
>| Reported-by: kernel test robot <lkp@intel.com>
>| Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
>
>All errors (new ones prefixed by >>):
>
> ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
>>> security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'
Thanks for reporting the error! This happens when
set_module_sig_enforced is called without CONFIG_MODULE_SIG not enabled.
I'll address this issue by declaring set_module_sig_enforced only when
CONFIG_MODULE_SIG is enabled.
>
>
>vim +587 security/integrity/ima/ima_appraise.c
>
> 483
> 484 /*
> 485 * ima_appraise_measurement - appraise file measurement
> 486 *
> 487 * Call evm_verifyxattr() to verify the integrity of 'security.ima'.
> 488 * Assuming success, compare the xattr hash with the collected measurement.
> 489 *
> 490 * Return 0 on success, error code otherwise
> 491 */
> 492 int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
> 493 struct file *file, const unsigned char *filename,
> 494 struct evm_ima_xattr_data *xattr_value,
> 495 int xattr_len, const struct modsig *modsig)
> 496 {
> 497 static const char op[] = "appraise_data";
> 498 int audit_msgno = AUDIT_INTEGRITY_DATA;
> 499 const char *cause = "unknown";
> 500 struct dentry *dentry = file_dentry(file);
> 501 struct inode *inode = d_backing_inode(dentry);
> 502 enum integrity_status status = INTEGRITY_UNKNOWN;
> 503 int rc = xattr_len;
> 504 bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
> 505 bool enforce_module_sig = iint->flags & IMA_DIGSIG_REQUIRED && func == MODULE_CHECK;
> 506
> 507 /* If not appraising a modsig or using default module verification, we need an xattr. */
> 508 if (!(inode->i_opflags & IOP_XATTR) && !try_modsig && !enforce_module_sig)
> 509 return INTEGRITY_UNKNOWN;
> 510
> 511 /*
> 512 * Unlike any of the other LSM hooks where the kernel enforces file
> 513 * integrity, enforcing file integrity for the bprm_creds_for_exec()
> 514 * LSM hook with the AT_EXECVE_CHECK flag is left up to the discretion
> 515 * of the script interpreter(userspace). Differentiate kernel and
> 516 * userspace enforced integrity audit messages.
> 517 */
> 518 if (is_bprm_creds_for_exec(func, file))
> 519 audit_msgno = AUDIT_INTEGRITY_USERSPACE;
> 520
> 521 /* If reading the xattr failed and there's no modsig or module verification, error out. */
> 522 if (rc <= 0 && !try_modsig && !enforce_module_sig) {
> 523 if (rc && rc != -ENODATA)
> 524 goto out;
> 525
> 526 if (iint->flags & IMA_DIGSIG_REQUIRED) {
> 527 if (iint->flags & IMA_VERITY_REQUIRED)
> 528 cause = "verity-signature-required";
> 529 else
> 530 cause = "IMA-signature-required";
> 531 } else {
> 532 cause = "missing-hash";
> 533 }
> 534
> 535 status = INTEGRITY_NOLABEL;
> 536 if (file->f_mode & FMODE_CREATED)
> 537 iint->flags |= IMA_NEW_FILE;
> 538 if ((iint->flags & IMA_NEW_FILE) &&
> 539 (!(iint->flags & IMA_DIGSIG_REQUIRED) ||
> 540 (inode->i_size == 0)))
> 541 status = INTEGRITY_PASS;
> 542 goto out;
> 543 }
> 544
> 545 status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value,
> 546 rc < 0 ? 0 : rc);
> 547 switch (status) {
> 548 case INTEGRITY_PASS:
> 549 case INTEGRITY_PASS_IMMUTABLE:
> 550 case INTEGRITY_UNKNOWN:
> 551 break;
> 552 case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */
> 553 /* Fine to not have xattrs when using a modsig or default module verification. */
> 554 if (try_modsig || enforce_module_sig)
> 555 break;
> 556 fallthrough;
> 557 case INTEGRITY_NOLABEL: /* No security.evm xattr. */
> 558 cause = "missing-HMAC";
> 559 goto out;
> 560 case INTEGRITY_FAIL_IMMUTABLE:
> 561 set_bit(IMA_DIGSIG, &iint->atomic_flags);
> 562 cause = "invalid-fail-immutable";
> 563 goto out;
> 564 case INTEGRITY_FAIL: /* Invalid HMAC/signature. */
> 565 cause = "invalid-HMAC";
> 566 goto out;
> 567 default:
> 568 WARN_ONCE(true, "Unexpected integrity status %d\n", status);
> 569 }
> 570
> 571 if (xattr_value)
> 572 rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
> 573 &cause);
> 574
> 575 /*
> 576 * If we have a modsig and either no imasig or the imasig's key isn't
> 577 * known, then try verifying the modsig.
> 578 */
> 579 if (try_modsig &&
> 580 (!xattr_value || xattr_value->type == IMA_XATTR_DIGEST_NG ||
> 581 rc == -ENOKEY))
> 582 rc = modsig_verify(func, modsig, &status, &cause);
> 583
> 584 /* Fall back to default kernel module signature verification */
> 585 if (rc && enforce_module_sig) {
> 586 rc = 0;
> > 587 set_module_sig_enforced();
> 588 /* CONFIG_MODULE_SIG may be disabled */
> 589 if (is_module_sig_enforced()) {
> 590 rc = 0;
> 591 status = INTEGRITY_PASS;
> 592 pr_debug("Fall back to default kernel module verification for %s\n", filename);
> 593 }
> 594 }
> 595
> 596 out:
> 597 /*
> 598 * File signatures on some filesystems can not be properly verified.
> 599 * When such filesystems are mounted by an untrusted mounter or on a
> 600 * system not willing to accept such a risk, fail the file signature
> 601 * verification.
> 602 */
> 603 if ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) &&
> 604 ((inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) ||
> 605 (iint->flags & IMA_FAIL_UNVERIFIABLE_SIGS))) {
> 606 status = INTEGRITY_FAIL;
> 607 cause = "unverifiable-signature";
> 608 integrity_audit_msg(audit_msgno, inode, filename,
> 609 op, cause, rc, 0);
> 610 } else if (status != INTEGRITY_PASS) {
> 611 /* Fix mode, but don't replace file signatures. */
> 612 if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
> 613 (!xattr_value ||
> 614 xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
> 615 if (!ima_fix_xattr(dentry, iint))
> 616 status = INTEGRITY_PASS;
> 617 }
> 618
> 619 /*
> 620 * Permit new files with file/EVM portable signatures, but
> 621 * without data.
> 622 */
> 623 if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
> 624 test_bit(IMA_DIGSIG, &iint->atomic_flags)) {
> 625 status = INTEGRITY_PASS;
> 626 }
> 627
> 628 integrity_audit_msg(audit_msgno, inode, filename,
> 629 op, cause, rc, 0);
> 630 } else {
> 631 ima_cache_flags(iint, func);
> 632 }
> 633
> 634 ima_set_cache_status(iint, func, status);
> 635 return status;
> 636 }
> 637
>
>--
>0-DAY CI Kernel Test Service
>https://github.com/intel/lkp-tests/wiki
>
--
Best regards,
Coiby
^ 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