[PATCH net-next] ieee802154: properly unshare skbs in ieee802154 *_rcv functions

[PATCH net-next] ieee802154: properly unshare skbs in ieee802154 *_rcv functions

[Phoebe Buckheister]

ieee802154 sockets do not properly unshare received skbs, which leads to
panics (at least) when they are used in conjunction with 6lowpan, so
run skb_share_check on received skbs.
6lowpan also contains a use-after-free, which is trivially fixed by
replacing the inlined skb_share_check with the explicit call.

Signed-off-by: Phoebe Buckheister <phoebe.buckheister@itwm.fraunhofer.de>
---
 net/ieee802154/6lowpan_rtnl.c |   29 +++++++++++++----------------
 net/ieee802154/dgram.c        |    3 +++
 net/ieee802154/raw.c          |    3 +++
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 602c6e0..c6d1afb 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -444,51 +444,48 @@ static int lowpan_validate(struct nlattr *tb[], struct nlattr *data[])
 static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev,
 	struct packet_type *pt, struct net_device *orig_dev)
 {
-	struct sk_buff *local_skb;
 	int ret;
 
+	skb = skb_share_check(skb, GFP_ATOMIC);
+	if (!skb)
+		goto drop;
+
 	if (!netif_running(dev))
 		goto drop_skb;
 
 	if (dev->type != ARPHRD_IEEE802154)
 		goto drop_skb;
 
-	local_skb = skb_clone(skb, GFP_ATOMIC);
-	if (!local_skb)
-		goto drop_skb;
-
-	kfree_skb(skb);
-
 	/* check that it's our buffer */
 	if (skb->data[0] == LOWPAN_DISPATCH_IPV6) {
-		local_skb->protocol = htons(ETH_P_IPV6);
-		local_skb->pkt_type = PACKET_HOST;
+		skb->protocol = htons(ETH_P_IPV6);
+		skb->pkt_type = PACKET_HOST;
 
 		/* Pull off the 1-byte of 6lowpan header. */
-		skb_pull(local_skb, 1);
+		skb_pull(skb, 1);
 
-		ret = lowpan_give_skb_to_devices(local_skb, NULL);
+		ret = lowpan_give_skb_to_devices(skb, NULL);
 		if (ret == NET_RX_DROP)
 			goto drop;
 	} else {
 		switch (skb->data[0] & 0xe0) {
 		case LOWPAN_DISPATCH_IPHC:	/* ipv6 datagram */
-			ret = process_data(local_skb);
+			ret = process_data(skb);
 			if (ret == NET_RX_DROP)
 				goto drop;
 			break;
 		case LOWPAN_DISPATCH_FRAG1:	/* first fragment header */
-			ret = lowpan_frag_rcv(local_skb, LOWPAN_DISPATCH_FRAG1);
+			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
 			if (ret == 1) {
-				ret = process_data(local_skb);
+				ret = process_data(skb);
 				if (ret == NET_RX_DROP)
 					goto drop;
 			}
 			break;
 		case LOWPAN_DISPATCH_FRAGN:	/* next fragments headers */
-			ret = lowpan_frag_rcv(local_skb, LOWPAN_DISPATCH_FRAGN);
+			ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
 			if (ret == 1) {
-				ret = process_data(local_skb);
+				ret = process_data(skb);
 				if (ret == NET_RX_DROP)
 					goto drop;
 			}
diff --git a/net/ieee802154/dgram.c b/net/ieee802154/dgram.c
index 66e0c90..4f6fba9 100644
--- a/net/ieee802154/dgram.c
+++ b/net/ieee802154/dgram.c
@@ -341,6 +341,9 @@ out:
 
 static int dgram_rcv_skb(struct sock *sk, struct sk_buff *skb)
 {
+	if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL)
+		return NET_RX_DROP;
+
 	if (sock_queue_rcv_skb(sk, skb) < 0) {
 		kfree_skb(skb);
 		return NET_RX_DROP;
diff --git a/net/ieee802154/raw.c b/net/ieee802154/raw.c
index 41f538b..fadd3bf 100644
--- a/net/ieee802154/raw.c
+++ b/net/ieee802154/raw.c
@@ -209,6 +209,9 @@ out:
 
 static int raw_rcv_skb(struct sock *sk, struct sk_buff *skb)
 {
+	if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL)
+		return NET_RX_DROP;
+
 	if (sock_queue_rcv_skb(sk, skb) < 0) {
 		kfree_skb(skb);
 		return NET_RX_DROP;
-- 
1.7.9.5

Re: [Linux-zigbee-devel] [PATCH net-next] ieee802154: properly unshare skbs in ieee802154 *_rcv functions

[Alexander Aring]

Hi Phoebe,

looks interesting but doens't apply on current net-next for me. :(
I didn't test it yet.

line 444 in this patch looks different than current net-next one.

Maybe you can respin this patch before net-next is closed. Thanks.

- Alex

Re: [Linux-zigbee-devel] [PATCH net-next] ieee802154: properly unshare skbs in ieee802154 *_rcv functions

[Phoebe Buckheister]

On Mon, March 17, 2014 5:21 pm, Alexander Aring wrote:
> Hi Phoebe,
>
> looks interesting but doens't apply on current net-next for me. :(
> I didn't test it yet.

Indeed, I've unwittingly tested whether it applies against a stale version
of net-next. Sorry for the inconvenience.

Re: [PATCH net-next] ieee802154: properly unshare skbs in ieee802154 *_rcv functions

[David Miller]

From: Phoebe Buckheister <phoebe.buckheister@itwm.fraunhofer.de>
Date: Mon, 17 Mar 2014 16:41:28 +0100

> ieee802154 sockets do not properly unshare received skbs, which leads to
> panics (at least) when they are used in conjunction with 6lowpan, so
> run skb_share_check on received skbs.
> 6lowpan also contains a use-after-free, which is trivially fixed by
> replacing the inlined skb_share_check with the explicit call.
> 
> Signed-off-by: Phoebe Buckheister <phoebe.buckheister@itwm.fraunhofer.de>

Applied, thank you.