netdev - Re: [PATCH net-next 3/4] 6lowpan: the two bytes of u16 tag needs to be stored/accessed the other way around

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 13 Jun 2012 00:42:32 -0400
From:	Tony Cheneau <tony.cheneau@...esiak.org>
To:	Alexander Smirnov <alex.bluesman.smirnov@...il.com>
Cc:	netdev@...r.kernel.org, linux-zigbee-devel@...ts.sourceforge.net
Subject: Re: [PATCH net-next 3/4] 6lowpan: the two bytes of u16 tag needs to
 be stored/accessed the other way around

Hello Alexander,

Please see my response inline.

Le Tue, 12 Jun 2012 22:07:12 +0400,
Alexander Smirnov <alex.bluesman.smirnov@...il.com> a écrit :

> 2012/6/11 Tony Cheneau <tony.cheneauh@...esiak.org>:
> > Or else, tag values, as displayed with a trafic analyser, such a
> > Wireshark, are not properly displayed (e.g. 0x01 00 insted of 0x00
> > 01, and so on). ---
> >  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 af4f29b..af2f12e 100644
> > --- a/net/ieee802154/6lowpan.c
> > +++ b/net/ieee802154/6lowpan.c
> > @@ -307,7 +307,7 @@ static u16 lowpan_fetch_skb_u16(struct sk_buff
> > *skb)
> >
> >        BUG_ON(!pskb_may_pull(skb, 2));
> >
> > -       ret = skb->data[0] | (skb->data[1] << 8);
> > +       ret = (skb->data[0] << 8) | skb->data[1];
> 
> This function aimed to obtain u16 from skb, why did you change the
> byte-order here instead of 'tag' variable byte-shifting? 
This probably reflects my lack of practice in writing/reading network
code. I witnessed in my network that the byte order was wrong when
sending/receiving packet (using Wireshark) and though it would be a
quick fix. Also, I assumed skb->data would store data using network
ordering, so that skb->data[0] would have been the MSB and
skb->data[1] would have been the LSB. I'll look more into that.

> Will this
> code work properly on the both little and big-endian machines?
I haven't checked that. But, just for clarification, if my changes are
not working properly on both architectures, it means that the original
code doesn't either, right?

> Please
> rework this to keep order properly for all the architectures.
I will read some more network code to see how it is handled in other
part of the kernel and rewrite the patch accordingly (if at all
needed). 

Thank you for your advises.

> >        skb_pull(skb, 2);
> >        return ret;
> >  }
> > @@ -1002,8 +1002,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
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regards,
	Tony
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html