Re: tcpdump: Ethernet header is not dumped with -xX if IP header is unaligned
Still looking for OK's.
On Wed, Nov 26, 2014 at 17:18 +0100, Mike Belopuhov wrote:
better diff. the problem is that dissectors use packetp and
snapend pointers themselves therefore they should be pointing
to the newly allocated structure. we can restore them once
we're done with the inner content and go back to the caller
to see if we need to hexdump the contents.
i'll see if i can cook and test the ipv6 version.
OK?
now with an ip6 version and i've made sure that this fixes
dumping unaligned ipv6 packets as well. in the meantime
jsg@ has lured me into looking at the afl crash in the same
code and it looks like the check from ip6_print is useful
here: if we haven't got enough data for a header, don't
bother with anything else and just bail.
ok?
diff --git usr.sbin/tcpdump/print-ip.c usr.sbin/tcpdump/print-ip.c
index 3f4194c..e9d2185 100644
--- usr.sbin/tcpdump/print-ip.c
+++ usr.sbin/tcpdump/print-ip.c
@@ -351,22 +351,27 @@ in_cksum(const u_short *addr, register int len, int
csum)
* print an IP datagram.
*/
void
ip_print(register const u_char *bp, register u_int length)
{
+ static u_char *abuf = NULL;
register const struct ip *ip;
register u_int hlen, len, off;
register const u_char *cp;
+ const u_char *pktp = packetp;
+ const u_char *send = snapend;
ip = (const struct ip *)bp;
+ if ((u_char *)(ip + 1) snapend) {
+ printf([|ip]);
+ return;
+ }
+
/*
* If the IP header is not aligned, copy into abuf.
- * This will never happen with BPF. It does happen with raw packet
- * dumps from -r.
*/
if ((intptr_t)ip (sizeof(long)-1)) {
- static u_char *abuf = NULL;
static int didwarn = 0;
int clen = snapend - bp;
if (clen snaplen)
clen = snaplen;
@@ -387,11 +392,11 @@ ip_print(register const u_char *bp, register u_int
length)
}
TCHECK(*ip);
if (ip-ip_v != IPVERSION) {
(void)printf(bad-ip-version %u, ip-ip_v);
- return;
+ goto out;
}
len = ntohs(ip-ip_len);
if (length len) {
(void)printf(truncated-ip - %d bytes missing!,
@@ -400,11 +405,11 @@ ip_print(register const u_char *bp, register u_int
length)
}
hlen = ip-ip_hl * 4;
if (hlen sizeof(struct ip) || hlen len) {
(void)printf(bad-hlen %d, hlen);
- return;
+ goto out;
}
len -= hlen;
/*
@@ -465,11 +470,11 @@ ip_print(register const u_char *bp, register u_int
length)
ipaddr_string(ip-ip_src),
ipaddr_string(ip-ip_dst));
ip_print(cp, len);
if (! vflag) {
printf( (encap));
- return;
+ goto out;
}
break;
#ifdef INET6
#ifndef IPPROTO_IPV6
@@ -482,11 +487,11 @@ ip_print(register const u_char *bp, register u_int
length)
ipaddr_string(ip-ip_src),
ipaddr_string(ip-ip_dst));
ip6_print(cp, len);
if (! vflag) {
printf( (encap));
- return;
+ goto out;
}
break;
#endif /*INET6*/
#ifndef IPPROTO_GRE
@@ -499,11 +504,11 @@ ip_print(register const u_char *bp, register u_int
length)
ipaddr_string(ip-ip_dst));
/* do it */
gre_print(cp, len);
if (! vflag) {
printf( (gre encap));
- return;
+ goto out;
}
break;
#ifndef IPPROTO_ESP
#define IPPROTO_ESP 50
@@ -528,11 +533,11 @@ ip_print(register const u_char *bp, register u_int
length)
ipaddr_string(ip-ip_src),
ipaddr_string(ip-ip_dst));
mobile_print(cp, len);
if (! vflag) {
printf( (mobile encap));
- return;
+ goto out;
}
break;
#ifndef IPPROTO_ETHERIP
#define IPPROTO_ETHERIP 97
@@ -653,10 +658,13 @@ ip_print(register const u_char *bp, register u_int
length)
(void)printf(%soptlen=%d, sep, hlen);
ip_optprint((u_char *)(ip + 1), hlen);
Re: tcpdump: Ethernet header is not dumped with -xX if IP header is unaligned
On 27 November 2014 at 03:12, Theo de Raadt dera...@cvs.openbsd.org wrote: On Tue, Nov 25, 2014 at 18:42 +0100, Mike Belopuhov wrote: On Mon, Nov 24, 2014 at 19:04 +0100, Mike Belopuhov wrote: Hi, IP header is not always aligned since bpf copies out the mbuf chain into the contigous buffer provided by the userland. I've seen this with large packet sizes on VLANs. ip_print will then copy the packet but the Ethernet header into the internal buffer so that it can cast it to the IP header structure and update global packetp and snapend pointers hence preventing the -Xx dumping code from printing out the Ethernet header itself. Diff below fixes it. OK? better diff. the problem is that dissectors use packetp and snapend pointers themselves therefore they should be pointing to the newly allocated structure. we can restore them once we're done with the inner content and go back to the caller to see if we need to hexdump the contents. i'll see if i can cook and test the ipv6 version. OK? now with an ip6 version and i've made sure that this fixes dumping unaligned ipv6 packets as well. in the meantime jsg@ has lured me into looking at the afl crash in the same code and it looks like the check from ip6_print is useful here: if we haven't got enough data for a header, don't bother with anything else and just bail. ok? Did you test on a strict alignment machine? works fine on sparc64.
Re: tcpdump: Ethernet header is not dumped with -xX if IP header is unaligned
On Tue, Nov 25, 2014 at 18:42 +0100, Mike Belopuhov wrote:
On Mon, Nov 24, 2014 at 19:04 +0100, Mike Belopuhov wrote:
Hi,
IP header is not always aligned since bpf copies out the mbuf
chain into the contigous buffer provided by the userland. I've
seen this with large packet sizes on VLANs. ip_print will then
copy the packet but the Ethernet header into the internal buffer
so that it can cast it to the IP header structure and update
global packetp and snapend pointers hence preventing the -Xx
dumping code from printing out the Ethernet header itself.
Diff below fixes it. OK?
better diff. the problem is that dissectors use packetp and
snapend pointers themselves therefore they should be pointing
to the newly allocated structure. we can restore them once
we're done with the inner content and go back to the caller
to see if we need to hexdump the contents.
i'll see if i can cook and test the ipv6 version.
OK?
now with an ip6 version and i've made sure that this fixes
dumping unaligned ipv6 packets as well. in the meantime
jsg@ has lured me into looking at the afl crash in the same
code and it looks like the check from ip6_print is useful
here: if we haven't got enough data for a header, don't
bother with anything else and just bail.
ok?
diff --git usr.sbin/tcpdump/print-ip.c usr.sbin/tcpdump/print-ip.c
index 3f4194c..e9d2185 100644
--- usr.sbin/tcpdump/print-ip.c
+++ usr.sbin/tcpdump/print-ip.c
@@ -351,22 +351,27 @@ in_cksum(const u_short *addr, register int len, int csum)
* print an IP datagram.
*/
void
ip_print(register const u_char *bp, register u_int length)
{
+ static u_char *abuf = NULL;
register const struct ip *ip;
register u_int hlen, len, off;
register const u_char *cp;
+ const u_char *pktp = packetp;
+ const u_char *send = snapend;
ip = (const struct ip *)bp;
+ if ((u_char *)(ip + 1) snapend) {
+ printf([|ip]);
+ return;
+ }
+
/*
* If the IP header is not aligned, copy into abuf.
-* This will never happen with BPF. It does happen with raw packet
-* dumps from -r.
*/
if ((intptr_t)ip (sizeof(long)-1)) {
- static u_char *abuf = NULL;
static int didwarn = 0;
int clen = snapend - bp;
if (clen snaplen)
clen = snaplen;
@@ -387,11 +392,11 @@ ip_print(register const u_char *bp, register u_int length)
}
TCHECK(*ip);
if (ip-ip_v != IPVERSION) {
(void)printf(bad-ip-version %u, ip-ip_v);
- return;
+ goto out;
}
len = ntohs(ip-ip_len);
if (length len) {
(void)printf(truncated-ip - %d bytes missing!,
@@ -400,11 +405,11 @@ ip_print(register const u_char *bp, register u_int length)
}
hlen = ip-ip_hl * 4;
if (hlen sizeof(struct ip) || hlen len) {
(void)printf(bad-hlen %d, hlen);
- return;
+ goto out;
}
len -= hlen;
/*
@@ -465,11 +470,11 @@ ip_print(register const u_char *bp, register u_int length)
ipaddr_string(ip-ip_src),
ipaddr_string(ip-ip_dst));
ip_print(cp, len);
if (! vflag) {
printf( (encap));
- return;
+ goto out;
}
break;
#ifdef INET6
#ifndef IPPROTO_IPV6
@@ -482,11 +487,11 @@ ip_print(register const u_char *bp, register u_int length)
ipaddr_string(ip-ip_src),
ipaddr_string(ip-ip_dst));
ip6_print(cp, len);
if (! vflag) {
printf( (encap));
- return;
+ goto out;
}
break;
#endif /*INET6*/
#ifndef IPPROTO_GRE
@@ -499,11 +504,11 @@ ip_print(register const u_char *bp, register u_int length)
ipaddr_string(ip-ip_dst));
/* do it */
gre_print(cp, len);
if (! vflag) {
printf( (gre encap));
- return;
+ goto out;
}
break;
#ifndef IPPROTO_ESP
#define IPPROTO_ESP 50
@@ -528,11 +533,11 @@ ip_print(register const u_char *bp, register u_int length)
ipaddr_string(ip-ip_src),
ipaddr_string(ip-ip_dst));
tcpdump: Ethernet header is not dumped with -xX if IP header is unaligned
Hi,
IP header is not always aligned since bpf copies out the mbuf
chain into the contigous buffer provided by the userland. I've
seen this with large packet sizes on VLANs. ip_print will then
copy the packet but the Ethernet header into the internal buffer
so that it can cast it to the IP header structure and update
global packetp and snapend pointers hence preventing the -Xx
dumping code from printing out the Ethernet header itself.
Diff below fixes it. OK?
diff --git usr.sbin/tcpdump/print-ip.c usr.sbin/tcpdump/print-ip.c
index 3f4194c..6d90b3d 100644
--- usr.sbin/tcpdump/print-ip.c
+++ usr.sbin/tcpdump/print-ip.c
@@ -252,7 +252,7 @@ ip_printroute(const char *type, register const u_char *cp,
u_int length)
* print IP options.
*/
static void
-ip_optprint(register const u_char *cp, u_int length)
+ip_optprint(register const u_char *cp, u_char *send, u_int length)
{
register u_int len;
int tt;
@@ -265,7 +265,7 @@ ip_optprint(register const u_char *cp, u_int length)
printf([|ip op len %d], len);
return;
}
- if (cp[1] = snapend || cp + len snapend) {
+ if (cp[1] = send || cp + len send) {
printf([|ip]);
return;
}
@@ -356,12 +356,11 @@ ip_print(register const u_char *bp, register u_int length)
register const struct ip *ip;
register u_int hlen, len, off;
register const u_char *cp;
+ u_char *send = snapend;
ip = (const struct ip *)bp;
/*
* If the IP header is not aligned, copy into abuf.
-* This will never happen with BPF. It does happen with raw packet
-* dumps from -r.
*/
if ((intptr_t)ip (sizeof(long)-1)) {
static u_char *abuf = NULL;
@@ -376,8 +375,7 @@ ip_print(register const u_char *bp, register u_int length)
error(ip_print: malloc);
}
memmove((char *)abuf, (char *)ip, min(length, clen));
- snapend = abuf + clen;
- packetp = abuf;
+ send = abuf + clen;
ip = (struct ip *)abuf;
/* We really want libpcap to give us aligned packets */
if (!didwarn) {
@@ -538,7 +536,7 @@ ip_print(register const u_char *bp, register u_int length)
#define IPPROTO_ETHERIP97
#endif
case IPPROTO_ETHERIP:
- etherip_print(cp, snapend - cp, len,
+ etherip_print(cp, send - cp, len,
(const u_char *)ip);
break;
@@ -573,7 +571,7 @@ ip_print(register const u_char *bp, register u_int length)
#endif
case IPPROTO_PFSYNC:
pfsync_ip_print(cp,
- (int)(snapend - (u_char *)ip) - hlen,
+ (int)(send - (u_char *)ip) - hlen,
(const u_char *)ip);
break;
@@ -638,7 +636,7 @@ ip_print(register const u_char *bp, register u_int length)
}
(void)printf(%slen %u, sep, ntohs(ip-ip_len));
sep = , ;
- if ((u_char *)ip + hlen = snapend) {
+ if ((u_char *)ip + hlen = send) {
u_int16_t sum, ip_sum;
sum = in_cksum((const u_short *)ip, hlen, 0);
if (sum != 0) {
@@ -651,7 +649,7 @@ ip_print(register const u_char *bp, register u_int length)
if (hlen sizeof(struct ip)) {
hlen -= sizeof(struct ip);
(void)printf(%soptlen=%d, sep, hlen);
- ip_optprint((u_char *)(ip + 1), hlen);
+ ip_optprint((u_char *)(ip + 1), send, hlen);
}
printf());
}
Re: tcpdump: Ethernet header is not dumped with -xX if IP header is unaligned
On Nov 24, 2014 7:10 PM, Mike Belopuhov m...@belopuhov.com wrote: Hi, IP header is not always aligned since bpf copies out the mbuf chain into the contigous buffer provided by the userland. I've seen this with large packet sizes on VLANs. ip_print will then copy the packet but the Ethernet header into the internal buffer so that it can cast it to the IP header structure and update global packetp and snapend pointers hence preventing the -Xx dumping code from printing out the Ethernet header itself. Diff below fixes it. OK? Err, I think I've sent the wrong diff. Please ignore it. I'll resend it tomorrow.
