Kernel 2.6.13 breaks libpcap (and tcpdump).

Kernel 2.6.13 breaks libpcap (and tcpdump).

[John McGowan]

Kernel 2.6.13. Breaks libpcap.

Fedora Core 2, gcc 3.3.3, Pentium III (933MHz)

I had written about my dismay that traceproto and tcptraceroute
no longer worked and suspected that libnet was broken.

It seems that it is libpcap that is broken by kernel 2.6.13 and
tcpdump itself no longer works.
Well, it works ... but not correctly.

 Capture data, then look for ICMP messages
 (e.g. Time Exceeded errors as in a traceroute)
 by filtering the file.
 
  tcpdump -w 1.cap
  tcpdump -f "ip proto \icmp" -r 1.cap

That works.


 Filter incoming data, looking for ICMP messages:
 
  tcpdump -f "ip proto \icmp"
 
Well, that catches nothing.


I tried recompiling (source RPM, Fedora Core 2) tcpdump
(libpcap, tcpdump, etc.) and reinstalling. That did not
fix the problem with tcpdump.

It also broke a tethereal script I was using (which I changed
to capture all packets, which works as indicated above, and
then used a '-R', read, filter to display the one's I want).

Re: Kernel 2.6.13 breaks libpcap (and tcpdump).

[Andrew Morton]

John McGowan <jmcgowan@inch.com> wrote:
>
> Kernel 2.6.13. Breaks libpcap.
> 
> Fedora Core 2, gcc 3.3.3, Pentium III (933MHz)
> 
> I had written about my dismay that traceproto and tcptraceroute
> no longer worked and suspected that libnet was broken.
> 
> It seems that it is libpcap that is broken by kernel 2.6.13 and
> tcpdump itself no longer works.
> Well, it works ... but not correctly.
> 
>  Capture data, then look for ICMP messages
>  (e.g. Time Exceeded errors as in a traceroute)
>  by filtering the file.
>  
>   tcpdump -w 1.cap
>   tcpdump -f "ip proto \icmp" -r 1.cap
> 
> That works.
> 
> 
>  Filter incoming data, looking for ICMP messages:
>  
>   tcpdump -f "ip proto \icmp"
>  
> Well, that catches nothing.
> 
> 
> I tried recompiling (source RPM, Fedora Core 2) tcpdump
> (libpcap, tcpdump, etc.) and reinstalling. That did not
> fix the problem with tcpdump.
> 
> It also broke a tethereal script I was using (which I changed
> to capture all packets, which works as indicated above, and
> then used a '-R', read, filter to display the one's I want).
> 

(cc netdev)

Re: Kernel 2.6.13 breaks libpcap (and tcpdump).

[Herbert Xu]

Andrew Morton <akpm@osdl.org> wrote:
> 
>>  Filter incoming data, looking for ICMP messages:
>>  
>>   tcpdump -f "ip proto \icmp"
>>  
>> Well, that catches nothing.

We aren't handling the reading of specific fields like the IP protocol
field correctly.  This patch should make it work again.

I tried to move this logic into the new load_pointer function but it
all came out messy so I simply rolled it back.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/core/filter.c b/net/core/filter.c
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -36,7 +36,7 @@
 #include <linux/filter.h>
 
 /* No hurry in this branch */
-static void *__load_pointer(struct sk_buff *skb, int k)
+static void *load_pointer(struct sk_buff *skb, int k)
 {
 	u8 *ptr = NULL;
 
@@ -50,18 +50,6 @@ static void *__load_pointer(struct sk_bu
 	return NULL;
 }
 
-static inline void *load_pointer(struct sk_buff *skb, int k,
-                                 unsigned int size, void *buffer)
-{
-	if (k >= 0)
-		return skb_header_pointer(skb, k, size, buffer);
-	else {
-		if (k >= SKF_AD_OFF)
-			return NULL;
-		return __load_pointer(skb, k);
-	}
-}
-
 /**
  *	sk_run_filter	- 	run a filter on a socket
  *	@skb: buffer to run the filter on
@@ -177,7 +165,13 @@ int sk_run_filter(struct sk_buff *skb, s
 		case BPF_LD|BPF_W|BPF_ABS:
 			k = fentry->k;
  load_w:
-			ptr = load_pointer(skb, k, 4, &tmp);
+			if (k >= 0)
+				ptr = skb_header_pointer(skb, k, 4, &tmp);
+			else if (k < SKF_AD_OFF)
+				ptr = load_pointer(skb, k);
+			else
+				break;
+
 			if (ptr != NULL) {
 				A = ntohl(*(u32 *)ptr);
 				continue;
@@ -186,7 +180,13 @@ int sk_run_filter(struct sk_buff *skb, s
 		case BPF_LD|BPF_H|BPF_ABS:
 			k = fentry->k;
  load_h:
-			ptr = load_pointer(skb, k, 2, &tmp);
+			if (k >= 0)
+				ptr = skb_header_pointer(skb, k, 2, &tmp);
+			else if (k < SKF_AD_OFF)
+				ptr = load_pointer(skb, k);
+			else
+				break;
+
 			if (ptr != NULL) {
 				A = ntohs(*(u16 *)ptr);
 				continue;
@@ -195,7 +195,13 @@ int sk_run_filter(struct sk_buff *skb, s
 		case BPF_LD|BPF_B|BPF_ABS:
 			k = fentry->k;
 load_b:
-			ptr = load_pointer(skb, k, 1, &tmp);
+			if (k >= 0)
+				ptr = skb_header_pointer(skb, k, 1, &tmp);
+			else if (k < SKF_AD_OFF)
+				ptr = load_pointer(skb, k);
+			else
+				break;
+
 			if (ptr != NULL) {
 				A = *(u8 *)ptr;
 				continue;
@@ -217,7 +223,9 @@ load_b:
 			k = X + fentry->k;
 			goto load_b;
 		case BPF_LDX|BPF_B|BPF_MSH:
-			ptr = load_pointer(skb, fentry->k, 1, &tmp);
+			if (fentry->k < 0)
+				return 0;
+			ptr = skb_header_pointer(skb, fentry->k, 1, &tmp);
 			if (ptr != NULL) {
 				X = (*(u8 *)ptr & 0xf) << 2;
 				continue;

Re: Kernel 2.6.13 breaks libpcap (and tcpdump).

[Patrick McHardy]

Herbert Xu wrote:
> We aren't handling the reading of specific fields like the IP protocol
> field correctly.  This patch should make it work again.

I can't spot the problem, could you give me a hint?

> I tried to move this logic into the new load_pointer function but it
> all came out messy so I simply rolled it back.

  		case BPF_LD|BPF_W|BPF_ABS:
  			k = fentry->k;
   load_w:
-			ptr = load_pointer(skb, k, 4, &tmp);
+			if (k >= 0)
+				ptr = skb_header_pointer(skb, k, 4, &tmp);
+			else if (k < SKF_AD_OFF)
+				ptr = load_pointer(skb, k);
+			else
+				break;

The old value of ptr will be used in this case, it should
be explicitly set to NULL to abort.

Re: Kernel 2.6.13 breaks libpcap (and tcpdump).

[Patrick McHardy]

Patrick McHardy wrote:
> Herbert Xu wrote:
> 
>> We aren't handling the reading of specific fields like the IP protocol
>> field correctly.  This patch should make it work again.
> 
> 
> I can't spot the problem, could you give me a hint?

Never mind, I got it, we never fall through to the second switch
statement anymore. I think we could simply break when load_pointer
returns NULL. The switch statement will fall through to the default
case and return 0 for all cases but 0 > k >= SKF_AD_OFF.

Re: Kernel 2.6.13 breaks libpcap (and tcpdump).

[Herbert Xu]

Patrick McHardy <kaber@trash.net> wrote:
> 
> Never mind, I got it, we never fall through to the second switch
> statement anymore. I think we could simply break when load_pointer
> returns NULL. The switch statement will fall through to the default
> case and return 0 for all cases but 0 > k >= SKF_AD_OFF.

Thanks Patrick, that's a much better idea.  Here's a patch to do just
that.

I left BPF_MSH alone because it's really a hack to calculate the IP
header length, which makes no sense when applied to the special data.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

BTW, you should be able to send me mail now.  Sorry about that.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/core/filter.c b/net/core/filter.c
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -182,7 +182,7 @@ int sk_run_filter(struct sk_buff *skb, s
 				A = ntohl(*(u32 *)ptr);
 				continue;
 			}
-			return 0;
+			break;
 		case BPF_LD|BPF_H|BPF_ABS:
 			k = fentry->k;
  load_h:
@@ -191,7 +191,7 @@ int sk_run_filter(struct sk_buff *skb, s
 				A = ntohs(*(u16 *)ptr);
 				continue;
 			}
-			return 0;
+			break;
 		case BPF_LD|BPF_B|BPF_ABS:
 			k = fentry->k;
 load_b:
@@ -200,7 +200,7 @@ load_b:
 				A = *(u8 *)ptr;
 				continue;
 			}
-			return 0;
+			break;
 		case BPF_LD|BPF_W|BPF_LEN:
 			A = skb->len;
 			continue;