Patchwork BUG: unable to handle kernel NULL pointer dereference at br_multicast_leave_group

login
register
mail settings
Submitter michael-dev@fami-braun.de
Date March 14, 2010, 2:28 a.m.
Message ID <4B9C49E7.1080703@fami-braun.de>
Download mbox | patch
Permalink /patch/47725/
State RFC
Delegated to: David Miller
Headers show

Comments

michael-dev@fami-braun.de - March 14, 2010, 2:28 a.m.
Hi,

I'm currently using linux-next and have been running into an OOPs which
I think might be caused by a patch you submitted on 2010-02-27.

It's a linux-next kernel from 2010-03-12 on an x86 system and it
OOPs in the bridge module in br_mdp_ip_get (called by
br_multicast_leave_group) because the br->mdb is null.

Digging deeper it seems that when a igmp leave packet is received, it is
not checked wether mdb has been populated yet. Though, if the group does
not exist, the code just skips leaving. So it seems the same action cna
be taken if mdb is still NULL. As far as I can see, commit
eb1d16414339a6e113d89e2cca2556005d7ce919 added multicast code to bridge
and so likely causes this crash. This crash does not happen on 2.6.33.

I've therefore modified br_multicast so that the leave is skipped if mdb
is empty.

Please don't hesitate to contact me if there are any questions.

Sincerely,
 M. Braun

// gdb output //
(gdb) print *((struct net_bridge*) 0xcec68800)
$3 = {lock = {{rlock = {raw_lock = {<No data fields>}}}}, port_list =
{next = 0xcec41790, prev = 0xcfb209f8}, dev = 0xcec68520, hash_lock =
{{rlock = {raw_lock = {<No data fields>}}}}, hash = {{first = 0x0},
{first = 0x0}, {first = 0x0}, {first = 0x0}, {
      first = 0xcfb4d980}, {first = 0x0} <repeats 19 times>, {first =
0xcec41200}, {first = 0x0} <repeats 119 times>, {first = 0xcf00ea80},
{first = 0x0} <repeats 29 times>, {first = 0xcfb6d680}, {first = 0x0}
<repeats 52 times>, {first = 0xcec40b00}, {first = 0x0}, {
      first = 0x0}, {first = 0x0}, {first = 0x0}, {first = 0x0}, {first
= 0x0}, {first = 0x0}, {first = 0xcf00d1c0}, {first = 0x0} <repeats 20
times>}, feature_mask = 16726117, fake_rtable = {u = {dst = {rcu_head =
{next = 0x0, func = 0}, child = 0x0, dev = 0xcec68520,
        error = 0, obsolete = 0, flags = 2, expires = 0, header_len = 0,
trailer_len = 0, rate_tokens = 0, rate_last = 0, path = 0xcec68c10,
neighbour = 0x0, hh = 0x0, xfrm = 0x0, input = 0, output = 0, ops =
0xd0cc2a40, metrics = {0, 1500, 0 <repeats 12 times>},
        tclassid = 0, __refcnt = {counter = 2}, __use = 0, lastuse = 0,
{next = 0x0, rt_next = 0x0, rt6_next = 0x0, dn_next = 0x0}}}, fl = {oif
= 0, iif = 0, mark = 0, nl_u = {ip4_u = {daddr = 0, saddr = 0, tos = 0
'\000', scope = 0 '\000'}, ip6_u = {daddr = {in6_u = {
              u6_addr8 = '\000' <repeats 15 times>, u6_addr16 = {0, 0,
0, 0, 0, 0, 0, 0}, u6_addr32 = {0, 0, 0, 0}}}, saddr = {in6_u =
{u6_addr8 = '\000' <repeats 15 times>, u6_addr16 = {0, 0, 0, 0, 0, 0, 0,
0}, u6_addr32 = {0, 0, 0, 0}}}, flowlabel = 0}, dn_u = {
          daddr = 0, saddr = 0, scope = 0 '\000'}}, proto = 0 '\000',
flags = 0 '\000', uli_u = {ports = {sport = 0, dport = 0}, icmpt = {type
= 0 '\000', code = 0 '\000'}, dnports = {sport = 0, dport = 0}, spi = 0,
mht = {type = 0 '\000'}}, secid = 0}, idev = 0x0,
    rt_genid = 0, rt_flags = 0, rt_type = 0, rt_dst = 0, rt_src = 0,
rt_iif = 0, rt_gateway = 0, rt_spec_dst = 0, peer = 0x0}, flags = 0,
designated_root = {prio = "\200", addr = "\000\016\216\031\356v"},
bridge_id = {prio = "\200", addr = "\000\016\216\031\356v"},
  root_path_cost = 0, max_age = 20000, hello_time = 2000, forward_delay
= 15000, bridge_max_age = 20000, ageing_time = 300000, bridge_hello_time
= 2000, bridge_forward_delay = 15000, group_addr =
"\001\200\302\000\000", root_port = 0, stp_enabled = BR_KERNEL_STP,
  topology_change = 0 '\000', topology_change_detected = 0 '\000',
multicast_router = 1 '\001', multicast_disabled = 0 '\000',
hash_elasticity = 4, hash_max = 512, multicast_last_member_count = 2,
multicast_startup_queries_sent = 2, multicast_startup_query_count = 2,
  multicast_last_member_interval = 1000, multicast_membership_interval =
260000, multicast_querier_interval = 255000, multicast_query_interval =
125000, multicast_query_response_interval = 10000,
multicast_startup_query_interval = 31250, multicast_lock = {{rlock = {
        raw_lock = {<No data fields>}}}}, mdb = 0x0, router_list =
{first = 0x0}, mglist = {first = 0x0}, multicast_router_timer = {entry =
{next = 0x0, prev = 0x0}, expires = 0, function = 0xd0cbf264
<br_multicast_local_router_expired>, data = 0, base = 0xc0579640},
  multicast_querier_timer = {entry = {next = 0x0, prev = 0x0}, expires =
0, function = 0xd0cbf264 <br_multicast_local_router_expired>, data = 0,
base = 0xc0579640}, multicast_query_timer = {entry = {next = 0xd0a1f630,
prev = 0xc057a1a4}, expires = 1756782,
    function = 0xd0cc01e1 <br_multicast_query_expired>, data =
3469117440, base = 0xc0579640}, hello_timer = {entry = {next =
0xc0579f14, prev = 0xcfcb6e10}, expires = 1694000, function = 0xd0cbc75b
<br_hello_timer_expired>, data = 3469117440, base = 0xc0579640},
  tcn_timer = {entry = {next = 0x0, prev = 0x0}, expires = 0, function =
0xd0cbc6ce <br_tcn_timer_expired>, data = 3469117440, base =
0xc0579640}, topology_change_timer = {entry = {next = 0x0, prev =
0x200200}, expires = 225844,
    function = 0xd0cbc42c <br_topology_change_timer_expired>, data =
3469117440, base = 0xc0579640}, gc_timer = {entry = {next = 0xcfcb6e10,
prev = 0xc0579f14}, expires = 1694000, function = 0xd0cb8d8f
<br_fdb_cleanup>, data = 3469117440, base = 0xc0579640},
  ifobj = 0xcfca1164}
(gdb) print ((struct net_bridge*) 0xcec68800)->mdb
$4 = (struct net_bridge_mdb_htable *) 0x0
(gdb) bt
#0  jhash_3words (mdb=0x0, dst=0) at include/linux/jhash.h:128
#1  jhash_1word (mdb=0x0, dst=0) at include/linux/jhash.h:140
#2  br_ip_hash (mdb=0x0, dst=0) at net/bridge/br_multicast.c:32
#3  br_mdb_ip_get (mdb=0x0, dst=0) at net/bridge/br_multicast.c:52
#4  0xd0cc0ec8 in br_multicast_leave_group (br=0xcec68800, port=<value
optimised out>, skb=0xcf00e580) at net/bridge/br_multicast.c:902
#5  br_multicast_ipv4_rcv (br=0xcec68800, port=<value optimised out>,
skb=0xcf00e580) at net/bridge/br_multicast.c:1033
#6  br_multicast_rcv (br=0xcec68800, port=<value optimised out>,
skb=0xcf00e580) at net/bridge/br_multicast.c:1052
#7  0xd0cb9c06 in br_handle_frame_finish (skb=0xcf00e580) at
net/bridge/br_input.c:55
#8  0xd0cbea59 in NF_HOOK_THRESH (skb=0xcf00e580) at
include/linux/netfilter.h:206
#9  br_nf_pre_routing_finish (skb=0xcf00e580) at
net/bridge/br_netfilter.c:420
#10 0xd0cbf015 in NF_HOOK_THRESH (hook=<value optimised out>,
skb=0xcf00e580, in=<value optimised out>, out=0x0, okfn=0xd0cb9bad
<br_handle_frame_finish>) at include/linux/netfilter.h:206
#11 NF_HOOK (hook=<value optimised out>, skb=0xcf00e580, in=<value
optimised out>, out=0x0, okfn=0xd0cb9bad <br_handle_frame_finish>) at
include/linux/netfilter.h:228
#12 br_nf_pre_routing (hook=<value optimised out>, skb=0xcf00e580,
in=<value optimised out>, out=0x0, okfn=0xd0cb9bad
<br_handle_frame_finish>) at net/bridge/br_netfilter.c:610
#13 0xc0377247 in ?? ()



// commit //
commit eb1d16414339a6e113d89e2cca2556005d7ce919
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Sat Feb 27 19:41:45 2010 +0000

    bridge: Add core IGMP snooping support

    This patch adds the core functionality of IGMP snooping support
    without actually hooking it up.  So this patch should be a no-op
    as far as the bridge's external behaviour is concerned.

    All the new code and data is controlled by the Kconfig option
    BRIDGE_IGMP_SNOOPING.  A run-time toggle is also available.

    The multicast switching is done using an hash table that is
    lockless on the read-side through RCU.  On the write-side the
    new multicast_lock is used for all operations.  The hash table
    supports dynamic growth/rehashing.

    The hash table will be rehashed if any chain length exceeds a
    preset limit.  If rehashing does not reduce the maximum chain
    length then snooping will be disabled.

    These features may be added in future (in no particular order):

    * IGMPv3 source support
    * Non-querier router detection
    * IPv6

    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Patch

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 12ce1ea..85ca985 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -899,6 +899,8 @@  static void br_multicast_leave_group(struct net_bridge *br,
 		goto out;
 
 	mdb = br->mdb;
+	if (!mdb)
+		goto out;
 	mp = br_mdb_ip_get(mdb, group);
 	if (!mp)
 		goto out;