Re: [PATCH v2 2/6] Audit: Allow multiple records in an audit_buffer

Paul Moore
2025-03-12T23:51:36Z

On Mar 7, 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 (none are currently defined) as have been added
> to the list. Functions are created to manage the skb list
> in the audit_buffer.
>
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> kernel/audit.c | 111 +++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 89 insertions(+), 22 deletions(-)

This may need to be squashed with a later patch as I get
"defined but not used" warnings/errors for audit_buffer_aux_{new,end}().

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 2a567f667528..a4945f1c3ec0 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2412,26 +2471,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 +2489,26 @@ void audit_log_end(struct audit_buffer *ab)
> wake_up_interruptible(&kauditd_wait);

We should probably move the kauditd thread wake into audit_log_end()
so we don't end up poking the scheduler multiple times.

> } else
> audit_log_lost("rate limit exceeded");
> +}
> +
> +/**
> + * 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);

Put the kauditd thread wake here.

> audit_buffer_free(ab);
> }
> --
> 2.47.0

--
paul-moore.com