Re: [PATCH] audit: move the nlmsg_len fixup from __audit_log_end() to send time

[Ricardo Robaina]

On Thu, Aug 20, 2026 at 12:14 PM Bradley Morgan <include@xxxxxxxxx> wrote:
>
> On 14 August 2026 02:01:53 BST, Bradley Morgan <include@xxxxxxxxx> 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 <rrobaina@xxxxxxxxxx>
> >Tested-by: Ricardo Robaina <rrobaina@xxxxxxxxxx>
> >Signed-off-by: Bradley Morgan <include@xxxxxxxxx>
> >Link: https://lore.kernel.org/r/20260810125726.775689-2-rrobaina@xxxxxxxxxx
> >---
> > 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;
> >+
> >               /* can we send to anyone via unicast? */
> >               if (!sk) {
> >                       if (err_hook)
> >@@ -849,7 +855,6 @@ static void kauditd_send_multicast_skb(struct sk_buff *skb)
> > {
> >       struct sk_buff *copy;
> >       struct sock *sock = audit_get_sk(&init_net);
> >-      struct nlmsghdr *nlh;
> >
> >       /* NOTE: we are not taking an additional reference for init_net since
> >        *       we don't have to worry about it going away */
> >@@ -858,20 +863,12 @@ static void kauditd_send_multicast_skb(struct sk_buff *skb)
> >               return;
> >
> >       /*
> >-       * The seemingly wasteful skb_copy() rather than bumping the refcount
> >-       * using skb_get() is necessary because non-standard mods are made to
> >-       * the skb by the original kaudit unicast socket send routine.  The
> >-       * existing auditd daemon assumes this breakage.  Fixing this would
> >-       * require co-ordinating a change in the established protocol between
> >-       * the kaudit kernel subsystem and the auditd userspace code.  There is
> >-       * no reason for new multicast clients to continue with this
> >-       * non-compliance.
> >+       * skb_copy() rather than skb_get(): kauditd_send_queue() breaks
> >+       * nlmsg_len for auditd, keep the listeners on a standard message.
> >        */
> >       copy = skb_copy(skb, GFP_KERNEL);
> >       if (!copy)
> >               return;
> >-      nlh = nlmsg_hdr(copy);
> >-      nlh->nlmsg_len = skb->len;
> >
> >       nlmsg_multicast(sock, copy, 0, AUDIT_NLGRP_READLOG, GFP_KERNEL);
> > }
> >@@ -2785,13 +2782,12 @@ int audit_signal_info(int sig, struct task_struct *t)
> >  */
> > static void __audit_log_end(struct sk_buff *skb)
> > {
> >-      struct nlmsghdr *nlh;
> >-
> >       if (audit_rate_check()) {
> >-              /* setup the netlink header, see the comments in
> >-               * kauditd_send_multicast_skb() for length quirks */
> >-              nlh = nlmsg_hdr(skb);
> >-              nlh->nlmsg_len = skb->len - NLMSG_HDRLEN;
> >+              /*
> >+               * Records are built without keeping nlmsg_len up to date,
> >+               * finalize it here with the full message length.
> >+               */
> >+              nlmsg_end(skb, nlmsg_hdr(skb));
> >
> >               /* queue the netlink packet */
> >               skb_queue_tail(&audit_queue, skb);
> >
> add akpm:, I seem to be getting ignored on these kind of patches, very
> annoyingly, any reason why in particular?
>
> I'm getting slightly, could say angry, could say annoyed, either one.
> Thanks!
>

Hi Bradley,

Paul will probably explain better, but see the seesion "Don’t get
discouraged - or impatient" in [1], in the meantime. There was a merge
window going on recently.

[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#don-t-get-discouraged-or-impatient

-Ricardo