Re: [PATCH] audit: fix size of netlink messages

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
To: Eric Paris <eparis@redhat.com>
Cc: Thomas Graf <tgraf@suug.ch>,
	linux-audit@redhat.com, viro@zeniv.linux.org.uk
Subject: Re: [PATCH] audit: fix size of netlink messages
Date: Fri, 07 Jun 2013 18:24:02 +0200	[thread overview]
Message-ID: <51B20922.7070909@6wind.com> (raw)
In-Reply-To: <1370619781.3184.25.camel@localhost>

Put Thomas in CC.

Le 07/06/2013 17:43, Eric Paris a écrit :
> On Fri, 2013-06-07 at 17:25 +0200, Nicolas Dichtel wrote:
>
> NAK.
>
> I tried this once before and as I recal userspace actually expected the
> stoopidity of being unaligned and broke   :-(
On which userspace tools do you think?

For example, in the libnl, the function which tries to get the next netlink 
message expects this alignment:

struct nlmsghdr *nlmsg_next(struct nlmsghdr *nlh, int *remaining)
{
         int totlen = NLMSG_ALIGN(nlh->nlmsg_len);

         *remaining -= totlen;

         return (struct nlmsghdr *) ((unsigned char *) nlh + totlen);
}

http://www.infradead.org/~tgr/libnl/doc/core.html#_message_parsing_amp_construction

>
>> Netlink messages must be aligned on NLMSG_ALIGNTO (4 bytes), thus we need to
>> update the skb length before sending it to userspace.
>>
>> This patch adds the needed padding to be compliant with this requirement.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> ---
>>   kernel/audit.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/kernel/audit.c b/kernel/audit.c
>> index 21c7fa6..31d213a 100644
>> --- a/kernel/audit.c
>> +++ b/kernel/audit.c
>> @@ -1669,6 +1669,17 @@ void audit_log_end(struct audit_buffer *ab)
>>   		struct nlmsghdr *nlh = nlmsg_hdr(ab->skb);
>>   		nlh->nlmsg_len = ab->skb->len - NLMSG_HDRLEN;
>>
>> +		if (NLMSG_ALIGN(ab->skb->len) != ab->skb->len) {
Here, I think it's better to use nlmsg_padlen().

>> +			unsigned int pad = NLMSG_ALIGN(ab->skb->len) -
>> +					   ab->skb->len;
>> +
>> +			if (skb_tailroom(ab->skb) >= pad)
>> +				skb_put(ab->skb, pad);
>> +			else if (pskb_expand_head(ab->skb, 0, pad,
>> +						  GFP_KERNEL) < 0)
>> +				audit_log_lost("out of memory in audit_log_end");
>> +		}
>> +
>>   		if (audit_pid) {
>>   			skb_queue_tail(&audit_skb_queue, ab->skb);
>>   			wake_up_interruptible(&kauditd_wait);
>
>

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit