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>
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
p
Description: Text document
|