bridge: vlan: fix possible null ptr derefs on port init and deinit

When a new port is being added we need to make vlgrp available after
rhashtable has been initialized and when removing a port we need to
flush the vlans and free the resources after we're sure noone can use
the port, i.e. after it's removed from the port list and synchronize_rcu
is executed.

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 45e4757c6fd25ed05a14e24e2d026204c3e79506..934cae9fa317851baeb2045b751a41c94a270a4c 100644 (file)
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -248,7 +248,6 @@ static void del_nbp(struct net_bridge_port *p)
 
        list_del_rcu(&p->list);
 
-       nbp_vlan_flush(p);
        br_fdb_delete_by_port(br, p, 0, 1);
        nbp_update_port_count(br);
 
@@ -257,6 +256,8 @@ static void del_nbp(struct net_bridge_port *p)
        dev->priv_flags &= ~IFF_BRIDGE_PORT;
 
        netdev_rx_handler_unregister(dev);
+       /* use the synchronize_rcu done by netdev_rx_handler_unregister */
+       nbp_vlan_flush(p);
 
        br_multicast_del_port(p);
 

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 90ac4b0c55c13f7108f542e86f102ec68133cbf6..7e9d60a402e20648684c9c224efa2726443ffd30 100644 (file)
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -854,16 +854,20 @@ err_rhtbl:
 
 int nbp_vlan_init(struct net_bridge_port *p)
 {
+       struct net_bridge_vlan_group *vg;
        int ret = -ENOMEM;
 
-       p->vlgrp = kzalloc(sizeof(struct net_bridge_vlan_group), GFP_KERNEL);
-       if (!p->vlgrp)
+       vg = kzalloc(sizeof(struct net_bridge_vlan_group), GFP_KERNEL);
+       if (!vg)
                goto out;
 
-       ret = rhashtable_init(&p->vlgrp->vlan_hash, &br_vlan_rht_params);
+       ret = rhashtable_init(&vg->vlan_hash, &br_vlan_rht_params);
        if (ret)
                goto err_rhtbl;
-       INIT_LIST_HEAD(&p->vlgrp->vlan_list);
+       INIT_LIST_HEAD(&vg->vlan_list);
+       /* Make sure everything's committed before publishing vg */
+       smp_wmb();
+       p->vlgrp = vg;
        if (p->br->default_pvid) {
                ret = nbp_vlan_add(p, p->br->default_pvid,
                                   BRIDGE_VLAN_INFO_PVID |
@@ -875,9 +879,9 @@ out:
        return ret;
 
 err_vlan_add:
-       rhashtable_destroy(&p->vlgrp->vlan_hash);
+       rhashtable_destroy(&vg->vlan_hash);
 err_rhtbl:
-       kfree(p->vlgrp);
+       kfree(vg);
 
        goto out;
 }