Re: Kernel panic when using bridge

[Stephen Hemminger <shemminger@vyatta.com>]

On Mon, 11 Apr 2011 18:48:00 -0500
Scot Doyle <lkml@scotdoyle.com> wrote:

> On 04/09/2011 02:19 AM, Hiroaki SHIMODA wrote:
> >
> > It seems that the bug trap is occurred in ip_options_compile() due to
> > rt is NULL.
> >
> > 	8b 96 cc 00 00 00       mov    0xcc(%rsi),%edx
> > rsi is rt, and 0xcc means rt->rt_spec_dst. So I think below code hit
> > the bug trap.
> >
> > 332	if (skb) {
> > 333		memcpy(&optptr[optptr[2]-1],&rt->rt_spec_dst, 4);<- here
> > 334		opt->is_changed = 1;
> > 335	}
> >
> > And call trace seems as follows.
> >    __netif_receive_skb()
> >      ->  br_handle_frame()
> >           ->  NF_HOOK()
> >                ->  br_nf_pre_routing()
> >                     ->  br_parse_ip_options()
> >                          ->  ip_options_compile()
> >
> > br_parse_ip_options() was introduced at 462fb2a (bridge : Sanitize
> > skb before it enters the IP stack) but ip_options_compile() or
> > ip_options_rcv_srr() seems to be called with no rt info.
> 
> Thanks to a tip from Sebastian, I can now reproduce this panic by 
> running "IP Stack Integrity Checker v0.07" from another machine on the 
> same subnet with command "icmpsic -s x.y.z.a -d x.y.z.b" where "x.y.z.a" 
> is IP address of the other machine and "x.y.z.b" is the IP address of 
> the target. When I enable iptables logging on the target machine, no 
> panic occurs. When I disable iptables logging (but otherwise leave the 
> same iptables rules) a panic occurs within a few seconds.
> 
> Thanks Hiroaki for the analysis of the kernel panic output. I've 
> confirmed that you are correct by placing a printk just before those two 
> lines. In every panic, the printk was triggered on line 333 of 
> net/ipv4/ip_options.c
> 
> The kernel panic does not occur after applying the following patch.
> 
> # diff net/ipv4/ip_options.c.original net/ipv4/ip_options.c.fix
> 332c332
> <                 if (skb) {
> ---
>  >                 if (skb && rt) {
> 374c374
> <                     if (skb) {
> ---
>  >                     if (skb && rt) {
> 
> What do you all think? Will it cause other problems?

It would help if you gave a little more context (like diff -up)
next time.

I think the correct fix is for the skb handed to ip_compile_options
to match the layout expected by ip_compile_options.

This patch is compile tested only, please validate.


Subject: [PATCH] bridge: set pseudo-route table before calling ip_comple_options

For some ip options, ip_compile_options assumes it can find the associated
route table. The bridge to iptables code doesn't supply the necessary
reference causing NULL dereference.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
Patch against net-next-2.6, but if validated should go to net-2.6
and stable.

--- a/net/bridge/br_netfilter.c	2011-04-11 18:18:22.534837859 -0700
+++ b/net/bridge/br_netfilter.c	2011-04-11 18:25:15.427244826 -0700
@@ -221,6 +221,7 @@ static int br_parse_ip_options(struct sk
 	struct ip_options *opt;
 	struct iphdr *iph;
 	struct net_device *dev = skb->dev;
+	struct rtable *rt;
 	u32 len;
 
 	iph = ip_hdr(skb);
@@ -255,6 +256,14 @@ static int br_parse_ip_options(struct sk
 		return 0;
 	}
 
+	/* Associate bogus bridge route table */
+	rt = bridge_parent_rtable(dev);
+	if (!rt) {
+		kfree_skb(skb);
+		return 0;
+	}
+	skb_dst_set(skb, &rt->dst);
+
 	opt->optlen = iph->ihl*4 - sizeof(struct iphdr);
 	if (ip_options_compile(dev_net(dev), opt, skb))
 		goto inhdr_error;