Re: bridge netfilter output bug on 2.6.39

[Eric Dumazet <eric.dumazet@gmail.com>]

Le mardi 24 mai 2011 à 07:41 -0700, Stephen Hemminger a écrit :
> Got this bug report against 2.6.39.  Looks like ip_fragment() is now
> getting confused when called from bridge netfilter. Probably related to
> the changes to do ip_options_compile for the bridge input path.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=35672
> 
> May 23 02:04:24 lxc kernel: [99498.329036] BUG: unable to handle kernel NULL
> pointer dereference at 00000004
> May 23 02:04:24 lxc kernel: [99498.330017] IP: [<c143d6bf>] dst_mtu+0xb/0x1c
> May 23 02:04:24 lxc kernel: [99498.330017] *pdpt = 000000001fb55001 *pde =
> 0000000000000000
> May 23 02:04:24 lxc kernel: [99498.330017] Oops: 0000 [#1] SMP
> May 23 02:04:24 lxc kernel: [99498.330017] last sysfs file:
> /sys/devices/virtual/vc/vcsa8/uevent
> May 23 02:04:24 lxc kernel: [99498.330017] Modules linked in: lp ppdev
> parport_pc parport fuse firewire_ohci firewire_core crc_itu_t intel_agp
> intel_gtt
> May 23 02:04:24 lxc kernel: [99498.330017]
> May 23 02:04:24 lxc kernel: [99498.330017] Pid: 0, comm: swapper Not tainted
> 2.6.39-lxc #2 .   .  /IP35 Pro XE(Intel P35-ICH9R)
> May 23 02:04:24 lxc kernel: [99498.330017] EIP: 0060:[<c143d6bf>] EFLAGS:
> 00010246 CPU: 0
> May 23 02:04:24 lxc kernel: [99498.330017] EIP is at dst_mtu+0xb/0x1c
> May 23 02:04:24 lxc kernel: [99498.330017] EAX: 00000000 EBX: e90b6b40 ECX:
> effc981c EDX: effc9000
> May 23 02:04:24 lxc kernel: [99498.330017] ESI: c1a0d84e EDI: dda6331e EBP:
> f080bb44 ESP: f080bb44
> May 23 02:04:24 lxc kernel: [99498.330017]  DS: 007b ES: 007b FS: 00d8 GS: 0000
> SS: 0068
> May 23 02:04:24 lxc kernel: [99498.330017] Process swapper (pid: 0, ti=f080a000
> task=c172b7e0 task.ti=c1724000)
> May 23 02:04:24 lxc kernel: [99498.330017] Stack:
> May 23 02:04:24 lxc kernel: [99498.330017]  f080bb8c c143e20d 00000004 f080bb88
> c141aab2 c14b46db effc9000 00000014
> May 23 02:04:24 lxc kernel: [99498.330017]  c14b8a44 effc9000 e90b6b40 00000014
> effc981c e90b6b58 cd472800 e90b6b40
> May 23 02:04:24 lxc kernel: [99498.330017]  c14b8a44 dda6331e f080bb98 c14b8aa0
> e90b6b40 f080bba8 c14b881a e90b6b40
> May 23 02:04:24 lxc kernel: [99498.330017] Call Trace:
> May 23 02:04:24 lxc kernel: [99498.330017]  [<c143e20d>] ip_fragment+0xb5/0x66c
> May 23 02:04:24 lxc kernel: [99498.330017]  [<c141aab2>] ?
> nf_hook_slow+0x43/0xd1
> May 23 02:04:24 lxc kernel: [99498.330017]  [<c14b46db>] ? br_flood+0x83/0x83
> May 23 02:04:24 lxc kernel: [99498.330017]  [<c14b8a44>] ?
> br_parse_ip_options+0x1b0/0x1b0
> May 23 02:04:24 lxc kernel: [99498.330017]  [<c14b8a44>] ?
> br_parse_ip_options+0x1b0/0x1b0
> May 23 02:04:24 lxc kernel: [99498.330017]  [<c14b8aa0>]
> br_nf_dev_queue_xmit+0x5c/0x68
> --

I would say its more likely a problem with dst metrics changes

In this crash, we dereference a NULL dst->_metrics 'pointer' in
dst_metric_raw(dst, RTAX_MTU);

Hmm, it seems __dst_destroy_metrics_generic() doesnt add the
DST_METRICS_READ_ONLY flag ?

[PATCH] net: fix __dst_destroy_metrics_generic()

dst_default_metrics is readonly, we dont want to kfree() it later.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Stephen Hemminger <shemminger@vyatta.com>
CC: Herbert Xu <herbert@gondor.apana.org.au>
---
 net/core/dst.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/dst.c b/net/core/dst.c
index 81a4fa1..1badc98 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -315,7 +315,7 @@ void __dst_destroy_metrics_generic(struct dst_entry *dst, unsigned long old)
 {
 	unsigned long prev, new;
 
-	new = (unsigned long) dst_default_metrics;
+	new = ((unsigned long) dst_default_metrics) | DST_METRICS_READ_ONLY;
 	prev = cmpxchg(&dst->_metrics, old, new);
 	if (prev == old)
 		kfree(__DST_METRICS_PTR(old));