Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3a36515f authored by Pablo Neira's avatar Pablo Neira Committed by David S. Miller
Browse files

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: default avatarFengguang Wu <fengguang.wu@intel.com>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 5e6700b3

include/linux/netlink.h

+16 −0
Original line number Diff line number Diff line
@@ -85,6 +85,22 @@ int netlink_attachskb(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);



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.

 *	But we should limit to 8K so that userspace does not have to

net/ipv4/fib_frontend.c

+1 −1
Original line number Diff line number Diff line
@@ -961,7 +961,7 @@ static void nl_fib_input(struct sk_buff *skb) 
	    nlmsg_len(nlh) < sizeof(*frn))

		return;



	skb = skb_clone(skb, GFP_KERNEL);

	skb = netlink_skb_clone(skb, GFP_KERNEL);

	if (skb == NULL)

		return;

	nlh = nlmsg_hdr(skb);

net/netlink/af_netlink.c

+18 −17
Original line number Diff line number Diff line
@@ -849,7 +849,10 @@ static void netlink_skb_destructor(struct sk_buff *skb) 
	}

#endif

	if (is_vmalloc_addr(skb->head)) {

		if (!skb->cloned ||

		    !atomic_dec_return(&(skb_shinfo(skb)->dataref)))

			vfree(skb->head);



		skb->head = NULL;

	}

	if (skb->sk != NULL)
@@ -1532,33 +1535,31 @@ struct sock *netlink_getsockbyfilp(struct file *filp) 
	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;

	void *data;



	if (size <= NLMSG_GOODSIZE)

	if (size <= NLMSG_GOODSIZE || broadcast)

		return alloc_skb(size, GFP_KERNEL);



	skb = alloc_skb_head(GFP_KERNEL);

	if (skb == NULL)

		return NULL;

	size = SKB_DATA_ALIGN(size) +

	       SKB_DATA_ALIGN(sizeof(struct skb_shared_info));



	data = vmalloc(size);

	if (data == NULL)

		goto err;

		return NULL;



	skb->head	= data;

	skb->data	= data;

	skb_reset_tail_pointer(skb);

	skb->end	= skb->tail + size;

	skb->len	= 0;

	skb = build_skb(data, size);

	if (skb == NULL)

		vfree(data);

	else {

		skb->head_frag = 0;

		skb->destructor = netlink_skb_destructor;

	}



	return skb;

err:

	kfree_skb(skb);

	return NULL;

}



/*
@@ -2244,7 +2245,7 @@ static int netlink_sendmsg(struct kiocb *kiocb, struct socket *sock, 
	if (len > sk->sk_sndbuf - 32)

		goto out;

	err = -ENOBUFS;

	skb = netlink_alloc_large_skb(len);

	skb = netlink_alloc_large_skb(len, dst_group);

	if (skb == NULL)

		goto out;