[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4895afcecf518a0744b964cfa4c16e3a@paul-moore.com>
Date: Thu, 24 Apr 2025 18:18:32 -0400
From: Paul Moore <paul@...l-moore.com>
To: Casey Schaufler <casey@...aufler-ca.com>, casey@...aufler-ca.com, eparis@...hat.com, linux-security-module@...r.kernel.org, audit@...r.kernel.org
Cc: jmorris@...ei.org, serge@...lyn.com, keescook@...omium.org, john.johansen@...onical.com, penguin-kernel@...ove.sakura.ne.jp, stephen.smalley.work@...il.com, linux-kernel@...r.kernel.org, selinux@...r.kernel.org
Subject: Re: [PATCH v3 3/5] Audit: Add record for multiple task security contexts
On Mar 19, 2025 Casey Schaufler <casey@...aufler-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 (none are currently defined) 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 (1423) record is:
>
> type=MAC_TASK_CONTEXTS[1423]
> 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.
>
> Suggested-by: Paul Moore <paul@...l-moore.com>
> Signed-off-by: Casey Schaufler <casey@...aufler-ca.com>
> ---
> include/linux/audit.h | 6 ++
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 171 ++++++++++++++++++++++++++++++-------
> security/apparmor/lsm.c | 3 +
> security/selinux/hooks.c | 3 +
> security/smack/smack_lsm.c | 3 +
> 6 files changed, 158 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 0050ef288ab3..b493ca5976cf 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -37,6 +37,7 @@ struct audit_watch;
> struct audit_tree;
> struct sk_buff;
> struct kern_ipc_perm;
> +struct lsm_id;
>
> struct audit_krule {
> u32 pflags;
> @@ -210,6 +211,8 @@ extern u32 audit_enabled;
>
> extern int audit_signal_info(int sig, struct task_struct *t);
>
> +extern void audit_lsm_secctx(const struct lsm_id *lsmid);
> +
> #else /* CONFIG_AUDIT */
> static inline __printf(4, 5)
> void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
> @@ -269,6 +272,9 @@ static inline int audit_signal_info(int sig, struct task_struct *t)
> return 0;
> }
>
> +static inline void audit_lsm_secctx(const struct lsm_id *lsmid)
> +{ }
> +
> #endif /* CONFIG_AUDIT */
>
> #ifdef CONFIG_AUDIT_COMPAT_GENERIC
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index d9a069b4a775..5ebb5d80363d 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -146,6 +146,7 @@
> #define AUDIT_IPE_ACCESS 1420 /* IPE denial or grant */
> #define AUDIT_IPE_CONFIG_CHANGE 1421 /* IPE config change */
> #define AUDIT_IPE_POLICY_LOAD 1422 /* IPE policy load */
> +#define AUDIT_MAC_TASK_CONTEXTS 1423 /* Multiple LSM task contexts */
>
> #define AUDIT_FIRST_KERN_ANOM_MSG 1700
> #define AUDIT_LAST_KERN_ANOM_MSG 1799
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 6bbadb605ca3..7ec3919ae925 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -54,6 +54,7 @@
> #include <net/netlink.h>
> #include <linux/skbuff.h>
> #include <linux/security.h>
> +#include <linux/lsm_hooks.h>
> #include <linux/freezer.h>
> #include <linux/pid_namespace.h>
> #include <net/netns/generic.h>
> @@ -81,6 +82,11 @@ static u32 audit_failure = AUDIT_FAIL_PRINTK;
> /* private audit network namespace index */
> static unsigned int audit_net_id;
>
> +/* Number of modules that provide a security context.
> + List of lsms that provide a security context */
> +static u32 audit_secctx_cnt = 0;
> +static const struct lsm_id *audit_lsms[MAX_LSM_COUNT];
We've already talked about this in other threads, offline, etc., but
for the sake of others, this should be adjusted to use the counts
provided in the LSM initialization code rework.
https://lore.kernel.org/linux-security-module/20250409185019.238841-31-paul@paul-moore.com/
> @@ -2412,26 +2517,14 @@ int audit_signal_info(int sig, struct task_struct *t)
> }
>
> /**
> - * audit_log_end - end one audit record
> - * @ab: the audit_buffer
> - *
> - * We can not do a netlink send inside an irq context because it blocks (last
> - * arg, flags, is not set to MSG_DONTWAIT), so the audit buffer is placed on a
> - * queue and a kthread is scheduled to remove them from the queue outside the
> - * irq context. May be called in any context.
> + * __audit_log_end - enqueue one audit record
> + * @skb: the buffer to send
> */
> -void audit_log_end(struct audit_buffer *ab)
> +static void __audit_log_end(struct sk_buff *skb)
> {
> - struct sk_buff *skb;
> struct nlmsghdr *nlh;
>
> - if (!ab)
> - return;
> -
> if (audit_rate_check()) {
> - skb = ab->skb;
> - ab->skb = NULL;
> -
> /* setup the netlink header, see the comments in
> * kauditd_send_multicast_skb() for length quirks */
> nlh = nlmsg_hdr(skb);
> @@ -2442,6 +2535,26 @@ void audit_log_end(struct audit_buffer *ab)
> wake_up_interruptible(&kauditd_wait);
> } else
> audit_log_lost("rate limit exceeded");
> +}
Okay, this is twice now in one patchset ... as I mentioned in the v2
review:
"We should probably move the kauditd thread wake into
audit_log_end() so we don't end up poking the scheduler
multiple times."
https://lore.kernel.org/audit/69ee16ce82a564e09b2060d46fa2be0d@paul-moore.com/
> +/**
> + * audit_log_end - end one audit record
> + * @ab: the audit_buffer
> + *
> + * We can not do a netlink send inside an irq context because it blocks (last
> + * arg, flags, is not set to MSG_DONTWAIT), so the audit buffer is placed on a
> + * queue and a kthread is scheduled to remove them from the queue outside the
> + * irq context. May be called in any context.
> + */
> +void audit_log_end(struct audit_buffer *ab)
> +{
> + struct sk_buff *skb;
> +
> + if (!ab)
> + return;
> +
> + while ((skb = skb_dequeue(&ab->skb_list)))
> + __audit_log_end(skb);
The wakeup should go here.
> audit_buffer_free(ab);
> }
--
paul-moore.com
Powered by blists - more mailing lists