[PATCH]: fixed kernel panic when trying to find tcp option

[PATCH]: fixed kernel panic when trying to find tcp option

[Yasuyuki Kozakai]

[-- Attachment #1: Type: Text/Plain, Size: 295 bytes --]

Hi,

This patch fixed panic on 2.6.9-rc2 when trying to find tcp option
in the packet which has no option.

If not fixed yet, please apply this patch.

Regards,

-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>


[-- Attachment #2: tcp_find_option.patch --]
[-- Type: Text/Plain, Size: 512 bytes --]

--- linux-2.6.9-rc2/net/ipv4/netfilter/ip_tables.c	2004-09-15 14:04:16.000000000 +0900
+++ linux-2.6.9-rc2-nolinearize/net/ipv4/netfilter/ip_tables.c	2004-09-15 20:01:42.970226904 +0900
@@ -1464,8 +1464,9 @@ tcp_find_option(u_int8_t option,
 	unsigned int i;
 
 	duprintf("tcp_match: finding option\n");
+	if (!optlen)
+		return invert;
 	/* If we don't have the whole header, drop packet. */
-	BUG_ON(!optlen);
 	op = skb_header_pointer(skb,
 				skb->nh.iph->ihl*4 + sizeof(struct tcphdr),
 				optlen, _opt);

Re: [PATCH]: fixed kernel panic when trying to find tcp option

[David S. Miller]

On Wed, 15 Sep 2004 20:36:44 +0900 (JST)
Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> wrote:

> This patch fixed panic on 2.6.9-rc2 when trying to find tcp option
> in the packet which has no option.
> 
> If not fixed yet, please apply this patch.

This patch is unnecessary, you might want to explain how
this condition can possibly occur.

All callers, well there is only one, make sure optlen is
non-zero.

Here is the call site in tcp_match():

	if (tcpinfo->option) {
		if (th->doff * 4 < sizeof(_tcph)) {
			*hotdrop = 1;
			return 0;
		}
		if (!tcp_find_option(tcpinfo->option, skb,
				     th->doff*4 - sizeof(_tcph),
				     tcpinfo->invflags & IPT_TCP_INV_OPTION,
				     hotdrop))
			return 0;
	}

If the optlen would be zero, second if() passes and we
set *hotdrop to 1 and return.

This is why tcp_find_option() uses BUG_ON(!optlen), it means
that callers must check this.

You must be changing this code or using it is some other way,
if so you must make sure you check for non-zero optlen before
invoking tcp_find_option().

Re: [PATCH]: fixed kernel panic when trying to find tcp option

[David S. Miller]

On Wed, 15 Sep 2004 08:43:35 -0700
"David S. Miller" <davem@davemloft.net> wrote:

> All callers, well there is only one, make sure optlen is
> non-zero.
> 
> Here is the call site in tcp_match():
> 
> 	if (tcpinfo->option) {
> 		if (th->doff * 4 < sizeof(_tcph)) {
> 			*hotdrop = 1;
> 			return 0;
> 		}
> 		if (!tcp_find_option(tcpinfo->option, skb,
> 				     th->doff*4 - sizeof(_tcph),
> 				     tcpinfo->invflags & IPT_TCP_INV_OPTION,
> 				     hotdrop))
> 			return 0;
> 	}
> 
> If the optlen would be zero, second if() passes and we
> set *hotdrop to 1 and return.

Wait, that is the bug, the condition test has an
off-by-one error, it should read:

		if (th->doff * 4 <= sizeof(_tcph))

I will fix this in my tree.

Re: [PATCH]: fixed kernel panic when trying to find tcp option

[Yasuyuki Kozakai]

From: "David S. Miller" <davem@davemloft.net>
Date: Wed, 15 Sep 2004 08:50:49 -0700

> On Wed, 15 Sep 2004 08:43:35 -0700
> "David S. Miller" <davem@davemloft.net> wrote:
> 
> > All callers, well there is only one, make sure optlen is
> > non-zero.
> > 
> > Here is the call site in tcp_match():
> > 
> > 	if (tcpinfo->option) {
> > 		if (th->doff * 4 < sizeof(_tcph)) {
> > 			*hotdrop = 1;
> > 			return 0;
> > 		}
> > 		if (!tcp_find_option(tcpinfo->option, skb,
> > 				     th->doff*4 - sizeof(_tcph),
> > 				     tcpinfo->invflags & IPT_TCP_INV_OPTION,
> > 				     hotdrop))
> > 			return 0;
> > 	}
> > 
> > If the optlen would be zero, second if() passes and we
> > set *hotdrop to 1 and return.
> 
> Wait, that is the bug, the condition test has an
> off-by-one error, it should read:
> 
> 		if (th->doff * 4 <= sizeof(_tcph))
> 
> I will fix this in my tree.

If you do it, all tcp packets which has no option will be dropped
in the case of adding tcp-option match rule.

For example,

Try to configure

	ip6tables -p tcp --tcp-option 2 -j LOG

and send tcp packets which has no tcp option.

With this situation, rule should not match. Then, tcp_match()
should return 0. On the other hand, if you configure

	ip6tables -p tcp ! --tcp-option 2 -j LOG

tcp_match() should return 1.

This is the reason I sent that patch.

Regards,

-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>

Re: [PATCH]: fixed kernel panic when trying to find tcp option

[David S. Miller]

On Thu, 16 Sep 2004 02:20:18 +0900 (JST)
Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> wrote:

> If you do it, all tcp packets which has no option will be dropped
> in the case of adding tcp-option match rule.

You are right.

Did your patch return "invert" from tcp_find_option() when optlen
is zero?  I deleted it, sorry.

Re: [PATCH]: fixed kernel panic when trying to find tcp option

[Yasuyuki Kozakai]

From: "David S. Miller" <davem@davemloft.net>
Date: Wed, 15 Sep 2004 10:22:36 -0700

> On Thu, 16 Sep 2004 02:20:18 +0900 (JST)
> Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp> wrote:
> 
> > If you do it, all tcp packets which has no option will be dropped
> > in the case of adding tcp-option match rule.
> 
> You are right.
> 
> Did your patch return "invert" from tcp_find_option() when optlen
> is zero?  I deleted it, sorry.

Yes. Because

From: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>
Date: Thu, 16 Sep 2004 02:20:18 +0900 (JST)

> should return 0. On the other hand, if you configure
> 
> 	ip6tables -p tcp ! --tcp-option 2 -j LOG
> 
> tcp_match() should return 1.

Regards,

-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>