On 20 August 2026 16:57:12 BST, Ricardo Robaina <[email protected]> wrote: >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 > > Sigh, [1]
This ignoring seems to be a deliberate thing, NOTE THAT THAT WAS MY FIRST PATCH ever after studying for god knows how long. I have the right to be annoyed [1] https://lore.kernel.org/all/[email protected]/ Thanks!
