[PATCH net-next v3 0/3] 6lowpan: Various bug fixes

Hello,

After reading and playing with the 6lowpan code, I found out a few issues. This
patchset fixes them. This patchset should apply cleanly against the current
net-next. It contains only bug fixes, I'll send later on an other patchset that
will contain new functionalities.

Changes since version 2:
- remove a patch that prevented fragmentation to work after few packets have
  been send: Alexander included the patch in his patchset
- fix the title of the git commit to include the "6lowpan" tag

Regards,
	Tony Cheneau

Tony Cheneau (3):
  Fix null pointer dereference in UDP uncompression function
  Change byte order when storing/accessing u16 tag
  Change byte order when storing/accessing to len field

 net/ieee802154/6lowpan.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

-- 
1.7.3.4

When a UDP packet gets fragmented, a crash will occur at reassembly time.
This is because skb->transport_header is not set during earlier period of fragment reassembly.
As a consequence, call to udp_hdr() return NULL and uh (which is NULL) gets
dereferenced without much test.

Signed-off-by: Tony Cheneau <redacted>
---
 net/ieee802154/6lowpan.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index f4070e5..0c9f6d1 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c

@@ -315,6 +315,9 @@ lowpan_uncompress_udp_header(struct sk_buff *skb)
 	struct udphdr *uh = udp_hdr(skb);
 	u8 tmp;
 
+	if (!uh)
+		goto err;
+
 	if (lowpan_fetch_skb_u8(skb, &tmp))
 		goto err;
 

-- 
1.7.3.4

The tag field should be stored and accessed using big endian byte order (as
intended in the specs). Or else, when displayed with a trafic analyser, such a
Wireshark, the field not properly displayed (e.g. 0x01 00 instead of 0x00 01,
and so on).

Signed-off-by: Tony Cheneau <redacted>
---
 net/ieee802154/6lowpan.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 0c9f6d1..9de1ece 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c

@@ -303,7 +303,7 @@ static inline int lowpan_fetch_skb_u16(struct sk_buff *skb, u16 *val)
 	if (unlikely(!pskb_may_pull(skb, 2)))
 		return -EINVAL;
 
-	*val = skb->data[0] | (skb->data[1] << 8);
+	*val = (skb->data[0] << 8) | skb->data[1];
 	skb_pull(skb, 2);
 
 	return 0;

@@ -1010,8 +1010,8 @@ lowpan_skb_fragmentation(struct sk_buff *skb)
 	/* first fragment header */
 	head[0] = LOWPAN_DISPATCH_FRAG1 | (payload_length & 0x7);
 	head[1] = (payload_length >> 3) & 0xff;
-	head[2] = tag & 0xff;
-	head[3] = tag >> 8;
+	head[2] = tag >> 8;
+	head[3] = tag & 0xff;
 
 	err = lowpan_fragment_xmit(skb, head, header_length, 0, 0);
 

-- 
1.7.3.4

Lenght field should be encoded using big endian byte order, such as intend in the specs.
As it is currently written, the len field would not be decoded properly on an implementation using the correct byte ordering. Hence, it could lead to interroperability issues.

Also, I rewrote the code so that iphc0 argument of lowpan_alloc_new_frame could be removed.

Signed-off-by: Tony Cheneau <redacted>
---
 net/ieee802154/6lowpan.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 9de1ece..75d91bb 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c

@@ -649,7 +649,7 @@ static void lowpan_fragment_timer_expired(unsigned long entry_addr)
 }
 
 static struct lowpan_fragment *
-lowpan_alloc_new_frame(struct sk_buff *skb, u8 iphc0, u8 len, u8 tag)
+lowpan_alloc_new_frame(struct sk_buff *skb, u8 len, u8 tag)
 {
 	struct lowpan_fragment *frame;
 

@@ -660,7 +660,7 @@ lowpan_alloc_new_frame(struct sk_buff *skb, u8 iphc0, u8 len, u8 tag)
 
 	INIT_LIST_HEAD(&frame->list);
 
-	frame->length = (iphc0 & 7) | (len << 3);
+	frame->length = len;
 	frame->tag = tag;
 
 	/* allocate buffer for frame assembling */

@@ -718,14 +718,18 @@ lowpan_process_data(struct sk_buff *skb)
 	case LOWPAN_DISPATCH_FRAGN:
 	{
 		struct lowpan_fragment *frame;
-		u8 len, offset;
-		u16 tag;
+		/* slen stores the rightmost 8 bits of the 11 bits length */
+		u8 slen, offset;
+		u16 len, tag;
 		bool found = false;
 
-		if (lowpan_fetch_skb_u8(skb, &len) || /* frame length */
+		if (lowpan_fetch_skb_u8(skb, &slen) || /* frame length */
 		    lowpan_fetch_skb_u16(skb, &tag))  /* fragment tag */
 			goto drop;
 
+		/* adds the 3 MSB to the 8 LSB to retrieve the 11 bits length */
+		len = ((iphc0 & 7) << 8) | slen;
+
 		/*
 		 * check if frame assembling with the same tag is
 		 * already in progress

@@ -740,7 +744,7 @@ lowpan_process_data(struct sk_buff *skb)
 
 		/* alloc new frame structure */
 		if (!found) {
-			frame = lowpan_alloc_new_frame(skb, iphc0, len, tag);
+			frame = lowpan_alloc_new_frame(skb, len, tag);
 			if (!frame)
 				goto unlock_and_drop;
 		}

@@ -1008,8 +1012,8 @@ lowpan_skb_fragmentation(struct sk_buff *skb)
 	tag = fragment_tag++;
 
 	/* first fragment header */
-	head[0] = LOWPAN_DISPATCH_FRAG1 | (payload_length & 0x7);
-	head[1] = (payload_length >> 3) & 0xff;
+	head[0] = LOWPAN_DISPATCH_FRAG1 | ((payload_length >> 8) & 0x7);
+	head[1] = payload_length & 0xff;
 	head[2] = tag >> 8;
 	head[3] = tag & 0xff;
 

-- 
1.7.3.4

From: Tony Cheneau <redacted>
Date: Wed, 11 Jul 2012 12:51:13 -0400

After reading and playing with the 6lowpan code, I found out a few issues. This
patchset fixes them. This patchset should apply cleanly against the current
net-next. It contains only bug fixes, I'll send later on an other patchset that
will contain new functionalities.

Changes since version 2:
- remove a patch that prevented fragmentation to work after few packets have
  been send: Alexander included the patch in his patchset
- fix the title of the git commit to include the "6lowpan" tag

Series applied, thanks.