diff mbox series

batctl: tcpdump: Fix endianness in ICMPv6 Echo Request/Reply parsing

Message ID 20200913213019.4250-1-linus.luessing@c0d3.blue
State Accepted, archived
Delegated to: Simon Wunderlich
Headers
Series batctl: tcpdump: Fix endianness in ICMPv6 Echo Request/Reply parsing |

Commit Message

Linus Lüssing Sept. 13, 2020, 9:30 p.m. UTC 
  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(-)

Comments

txt.file@txtfile.eu Sept. 13, 2020, 10:07 p.m. UTC | #1 
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",
>
Daniel Golle Sept. 13, 2020, 10:22 p.m. UTC | #2 
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 mbox series

Patch

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",