| Message ID | 20200913213019.4250-1-linus.luessing@c0d3.blue |
|---|---|
| State | Accepted, archived |
| Delegated to: | Simon Wunderlich |
| Headers | show |
| Series | batctl: tcpdump: Fix endianness in ICMPv6 Echo Request/Reply parsing | expand |
I wonder about the "_might_ need a conversion" as most cpu architectures run little endian. Especially x86. Wouldn't it be broken since years if it needed big → little conversion? kind regards txt.file On 9/13/20 11:30 PM, Linus Lüssing wrote: > The ICMPv6 Echo Request/Reply sequence number and id as well as the > IPv6 header length are two byte long fields and therefore might need a > conversion on a little endian system. Otherwise the output will be > broken on such a machine. > > Fixes: 35b37756f4a3 ("add IPv6 support to tcpdump parser") > Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue> > --- > tcpdump.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/tcpdump.c b/tcpdump.c > index db93681..b9edc20 100644 > --- a/tcpdump.c > +++ b/tcpdump.c > @@ -589,13 +589,15 @@ static void dump_ipv6(unsigned char *packet_buff, ssize_t buff_len, > break; > case ICMP6_ECHO_REQUEST: > printf(" echo request, id: %d, seq: %d, length: %hu\n", > - icmphdr->icmp6_id, icmphdr->icmp6_seq, > - iphdr->ip6_plen); > + ntohs(icmphdr->icmp6_id), > + ntohs(icmphdr->icmp6_seq), > + ntohs(iphdr->ip6_plen)); > break; > case ICMP6_ECHO_REPLY: > printf(" echo reply, id: %d, seq: %d, length: %hu\n", > - icmphdr->icmp6_id, icmphdr->icmp6_seq, > - iphdr->ip6_plen); > + ntohs(icmphdr->icmp6_id), > + ntohs(icmphdr->icmp6_seq), > + ntohs(iphdr->ip6_plen)); > break; > case ICMP6_TIME_EXCEEDED: > printf(" time exceeded in-transit, length %zu\n", >
On Mon, Sep 14, 2020 at 12:07:41AM +0200, txt.file@txtfile.eu wrote: > I wonder about the "_might_ need a conversion" as most cpu architectures run > little endian. Especially x86. > > Wouldn't it be broken since years if it needed big → little conversion? The tcpdump output for IPv6 echo request/reply was for sure broken for years on little endian targets. Note that most old-school router platforms are big endian, hence it worked well on the common ubiquiti and tp-link devices. > > kind regards > txt.file > > On 9/13/20 11:30 PM, Linus Lüssing wrote: > > The ICMPv6 Echo Request/Reply sequence number and id as well as the > > IPv6 header length are two byte long fields and therefore might need a > > conversion on a little endian system. Otherwise the output will be > > broken on such a machine. > > > > Fixes: 35b37756f4a3 ("add IPv6 support to tcpdump parser") > > Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue> > > --- > > tcpdump.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/tcpdump.c b/tcpdump.c > > index db93681..b9edc20 100644 > > --- a/tcpdump.c > > +++ b/tcpdump.c > > @@ -589,13 +589,15 @@ static void dump_ipv6(unsigned char *packet_buff, ssize_t buff_len, > > break; > > case ICMP6_ECHO_REQUEST: > > printf(" echo request, id: %d, seq: %d, length: %hu\n", > > - icmphdr->icmp6_id, icmphdr->icmp6_seq, > > - iphdr->ip6_plen); > > + ntohs(icmphdr->icmp6_id), > > + ntohs(icmphdr->icmp6_seq), > > + ntohs(iphdr->ip6_plen)); > > break; > > case ICMP6_ECHO_REPLY: > > printf(" echo reply, id: %d, seq: %d, length: %hu\n", > > - icmphdr->icmp6_id, icmphdr->icmp6_seq, > > - iphdr->ip6_plen); > > + ntohs(icmphdr->icmp6_id), > > + ntohs(icmphdr->icmp6_seq), > > + ntohs(iphdr->ip6_plen)); > > break; > > case ICMP6_TIME_EXCEEDED: > > printf(" time exceeded in-transit, length %zu\n", > >
diff --git a/tcpdump.c b/tcpdump.c index db93681..b9edc20 100644 --- a/tcpdump.c +++ b/tcpdump.c @@ -589,13 +589,15 @@ static void dump_ipv6(unsigned char *packet_buff, ssize_t buff_len, break; case ICMP6_ECHO_REQUEST: printf(" echo request, id: %d, seq: %d, length: %hu\n", - icmphdr->icmp6_id, icmphdr->icmp6_seq, - iphdr->ip6_plen); + ntohs(icmphdr->icmp6_id), + ntohs(icmphdr->icmp6_seq), + ntohs(iphdr->ip6_plen)); break; case ICMP6_ECHO_REPLY: printf(" echo reply, id: %d, seq: %d, length: %hu\n", - icmphdr->icmp6_id, icmphdr->icmp6_seq, - iphdr->ip6_plen); + ntohs(icmphdr->icmp6_id), + ntohs(icmphdr->icmp6_seq), + ntohs(iphdr->ip6_plen)); break; case ICMP6_TIME_EXCEEDED: printf(" time exceeded in-transit, length %zu\n",
The ICMPv6 Echo Request/Reply sequence number and id as well as the IPv6 header length are two byte long fields and therefore might need a conversion on a little endian system. Otherwise the output will be broken on such a machine. Fixes: 35b37756f4a3 ("add IPv6 support to tcpdump parser") Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue> --- tcpdump.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)