On 25 August 2026 01:08:46 BST, Paul Moore <[email protected]> wrote:
>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.
Hmm, interesting, yeah that works.
>
>> /* can we send to anyone via unicast? */
>> if (!sk) {
>> if (err_hook)
>
>--
>paul-moore.com
>
Thanks!
