* [PATCH net-next v3 1/3] 6lowpan: Fix null pointer dereference in UDP uncompression function
2012-07-11 16:51 [PATCH net-next v3 0/3] 6lowpan: Various bug fixes Tony Cheneau
@ 2012-07-11 16:51 ` Tony Cheneau
2012-07-11 16:51 ` [PATCH net-next v3 2/3] 6lowpan: Change byte order when storing/accessing u16 tag Tony Cheneau
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Tony Cheneau @ 2012-07-11 16:51 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Alexander Smirnov
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 <tony.cheneau@amnesiak.org>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH net-next v3 2/3] 6lowpan: Change byte order when storing/accessing u16 tag
2012-07-11 16:51 [PATCH net-next v3 0/3] 6lowpan: Various bug fixes Tony Cheneau
2012-07-11 16:51 ` [PATCH net-next v3 1/3] 6lowpan: Fix null pointer dereference in UDP uncompression function Tony Cheneau
@ 2012-07-11 16:51 ` Tony Cheneau
2012-07-11 16:51 ` [PATCH net-next v3 3/3] 6lowpan: Change byte order when storing/accessing to len field Tony Cheneau
2012-07-17 5:52 ` [PATCH net-next v3 0/3] 6lowpan: Various bug fixes David Miller
3 siblings, 0 replies; 5+ messages in thread
From: Tony Cheneau @ 2012-07-11 16:51 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Alexander Smirnov
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 <tony.cheneau@amnesiak.org>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next v3 3/3] 6lowpan: Change byte order when storing/accessing to len field
2012-07-11 16:51 [PATCH net-next v3 0/3] 6lowpan: Various bug fixes Tony Cheneau
2012-07-11 16:51 ` [PATCH net-next v3 1/3] 6lowpan: Fix null pointer dereference in UDP uncompression function Tony Cheneau
2012-07-11 16:51 ` [PATCH net-next v3 2/3] 6lowpan: Change byte order when storing/accessing u16 tag Tony Cheneau
@ 2012-07-11 16:51 ` Tony Cheneau
2012-07-17 5:52 ` [PATCH net-next v3 0/3] 6lowpan: Various bug fixes David Miller
3 siblings, 0 replies; 5+ messages in thread
From: Tony Cheneau @ 2012-07-11 16:51 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Alexander Smirnov
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 <tony.cheneau@amnesiak.org>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net-next v3 0/3] 6lowpan: Various bug fixes
2012-07-11 16:51 [PATCH net-next v3 0/3] 6lowpan: Various bug fixes Tony Cheneau
` (2 preceding siblings ...)
2012-07-11 16:51 ` [PATCH net-next v3 3/3] 6lowpan: Change byte order when storing/accessing to len field Tony Cheneau
@ 2012-07-17 5:52 ` David Miller
3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2012-07-17 5:52 UTC (permalink / raw)
To: tony.cheneau; +Cc: netdev, alex.bluesman.smirnov
From: Tony Cheneau <tony.cheneau@amnesiak.org>
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.
^ permalink raw reply [flat|nested] 5+ messages in thread