diff options
author | Pablo Neira <pablo@netfilter.org> | 2013-06-27 21:04:23 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2013-06-28 01:44:16 -0400 |
commit | 3a36515f729458c8efa0c124c7262d5843ad5c37 (patch) | |
tree | 75924228bbebccdbbbbe1c8cca9b65d121843a0a /net/netlink | |
parent | 5e6700b3bf98fe98d630bf9c939ad4c85ce95592 (diff) |
netlink: fix splat in skb_clone with large messages
Since (c05cdb1 netlink: allow large data transfers from user-space),
netlink splats if it invokes skb_clone on large netlink skbs since:
* skb_shared_info was not correctly initialized.
* skb->destructor is not set in the cloned skb.
This was spotted by trinity:
[ 894.990671] BUG: unable to handle kernel paging request at ffffc9000047b001
[ 894.991034] IP: [<ffffffff81a212c4>] skb_clone+0x24/0xc0
[...]
[ 894.991034] Call Trace:
[ 894.991034] [<ffffffff81ad299a>] nl_fib_input+0x6a/0x240
[ 894.991034] [<ffffffff81c3b7e6>] ? _raw_read_unlock+0x26/0x40
[ 894.991034] [<ffffffff81a5f189>] netlink_unicast+0x169/0x1e0
[ 894.991034] [<ffffffff81a601e1>] netlink_sendmsg+0x251/0x3d0
Fix it by:
1) introducing a new netlink_skb_clone function that is used in nl_fib_input,
that sets our special skb->destructor in the cloned skb. Moreover, handle
the release of the large cloned skb head area in the destructor path.
2) not allowing large skbuffs in the netlink broadcast path. I cannot find
any reasonable use of the large data transfer using netlink in that path,
moreover this helps to skip extra skb_clone handling.
I found two more netlink clients that are cloning the skbs, but they are
not in the sendmsg path. Therefore, the sole client cloning that I found
seems to be the fib frontend.
Thanks to Eric Dumazet for helping to address this issue.
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/netlink')
-rw-r--r-- | net/netlink/af_netlink.c | 35 |
1 files changed, 18 insertions, 17 deletions
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 6967fbcca6c5..0c61b59175dc 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c | |||
@@ -849,7 +849,10 @@ static void netlink_skb_destructor(struct sk_buff *skb) | |||
849 | } | 849 | } |
850 | #endif | 850 | #endif |
851 | if (is_vmalloc_addr(skb->head)) { | 851 | if (is_vmalloc_addr(skb->head)) { |
852 | vfree(skb->head); | 852 | if (!skb->cloned || |
853 | !atomic_dec_return(&(skb_shinfo(skb)->dataref))) | ||
854 | vfree(skb->head); | ||
855 | |||
853 | skb->head = NULL; | 856 | skb->head = NULL; |
854 | } | 857 | } |
855 | if (skb->sk != NULL) | 858 | if (skb->sk != NULL) |
@@ -1532,33 +1535,31 @@ struct sock *netlink_getsockbyfilp(struct file *filp) | |||
1532 | return sock; | 1535 | return sock; |
1533 | } | 1536 | } |
1534 | 1537 | ||
1535 | static struct sk_buff *netlink_alloc_large_skb(unsigned int size) | 1538 | static struct sk_buff *netlink_alloc_large_skb(unsigned int size, |
1539 | int broadcast) | ||
1536 | { | 1540 | { |
1537 | struct sk_buff *skb; | 1541 | struct sk_buff *skb; |
1538 | void *data; | 1542 | void *data; |
1539 | 1543 | ||
1540 | if (size <= NLMSG_GOODSIZE) | 1544 | if (size <= NLMSG_GOODSIZE || broadcast) |
1541 | return alloc_skb(size, GFP_KERNEL); | 1545 | return alloc_skb(size, GFP_KERNEL); |
1542 | 1546 | ||
1543 | skb = alloc_skb_head(GFP_KERNEL); | 1547 | size = SKB_DATA_ALIGN(size) + |
1544 | if (skb == NULL) | 1548 | SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); |
1545 | return NULL; | ||
1546 | 1549 | ||
1547 | data = vmalloc(size); | 1550 | data = vmalloc(size); |
1548 | if (data == NULL) | 1551 | if (data == NULL) |
1549 | goto err; | 1552 | return NULL; |
1550 | 1553 | ||
1551 | skb->head = data; | 1554 | skb = build_skb(data, size); |
1552 | skb->data = data; | 1555 | if (skb == NULL) |
1553 | skb_reset_tail_pointer(skb); | 1556 | vfree(data); |
1554 | skb->end = skb->tail + size; | 1557 | else { |
1555 | skb->len = 0; | 1558 | skb->head_frag = 0; |
1556 | skb->destructor = netlink_skb_destructor; | 1559 | skb->destructor = netlink_skb_destructor; |
1560 | } | ||
1557 | 1561 | ||
1558 | return skb; | 1562 | return skb; |
1559 | err: | ||
1560 | kfree_skb(skb); | ||
1561 | return NULL; | ||
1562 | } | 1563 | } |
1563 | 1564 | ||
1564 | /* | 1565 | /* |
@@ -2244,7 +2245,7 @@ static int netlink_sendmsg(struct kiocb *kiocb, struct socket *sock, | |||
2244 | if (len > sk->sk_sndbuf - 32) | 2245 | if (len > sk->sk_sndbuf - 32) |
2245 | goto out; | 2246 | goto out; |
2246 | err = -ENOBUFS; | 2247 | err = -ENOBUFS; |
2247 | skb = netlink_alloc_large_skb(len); | 2248 | skb = netlink_alloc_large_skb(len, dst_group); |
2248 | if (skb == NULL) | 2249 | if (skb == NULL) |
2249 | goto out; | 2250 | goto out; |
2250 | 2251 | ||