Commit 4f2c6ae5 authored by Ido Schimmel's avatar Ido Schimmel Committed by David S. Miller

switchdev: Require RTNL mutex to be held when sending FDB notifications

When switchdev drivers process FDB notifications from the underlying
device they resolve the netdev to which the entry points to and notify
the bridge using the switchdev notifier.

However, since the RTNL mutex is not held there is nothing preventing
the netdev from disappearing in the middle, which will cause
br_switchdev_event() to dereference a non-existing netdev.

Make switchdev drivers hold the lock at the beginning of the
notification processing session and release it once it ends, after
notifying the bridge.

Also, remove switchdev_mutex and fdb_lock, as they are no longer needed
when RTNL mutex is held.

Fixes: 03bf0c28 ("switchdev: introduce switchdev notifier")
Signed-off-by: default avatarIdo Schimmel <idosch@mellanox.com>
Signed-off-by: default avatarJiri Pirko <jiri@mellanox.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 7d0105b5
...@@ -120,7 +120,6 @@ struct mlxsw_sp { ...@@ -120,7 +120,6 @@ struct mlxsw_sp {
} fdb_notify; } fdb_notify;
#define MLXSW_SP_DEFAULT_AGEING_TIME 300 #define MLXSW_SP_DEFAULT_AGEING_TIME 300
u32 ageing_time; u32 ageing_time;
struct mutex fdb_lock; /* Make sure FDB sessions are atomic. */
struct mlxsw_sp_upper master_bridge; struct mlxsw_sp_upper master_bridge;
struct mlxsw_sp_upper lags[MLXSW_SP_LAG_MAX]; struct mlxsw_sp_upper lags[MLXSW_SP_LAG_MAX];
}; };
......
...@@ -45,6 +45,7 @@ ...@@ -45,6 +45,7 @@
#include <linux/if_bridge.h> #include <linux/if_bridge.h>
#include <linux/workqueue.h> #include <linux/workqueue.h>
#include <linux/jiffies.h> #include <linux/jiffies.h>
#include <linux/rtnetlink.h>
#include <net/switchdev.h> #include <net/switchdev.h>
#include "spectrum.h" #include "spectrum.h"
...@@ -1068,7 +1069,6 @@ static int mlxsw_sp_port_fdb_dump(struct mlxsw_sp_port *mlxsw_sp_port, ...@@ -1068,7 +1069,6 @@ static int mlxsw_sp_port_fdb_dump(struct mlxsw_sp_port *mlxsw_sp_port,
if (!sfd_pl) if (!sfd_pl)
return -ENOMEM; return -ENOMEM;
mutex_lock(&mlxsw_sp_port->mlxsw_sp->fdb_lock);
if (mlxsw_sp_port_is_vport(mlxsw_sp_port)) { if (mlxsw_sp_port_is_vport(mlxsw_sp_port)) {
u16 tmp; u16 tmp;
...@@ -1142,7 +1142,6 @@ static int mlxsw_sp_port_fdb_dump(struct mlxsw_sp_port *mlxsw_sp_port, ...@@ -1142,7 +1142,6 @@ static int mlxsw_sp_port_fdb_dump(struct mlxsw_sp_port *mlxsw_sp_port,
} while (num_rec == MLXSW_REG_SFD_REC_MAX_COUNT); } while (num_rec == MLXSW_REG_SFD_REC_MAX_COUNT);
out: out:
mutex_unlock(&mlxsw_sp_port->mlxsw_sp->fdb_lock);
kfree(sfd_pl); kfree(sfd_pl);
return stored_err ? stored_err : err; return stored_err ? stored_err : err;
} }
...@@ -1393,7 +1392,7 @@ static void mlxsw_sp_fdb_notify_work(struct work_struct *work) ...@@ -1393,7 +1392,7 @@ static void mlxsw_sp_fdb_notify_work(struct work_struct *work)
mlxsw_sp = container_of(work, struct mlxsw_sp, fdb_notify.dw.work); mlxsw_sp = container_of(work, struct mlxsw_sp, fdb_notify.dw.work);
mutex_lock(&mlxsw_sp->fdb_lock); rtnl_lock();
do { do {
mlxsw_reg_sfn_pack(sfn_pl); mlxsw_reg_sfn_pack(sfn_pl);
err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(sfn), sfn_pl); err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(sfn), sfn_pl);
...@@ -1406,7 +1405,7 @@ static void mlxsw_sp_fdb_notify_work(struct work_struct *work) ...@@ -1406,7 +1405,7 @@ static void mlxsw_sp_fdb_notify_work(struct work_struct *work)
mlxsw_sp_fdb_notify_rec_process(mlxsw_sp, sfn_pl, i); mlxsw_sp_fdb_notify_rec_process(mlxsw_sp, sfn_pl, i);
} while (num_rec); } while (num_rec);
mutex_unlock(&mlxsw_sp->fdb_lock); rtnl_unlock();
kfree(sfn_pl); kfree(sfn_pl);
mlxsw_sp_fdb_notify_work_schedule(mlxsw_sp); mlxsw_sp_fdb_notify_work_schedule(mlxsw_sp);
...@@ -1421,7 +1420,6 @@ static int mlxsw_sp_fdb_init(struct mlxsw_sp *mlxsw_sp) ...@@ -1421,7 +1420,6 @@ static int mlxsw_sp_fdb_init(struct mlxsw_sp *mlxsw_sp)
dev_err(mlxsw_sp->bus_info->dev, "Failed to set default ageing time\n"); dev_err(mlxsw_sp->bus_info->dev, "Failed to set default ageing time\n");
return err; return err;
} }
mutex_init(&mlxsw_sp->fdb_lock);
INIT_DELAYED_WORK(&mlxsw_sp->fdb_notify.dw, mlxsw_sp_fdb_notify_work); INIT_DELAYED_WORK(&mlxsw_sp->fdb_notify.dw, mlxsw_sp_fdb_notify_work);
mlxsw_sp->fdb_notify.interval = MLXSW_SP_DEFAULT_LEARNING_INTERVAL; mlxsw_sp->fdb_notify.interval = MLXSW_SP_DEFAULT_LEARNING_INTERVAL;
mlxsw_sp_fdb_notify_work_schedule(mlxsw_sp); mlxsw_sp_fdb_notify_work_schedule(mlxsw_sp);
......
...@@ -3531,12 +3531,14 @@ static void rocker_port_fdb_learn_work(struct work_struct *work) ...@@ -3531,12 +3531,14 @@ static void rocker_port_fdb_learn_work(struct work_struct *work)
info.addr = lw->addr; info.addr = lw->addr;
info.vid = lw->vid; info.vid = lw->vid;
rtnl_lock();
if (learned && removing) if (learned && removing)
call_switchdev_notifiers(SWITCHDEV_FDB_DEL, call_switchdev_notifiers(SWITCHDEV_FDB_DEL,
lw->rocker_port->dev, &info.info); lw->rocker_port->dev, &info.info);
else if (learned && !removing) else if (learned && !removing)
call_switchdev_notifiers(SWITCHDEV_FDB_ADD, call_switchdev_notifiers(SWITCHDEV_FDB_ADD,
lw->rocker_port->dev, &info.info); lw->rocker_port->dev, &info.info);
rtnl_unlock();
rocker_port_kfree(lw->trans, work); rocker_port_kfree(lw->trans, work);
} }
......
...@@ -121,6 +121,7 @@ static struct notifier_block br_device_notifier = { ...@@ -121,6 +121,7 @@ static struct notifier_block br_device_notifier = {
.notifier_call = br_device_event .notifier_call = br_device_event
}; };
/* called with RTNL */
static int br_switchdev_event(struct notifier_block *unused, static int br_switchdev_event(struct notifier_block *unused,
unsigned long event, void *ptr) unsigned long event, void *ptr)
{ {
...@@ -130,7 +131,6 @@ static int br_switchdev_event(struct notifier_block *unused, ...@@ -130,7 +131,6 @@ static int br_switchdev_event(struct notifier_block *unused,
struct switchdev_notifier_fdb_info *fdb_info; struct switchdev_notifier_fdb_info *fdb_info;
int err = NOTIFY_DONE; int err = NOTIFY_DONE;
rtnl_lock();
p = br_port_get_rtnl(dev); p = br_port_get_rtnl(dev);
if (!p) if (!p)
goto out; goto out;
...@@ -155,7 +155,6 @@ static int br_switchdev_event(struct notifier_block *unused, ...@@ -155,7 +155,6 @@ static int br_switchdev_event(struct notifier_block *unused,
} }
out: out:
rtnl_unlock();
return err; return err;
} }
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include <linux/list.h> #include <linux/list.h>
#include <linux/workqueue.h> #include <linux/workqueue.h>
#include <linux/if_vlan.h> #include <linux/if_vlan.h>
#include <linux/rtnetlink.h>
#include <net/ip_fib.h> #include <net/ip_fib.h>
#include <net/switchdev.h> #include <net/switchdev.h>
...@@ -567,7 +568,6 @@ int switchdev_port_obj_dump(struct net_device *dev, struct switchdev_obj *obj, ...@@ -567,7 +568,6 @@ int switchdev_port_obj_dump(struct net_device *dev, struct switchdev_obj *obj,
} }
EXPORT_SYMBOL_GPL(switchdev_port_obj_dump); EXPORT_SYMBOL_GPL(switchdev_port_obj_dump);
static DEFINE_MUTEX(switchdev_mutex);
static RAW_NOTIFIER_HEAD(switchdev_notif_chain); static RAW_NOTIFIER_HEAD(switchdev_notif_chain);
/** /**
...@@ -582,9 +582,9 @@ int register_switchdev_notifier(struct notifier_block *nb) ...@@ -582,9 +582,9 @@ int register_switchdev_notifier(struct notifier_block *nb)
{ {
int err; int err;
mutex_lock(&switchdev_mutex); rtnl_lock();
err = raw_notifier_chain_register(&switchdev_notif_chain, nb); err = raw_notifier_chain_register(&switchdev_notif_chain, nb);
mutex_unlock(&switchdev_mutex); rtnl_unlock();
return err; return err;
} }
EXPORT_SYMBOL_GPL(register_switchdev_notifier); EXPORT_SYMBOL_GPL(register_switchdev_notifier);
...@@ -600,9 +600,9 @@ int unregister_switchdev_notifier(struct notifier_block *nb) ...@@ -600,9 +600,9 @@ int unregister_switchdev_notifier(struct notifier_block *nb)
{ {
int err; int err;
mutex_lock(&switchdev_mutex); rtnl_lock();
err = raw_notifier_chain_unregister(&switchdev_notif_chain, nb); err = raw_notifier_chain_unregister(&switchdev_notif_chain, nb);
mutex_unlock(&switchdev_mutex); rtnl_unlock();
return err; return err;
} }
EXPORT_SYMBOL_GPL(unregister_switchdev_notifier); EXPORT_SYMBOL_GPL(unregister_switchdev_notifier);
...@@ -616,16 +616,17 @@ EXPORT_SYMBOL_GPL(unregister_switchdev_notifier); ...@@ -616,16 +616,17 @@ EXPORT_SYMBOL_GPL(unregister_switchdev_notifier);
* Call all network notifier blocks. This should be called by driver * Call all network notifier blocks. This should be called by driver
* when it needs to propagate hardware event. * when it needs to propagate hardware event.
* Return values are same as for atomic_notifier_call_chain(). * Return values are same as for atomic_notifier_call_chain().
* rtnl_lock must be held.
*/ */
int call_switchdev_notifiers(unsigned long val, struct net_device *dev, int call_switchdev_notifiers(unsigned long val, struct net_device *dev,
struct switchdev_notifier_info *info) struct switchdev_notifier_info *info)
{ {
int err; int err;
ASSERT_RTNL();
info->dev = dev; info->dev = dev;
mutex_lock(&switchdev_mutex);
err = raw_notifier_call_chain(&switchdev_notif_chain, val, info); err = raw_notifier_call_chain(&switchdev_notif_chain, val, info);
mutex_unlock(&switchdev_mutex);
return err; return err;
} }
EXPORT_SYMBOL_GPL(call_switchdev_notifiers); EXPORT_SYMBOL_GPL(call_switchdev_notifiers);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment