[Top] [All Lists]

[NETLINK] Do not netlink_unicast shared packet in kernel/audit.c

To: Tommy Christensen <tommy.christensen@xxxxxxxxx>
Subject: [NETLINK] Do not netlink_unicast shared packet in kernel/audit.c
From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date: Sun, 16 Jan 2005 19:02:54 +1100
Cc: simon.roscic@xxxxxxxxx, netdev@xxxxxxxxxxx, davem@xxxxxxxxxxxxx
In-reply-to: <41E97B1A.30205@xxxxxxxxx>
References: <E1Cpo1q-0007lI-00@xxxxxxxxxxxxxxxxxxxxxxxx> <41E942AF.3030202@xxxxxxxxx> <20050115183023.GA31211@xxxxxxxxxxxxxxxxxxx> <41E97B1A.30205@xxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.6+20040722i
On Sat, Jan 15, 2005 at 09:20:42PM +0100, Tommy Christensen wrote:
> Well, pskb_expand_head was added recently and isn't even always called.
> Have a look at audit_log_drain() in kernel/audit.c. This code does:
>   skb_get
>   netlink_unicast
>   ...
>   kfree_skb
> This is working just fine - until one day an over-sized skb comes along
> and hits the BUG in pskb_expand_head ...

You're always right :)
> Still, regarding netlink, I think the safe approach is to either handle
> or disallow shared skb's. Anything in-between could appear to be working,
> and then suddenly blow up on your production system. </paranoia>

kernel/audit.c seems to be the only caller that does this.  Strictly
speaking shared skb's are always illegal in netlink_unicast/netlink_broadcast
since we call functions such as skb_orphan which modifies the sk_buff
structure.  However, for callers such as kernel/audit.c that don't care
about those fields, it actually turns out to be harmless.

I'm hesitant to put an outright ban on it since it will force callers
such as kernel/audit.c to always do an skb_clone.

So here is a patch to handle shared skb's by cloning them if we're
going to do a pskb_expand_head.  This patch depends on the previous
one that moves skb_orphan into netlink_trim.

Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

Visit Openswan at
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page:
PGP Key:

Attachment: p
Description: Text document

<Prev in Thread] Current Thread [Next in Thread>