On Aug 13, 2026 Bradley Morgan <[email protected]> wrote:
>
> Right now the auditd breakage (nlmsg_len gets set to the payload
> length instead of the full message length) is applied when the record
> is queued, in __audit_log_end(). That is why
> kauditd_send_multicast_skb() has to deep copy every record and then
> undo the length on the copy, just so the multicast group still sees a
> standard netlink message.
>
> So flip it: finalize the header with the standard full length at
> queue time, and apply the auditd length at send time in
> kauditd_send_queue(), right before the unicast. Records stay standard
> netlink messages the whole time they sit in the queues, and the
> multicast copy stops needing its own fixup. The copy itself stays,
> because the rewrite still lands in the data region the listeners
> already hold.
>
> auditd sees the same bytes as before: the fixup is computed from
> skb->len and that does not change between queueing and sending, so
> records that come back around through the retry and hold queues get
> the same value again. Reply and rule list skbs are built with
> nlmsg_put() and go out on their own paths, none of that is touched.
>
> This came out of reviewing Ricardo's "use copied skb length" patch,
> where I suggested moving the fixup as the more interesting cleanup.
>
> Reviewed-by: Ricardo Robaina <[email protected]>
> Tested-by: Ricardo Robaina <[email protected]>
> Signed-off-by: Bradley Morgan <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> ---
> kernel/audit.c | 30 +++++++++++++-----------------
> 1 file changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 9412af9144bc..bcfed6e3678e 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -802,6 +802,12 @@ static int kauditd_send_queue(struct sock *sk, u32
> portid,
> if (skb_hook)
> (*skb_hook)(skb);
>
> + /*
> + * auditd wants nlmsg_len to be the payload length, not the
> + * full length, so break it here at send time.
> + */
> + nlmsg_hdr(skb)->nlmsg_len = skb->len - NLMSG_HDRLEN;
One of the reasons we set the length in __audit_log_end() is so that we
only do it once, plus the multicast fixup. In this patch we still set the
length in __audit_log_end() as well as potentially multiple times in
kauditd_send_queue(). This patch does drop the multicast fixup, but it
isn't as clean a solution conceptually as the current code in my opinion.
Ultimately, in this patch we still have at least two length calculations
with the potential for additional unnecessary calculations/assignments if
we need to loop through kauditd_send_queue() multiple times. The existing
code is capped at two calculations/assignments. I appreciate the time
you've put into this, but I think the existing code is the better option
at this point in time.
> /* can we send to anyone via unicast? */
> if (!sk) {
> if (err_hook)
--
paul-moore.com
