[v3,maint] batctl: tcpdump - fix IP header parsing

Message ID	1396420055-19598-1-git-send-email-antonio@meshcoding.com (mailing list archive)
State	Accepted, archived
Headers	Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=178.209.62.157; helo=s3.neomailbox.net; envelope-from=antonio@meshcoding.com; receiver=b.a.t.m.a.n@lists.open-mesh.org From: Antonio Quartulli <antonio@meshcoding.com> To: b.a.t.m.a.n@lists.open-mesh.org Date: Wed, 2 Apr 2014 08:27:35 +0200 Message-Id: <1396420055-19598-1-git-send-email-antonio@meshcoding.com> Cc: Antonio Quartulli <antonio@open-mesh.com> Subject: [B.A.T.M.A.N.] [PATCHv3 maint] batctl: tcpdump - fix IP header parsing Precedence: list Reply-To: The list for a Better Approach To Mobile Ad-hoc Networking <b.a.t.m.a.n@lists.open-mesh.org>

Message ID

1396420055-19598-1-git-send-email-antonio@meshcoding.com (mailing list archive)

State

Accepted, archived

Headers

Received-SPF: Pass (sender SPF authorized) identity=mailfrom;
	client-ip=178.209.62.157; helo=s3.neomailbox.net;
	envelope-from=antonio@meshcoding.com;
	receiver=b.a.t.m.a.n@lists.open-mesh.org 
From: Antonio Quartulli <antonio@meshcoding.com>
To: b.a.t.m.a.n@lists.open-mesh.org
Date: Wed,  2 Apr 2014 08:27:35 +0200
Message-Id: <1396420055-19598-1-git-send-email-antonio@meshcoding.com>
Cc: Antonio Quartulli <antonio@open-mesh.com>
Subject: [B.A.T.M.A.N.] [PATCHv3 maint] batctl: tcpdump - fix IP header
	parsing
Precedence: list
Reply-To: The list for a Better Approach To Mobile Ad-hoc Networking
	<b.a.t.m.a.n@lists.open-mesh.org>

Commit Message

Antonio Quartulli April 2, 2014, 6:27 a.m. UTC

  From: Antonio Quartulli <antonio@open-mesh.com>

Fix the IP header parsing by properly converting the source
and the destination addresses to strings.
The problem is given by the fact that inet_ntoa() uses a
static buffer to store its result, therefore it cannot be
invoked twice in a row without saving the first outcome.
Moreover inet_ntoa() is deprecated in favor of inet_pton().

Introduced by 35b37756f4a3a230bf7941034d9da178ee5b4948
("add IPv6 support to tcpdump parser")

This patch also slightly beautifies the code.

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---

Change since v2:
- fix commit subject

Change since v1:
- print all errors on stderr


 tcpdump.c | 73 ++++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 42 insertions(+), 31 deletions(-)

Comments

Marek Lindner April 6, 2014, 1:56 p.m. UTC | #1

On Wednesday 02 April 2014 08:27:35 Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> Fix the IP header parsing by properly converting the source
> and the destination addresses to strings.
> The problem is given by the fact that inet_ntoa() uses a
> static buffer to store its result, therefore it cannot be
> invoked twice in a row without saving the first outcome.
> Moreover inet_ntoa() is deprecated in favor of inet_pton().
> 
> Introduced by 35b37756f4a3a230bf7941034d9da178ee5b4948
> ("add IPv6 support to tcpdump parser")
> 
> This patch also slightly beautifies the code.
> 
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> ---
> 
> Change since v2:
> - fix commit subject
> 
> Change since v1:
> - print all errors on stderr
> 
> 
>  tcpdump.c | 73
> ++++++++++++++++++++++++++++++++++++--------------------------- 1 file
> changed, 42 insertions(+), 31 deletions(-)

Applied in revision f08a28d.

Thanks,
Marek

diff mbox

Patch

diff --git a/tcpdump.c b/tcpdump.c
index e9fac70..94e2a84 100644
--- a/tcpdump.c
+++ b/tcpdump.c
@@ -258,20 +258,27 @@  static void dump_ipv6(unsigned char *packet_buff, ssize_t buff_len,
 	struct ip6_hdr *iphdr;
 	struct icmp6_hdr *icmphdr;
 
-	char ipv6_addr_src[40], ipv6_addr_dst[40];
-	char ip_saddr[40], ip_daddr[40], nd_nas_target[40];
+	char ipsrc[INET6_ADDRSTRLEN], ipdst[INET6_ADDRSTRLEN];
 	struct nd_neighbor_solicit *nd_neigh_sol;
 	struct nd_neighbor_advert *nd_advert;
+	char nd_nas_target[INET6_ADDRSTRLEN];
 	const char ip_string[] = "IP6";
 
 	iphdr = (struct ip6_hdr *)packet_buff;
-	LEN_CHECK((size_t)buff_len, (size_t)(sizeof(struct ip6_hdr)), "IP6");
+	LEN_CHECK((size_t)buff_len, (size_t)(sizeof(struct ip6_hdr)), ip_string);
 
 	if (!time_printed)
 		print_time();
 
-	inet_ntop(AF_INET6, &(iphdr->ip6_src), ipv6_addr_src, 40);
-	inet_ntop(AF_INET6, &(iphdr->ip6_dst), ipv6_addr_dst, 40);
+	if (!inet_ntop(AF_INET6, &iphdr->ip6_src, ipsrc, sizeof(ipsrc))) {
+		fprintf(stderr, "Cannot decode source IPv6\n");
+		return;
+	}
+
+	if (!inet_ntop(AF_INET6, &iphdr->ip6_dst, ipdst, sizeof(ipdst))) {
+		fprintf(stderr, "Cannot decode destination IPv6\n");
+		return;
+	}
 
 	switch (iphdr->ip6_nxt) {
 	case IPPROTO_ICMPV6:
@@ -280,7 +287,7 @@  static void dump_ipv6(unsigned char *packet_buff, ssize_t buff_len,
 		icmphdr = (struct icmp6_hdr *)(packet_buff +
 					       sizeof(struct ip6_hdr));
 
-		printf("IP6 %s > %s ", ipv6_addr_src, ipv6_addr_dst);
+		printf("%s %s > %s ", ip_string, ipsrc, ipdst);
 		if (icmphdr->icmp6_type < ICMP6_INFOMSG_MASK &&
 		    (size_t)(buff_len) > IPV6_MIN_MTU) {
 			fprintf(stderr,
@@ -348,16 +355,12 @@  static void dump_ipv6(unsigned char *packet_buff, ssize_t buff_len,
 		}
 		break;
 	case IPPROTO_TCP:
-		inet_ntop(AF_INET6, &(iphdr->ip6_src), ip_saddr, 40);
-		inet_ntop(AF_INET6, &(iphdr->ip6_dst), ip_daddr, 40);
 		dump_tcp(ip_string, packet_buff, buff_len,
-			 sizeof(struct ip6_hdr), ip_saddr, ip_daddr);
+			 sizeof(struct ip6_hdr), ipsrc, ipdst);
 		break;
 	case IPPROTO_UDP:
-		inet_ntop(AF_INET6, &(iphdr->ip6_src), ip_saddr, 40);
-		inet_ntop(AF_INET6, &(iphdr->ip6_dst), ip_daddr, 40);
 		dump_udp(ip_string, packet_buff, buff_len,
-			 sizeof(struct ip6_hdr), ip_saddr, ip_daddr);
+			 sizeof(struct ip6_hdr), ipsrc, ipdst);
 		break;
 	default:
 		printf(" IPv6 unknown protocol: %i\n", iphdr->ip6_nxt);
@@ -367,29 +370,40 @@  static void dump_ipv6(unsigned char *packet_buff, ssize_t buff_len,
 static void dump_ip(unsigned char *packet_buff, ssize_t buff_len,
 		    int time_printed)
 {
+	char ipsrc[INET_ADDRSTRLEN], ipdst[INET_ADDRSTRLEN];
 	struct iphdr *iphdr, *tmp_iphdr;
+	const char ip_string[] = "IP";
 	struct udphdr *tmp_udphdr;
 	struct icmphdr *icmphdr;
-	const char ip_string[] = "IP";
 
 	iphdr = (struct iphdr *)packet_buff;
-	LEN_CHECK((size_t)buff_len, (size_t)(iphdr->ihl * 4), "IP");
+	LEN_CHECK((size_t)buff_len, (size_t)(iphdr->ihl * 4), ip_string);
 
 	if (!time_printed)
 		print_time();
 
+	if (!inet_ntop(AF_INET, &iphdr->saddr, ipsrc, sizeof(ipsrc))) {
+		fprintf(stderr, "Cannot decode source IP\n");
+		return;
+	}
+
+	if (!inet_ntop(AF_INET, &iphdr->daddr, ipdst, sizeof(ipdst))) {
+		fprintf(stderr, "Cannot decode destination IP\n");
+		return;
+	}
+
 	switch (iphdr->protocol) {
 	case IPPROTO_ICMP:
 		LEN_CHECK((size_t)buff_len - (iphdr->ihl * 4), sizeof(struct icmphdr), "ICMP");
 
 		icmphdr = (struct icmphdr *)(packet_buff + (iphdr->ihl * 4));
-		printf("IP %s > ", inet_ntoa(*(struct in_addr *)&iphdr->saddr));
+		printf("%s %s > ", ip_string, ipsrc);
 
 		switch (icmphdr->type) {
 		case ICMP_ECHOREPLY:
 			printf("%s: ICMP echo reply, id %hu, seq %hu, length %zu\n",
-				inet_ntoa(*(struct in_addr *)&iphdr->daddr),
-				ntohs(icmphdr->un.echo.id), ntohs(icmphdr->un.echo.sequence),
+				ipdst, ntohs(icmphdr->un.echo.id),
+				ntohs(icmphdr->un.echo.sequence),
 				(size_t)buff_len - (iphdr->ihl * 4));
 			break;
 		case ICMP_DEST_UNREACH:
@@ -401,46 +415,43 @@  static void dump_ip(unsigned char *packet_buff, ssize_t buff_len,
 				tmp_iphdr = (struct iphdr *)(((char *)icmphdr) + sizeof(struct icmphdr));
 				tmp_udphdr = (struct udphdr *)(((char *)tmp_iphdr) + (tmp_iphdr->ihl * 4));
 
-				printf("%s: ICMP ", inet_ntoa(*(struct in_addr *)&iphdr->daddr));
+				printf("%s: ICMP ", ipdst);
 				printf("%s udp port %hu unreachable, length %zu\n",
-					inet_ntoa(*(struct in_addr *)&tmp_iphdr->daddr),
-					ntohs(tmp_udphdr->dest), (size_t)buff_len - (iphdr->ihl * 4));
+					ipdst, ntohs(tmp_udphdr->dest),
+					(size_t)buff_len - (iphdr->ihl * 4));
 				break;
 			default:
 				printf("%s: ICMP unreachable %hhu, length %zu\n",
-					inet_ntoa(*(struct in_addr *)&iphdr->daddr),
-					icmphdr->code, (size_t)buff_len - (iphdr->ihl * 4));
+					ipdst, icmphdr->code,
+					(size_t)buff_len - (iphdr->ihl * 4));
 				break;
 			}
 
 			break;
 		case ICMP_ECHO:
 			printf("%s: ICMP echo request, id %hu, seq %hu, length %zu\n",
-				inet_ntoa(*(struct in_addr *)&iphdr->daddr),
-				ntohs(icmphdr->un.echo.id), ntohs(icmphdr->un.echo.sequence),
+				ipdst, ntohs(icmphdr->un.echo.id),
+				ntohs(icmphdr->un.echo.sequence),
 				(size_t)buff_len - (iphdr->ihl * 4));
 			break;
 		case ICMP_TIME_EXCEEDED:
 			printf("%s: ICMP time exceeded in-transit, length %zu\n",
-				inet_ntoa(*(struct in_addr *)&iphdr->daddr),
-				(size_t)buff_len - (iphdr->ihl * 4));
+				ipdst, (size_t)buff_len - (iphdr->ihl * 4));
 			break;
 		default:
 			printf("%s: ICMP type %hhu, length %zu\n",
-				inet_ntoa(*(struct in_addr *)&iphdr->daddr), icmphdr->type,
+				ipdst, icmphdr->type,
 				(size_t)buff_len - (iphdr->ihl * 4));
 			break;
 		}
 		break;
 	case IPPROTO_TCP:
 		dump_tcp(ip_string, packet_buff, buff_len, iphdr->ihl * 4,
-			 inet_ntoa(*(struct in_addr *)&iphdr->saddr),
-			 inet_ntoa(*(struct in_addr *)&iphdr->daddr));
+			 ipsrc, ipdst);
 		break;
 	case IPPROTO_UDP:
 		dump_udp(ip_string, packet_buff, buff_len, iphdr->ihl * 4,
-			 inet_ntoa(*(struct in_addr *)&iphdr->saddr),
-			 inet_ntoa(*(struct in_addr *)&iphdr->daddr));
+			 ipsrc, ipdst);
 		break;
 	default:
 		printf("IP unknown protocol: %i\n", iphdr->protocol);