On Thu, Aug 20, 2026 at 12:14 PM Bradley Morgan <[email protected]> wrote: > > On 14 August 2026 02:01:53 BST, 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; > >+ > > /* 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
