Log corruption

[Chris Wright]

* Steve Grubb (sgrubb at redhat.com) wrote:
> On Sunday 08 May 2005 09:47, Steve Grubb wrote:
> > Still testing the new kernel. I've noticed that there is now log corruption
> > since we added the netlink patches
> 
> I put a printk into the kernel to see what's happening. The nlmsg_len was set 
> for 85 bytes when it should have been 69. I've found that it seems to be 
> consistently 16 bytes too long. I also checked tail - data and that is also 
> 16 bytes too big.  
> 
> I did some more digging around. It turns out that the netlink header is 16 
> bytes long. I looked at how the kernel used to calculate nlmsg_len. 
> Basically, it is the payload length.
> 
> I also found this line in send_reply:
> 
>         nlh              = NLMSG_PUT(skb, pid, seq, t, len - sizeof(*nlh));
> 
> It seems to be subtracting the size of the netlink header to determine the 
> length. So, it seems that the fix is to change audit_log_drain to be 
> something like this:
> 
>                 if (audit_pid) {
>                         struct nlmsghdr *nlh = (struct nlmsghdr *)skb->data;
> -->                    nlh->nlmsg_len = skb->len - sizeof(*nlh);
>                         skb_get(skb); /* because netlink_* frees */
>                         retval = netlink_unicast(audit_sock, skb, audit_pid,
>                                                  MSG_DONTWAIT);
>                 }
> 
> Does anyone see anything wrong with that?

It's (mostly) fine.  Sorry, that's my fault.  I actually used my own
auditd I wrote for most of my testing, and just used the normal auditd
for sanity checking, and I account for header in userspace.  Technically,
the kernel side is right, and the userspace side should be updated (the
nlmsg_len is header, plus data, plus alignment), but since we own both
sides (and don't send multiple netlink messages in a single buffer),
it's fine.  However, it's not quite accurate...

Should be subtracting off NLMSG_SPACE(0) to get proper alignment.  I'm
still re-syncing against git, or I'd send a patch.

thanks,
-chris