xorl %eax, %eax

Linux kernel Bridge Multicast NULL Pointer Dereference

leave a comment »

This bug was reported to the netdev mailing list by Frank Arnold. From the given kernel OOPS we can read that there was a NULL pointer dereference while attempting to get the bridge’s MDB entry using br_mdb_ip_get(). As Eugene Teo pointed out this could be triggered remotely with an IGMP packet with no multicast table allocated. If this was the case, the MDB pointer will be pointing to NULL and the br_mdb_ip_get() (located at net/bridge/br_multicast.c) will execute the code below:

static struct net_bridge_mdb_entry *br_mdb_ip_get(
	struct net_bridge_mdb_htable *mdb, struct br_ip *dst)
{
	return __br_mdb_ip_get(mdb, dst, br_ip_hash(mdb, dst));
}

Which in turn, executes the following routine…

static struct net_bridge_mdb_entry *__br_mdb_ip_get(
        struct net_bridge_mdb_htable *mdb, __be32 dst, int hash)
{
        struct net_bridge_mdb_entry *mp;
        struct hlist_node *p;

        hlist_for_each_entry_rcu(mp, p, &mdb->mhash[hash], hlist[mdb->ver]) {
                if (dst == mp->addr)
                        return mp;
        }

        return NULL;
}

Clearly, the attempt to access ‘&mdb->mhash[hash]’ as well as ‘hlist[mdb->ver]’ will result in NULL pointer dereference since ‘mdb’ is set to NULL if there is no multicast table allocated. To fix this, the br_mdb_ip_get() was completely replaced by the following code:

static struct net_bridge_mdb_entry *br_mdb_ip_get(
	struct net_bridge_mdb_htable *mdb, struct br_ip *dst)
{
	if (!mdb)
		return NULL;

	return __br_mdb_ip_get(mdb, dst, br_ip_hash(mdb, dst));
}

Which explicitly checks ‘mdb’ against NULL before invoking __br_mdb_ip_get() and functions br_mdb_ip4_get() and br_mdb_ip6_get() which are responsible for the IPv4 and IPv6 packets respectively were updated to use this wrapper routine instead of __br_mdb_ip_get() directly. At last, br_mdb_get() was changed to remove the following check:

 struct net_bridge_mdb_entry *br_mdb_get(struct net_bridge *br,
 					struct sk_buff *skb)
 {
 	struct net_bridge_mdb_htable *mdb = br->mdb;
 	struct br_ip ip;
 
-	if (!mdb || br->multicast_disabled)
+	if (br->multicast_disabled)
 		return NULL;

Written by xorl

September 21, 2010 at 01:25

Posted in bugs, linux

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s