提交 3a36515f 编写于 作者: P Pablo Neira 提交者: David S. Miller

netlink: fix splat in skb_clone with large messages

Since (c05cdb1b 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: NFengguang Wu <fengguang.wu@intel.com>
Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>
上级 5e6700b3
...@@ -85,6 +85,22 @@ int netlink_attachskb(struct sock *sk, struct sk_buff *skb, ...@@ -85,6 +85,22 @@ int netlink_attachskb(struct sock *sk, struct sk_buff *skb,
void netlink_detachskb(struct sock *sk, struct sk_buff *skb); void netlink_detachskb(struct sock *sk, struct sk_buff *skb);
int netlink_sendskb(struct sock *sk, struct sk_buff *skb); int netlink_sendskb(struct sock *sk, struct sk_buff *skb);
static inline struct sk_buff *
netlink_skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
{
struct sk_buff *nskb;
nskb = skb_clone(skb, gfp_mask);
if (!nskb)
return NULL;
/* This is a large skb, set destructor callback to release head */
if (is_vmalloc_addr(skb->head))
nskb->destructor = skb->destructor;
return nskb;
}
/* /*
* skb should fit one page. This choice is good for headerless malloc. * skb should fit one page. This choice is good for headerless malloc.
* But we should limit to 8K so that userspace does not have to * But we should limit to 8K so that userspace does not have to
......
...@@ -961,7 +961,7 @@ static void nl_fib_input(struct sk_buff *skb) ...@@ -961,7 +961,7 @@ static void nl_fib_input(struct sk_buff *skb)
nlmsg_len(nlh) < sizeof(*frn)) nlmsg_len(nlh) < sizeof(*frn))
return; return;
skb = skb_clone(skb, GFP_KERNEL); skb = netlink_skb_clone(skb, GFP_KERNEL);
if (skb == NULL) if (skb == NULL)
return; return;
nlh = nlmsg_hdr(skb); nlh = nlmsg_hdr(skb);
......
...@@ -849,7 +849,10 @@ static void netlink_skb_destructor(struct sk_buff *skb) ...@@ -849,7 +849,10 @@ static void netlink_skb_destructor(struct sk_buff *skb)
} }
#endif #endif
if (is_vmalloc_addr(skb->head)) { if (is_vmalloc_addr(skb->head)) {
if (!skb->cloned ||
!atomic_dec_return(&(skb_shinfo(skb)->dataref)))
vfree(skb->head); vfree(skb->head);
skb->head = NULL; skb->head = NULL;
} }
if (skb->sk != NULL) if (skb->sk != NULL)
...@@ -1532,33 +1535,31 @@ struct sock *netlink_getsockbyfilp(struct file *filp) ...@@ -1532,33 +1535,31 @@ struct sock *netlink_getsockbyfilp(struct file *filp)
return sock; return sock;
} }
static struct sk_buff *netlink_alloc_large_skb(unsigned int size) static struct sk_buff *netlink_alloc_large_skb(unsigned int size,
int broadcast)
{ {
struct sk_buff *skb; struct sk_buff *skb;
void *data; void *data;
if (size <= NLMSG_GOODSIZE) if (size <= NLMSG_GOODSIZE || broadcast)
return alloc_skb(size, GFP_KERNEL); return alloc_skb(size, GFP_KERNEL);
skb = alloc_skb_head(GFP_KERNEL); size = SKB_DATA_ALIGN(size) +
if (skb == NULL) SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
return NULL;
data = vmalloc(size); data = vmalloc(size);
if (data == NULL) if (data == NULL)
goto err; return NULL;
skb->head = data; skb = build_skb(data, size);
skb->data = data; if (skb == NULL)
skb_reset_tail_pointer(skb); vfree(data);
skb->end = skb->tail + size; else {
skb->len = 0; skb->head_frag = 0;
skb->destructor = netlink_skb_destructor; skb->destructor = netlink_skb_destructor;
}
return skb; return skb;
err:
kfree_skb(skb);
return NULL;
} }
/* /*
...@@ -2244,7 +2245,7 @@ static int netlink_sendmsg(struct kiocb *kiocb, struct socket *sock, ...@@ -2244,7 +2245,7 @@ static int netlink_sendmsg(struct kiocb *kiocb, struct socket *sock,
if (len > sk->sk_sndbuf - 32) if (len > sk->sk_sndbuf - 32)
goto out; goto out;
err = -ENOBUFS; err = -ENOBUFS;
skb = netlink_alloc_large_skb(len); skb = netlink_alloc_large_skb(len, dst_group);
if (skb == NULL) if (skb == NULL)
goto out; goto out;
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册