Hi Sasha,
Wed, Mar 02, 2016 at 10:23:52PM IST, sasha.levin@xxxxxxxxxx wrote:
>From: Ido Schimmel <idosch@xxxxxxxxxxxx>
>
>This patch has been added to the 4.1 stable tree. If you have any
>objections, please let us know.
>
>===============
>
>[ Upstream commit 4f2c6ae5c64c353fb1b0425e4747e5603feadba1 ]
>
>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.
You removed the fdb_lock bits from the commit below since they aren't
present in kernel 4.1. Can you remove it from the description as well?
Should probably be:
"Also, remove switchdev_mutex, as it's no longer needed when RTNL mutex
is held."
Thanks, Ido.
>
>Fixes: 03bf0c281234 ("switchdev: introduce switchdev notifier")
>Signed-off-by: Ido Schimmel <idosch@xxxxxxxxxxxx>
>Signed-off-by: Jiri Pirko <jiri@xxxxxxxxxxxx>
>Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
>Signed-off-by: Sasha Levin <sasha.levin@xxxxxxxxxx>
>---
> drivers/net/ethernet/rocker/rocker.c | 2 ++
> net/bridge/br.c | 3 +--
> net/switchdev/switchdev.c | 15 ++++++++-------
> 3 files changed, 11 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
>index 73b6fc2..4fedf7f 100644
>--- a/drivers/net/ethernet/rocker/rocker.c
>+++ b/drivers/net/ethernet/rocker/rocker.c
>@@ -3384,12 +3384,14 @@ static void rocker_port_fdb_learn_work(struct work_struct *work)
> info.addr = lw->addr;
> info.vid = lw->vid;
>
>+ rtnl_lock();
> if (learned && removing)
> call_netdev_switch_notifiers(NETDEV_SWITCH_FDB_DEL,
> lw->dev, &info.info);
> else if (learned && !removing)
> call_netdev_switch_notifiers(NETDEV_SWITCH_FDB_ADD,
> lw->dev, &info.info);
>+ rtnl_unlock();
>
> kfree(work);
> }
>diff --git a/net/bridge/br.c b/net/bridge/br.c
>index 02c24cf..c72e01c 100644
>--- a/net/bridge/br.c
>+++ b/net/bridge/br.c
>@@ -121,6 +121,7 @@ static struct notifier_block br_device_notifier = {
> .notifier_call = br_device_event
> };
>
>+/* called with RTNL */
> static int br_netdev_switch_event(struct notifier_block *unused,
> unsigned long event, void *ptr)
> {
>@@ -130,7 +131,6 @@ static int br_netdev_switch_event(struct notifier_block *unused,
> struct netdev_switch_notifier_fdb_info *fdb_info;
> int err = NOTIFY_DONE;
>
>- rtnl_lock();
> p = br_port_get_rtnl(dev);
> if (!p)
> goto out;
>@@ -155,7 +155,6 @@ static int br_netdev_switch_event(struct notifier_block *unused,
> }
>
> out:
>- rtnl_unlock();
> return err;
> }
>
>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>index 055453d..a8dbe80 100644
>--- a/net/switchdev/switchdev.c
>+++ b/net/switchdev/switchdev.c
>@@ -15,6 +15,7 @@
> #include <linux/mutex.h>
> #include <linux/notifier.h>
> #include <linux/netdevice.h>
>+#include <linux/rtnetlink.h>
> #include <net/ip_fib.h>
> #include <net/switchdev.h>
>
>@@ -64,7 +65,6 @@ int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
> }
> EXPORT_SYMBOL_GPL(netdev_switch_port_stp_update);
>
>-static DEFINE_MUTEX(netdev_switch_mutex);
> static RAW_NOTIFIER_HEAD(netdev_switch_notif_chain);
>
> /**
>@@ -79,9 +79,9 @@ int register_netdev_switch_notifier(struct notifier_block *nb)
> {
> int err;
>
>- mutex_lock(&netdev_switch_mutex);
>+ rtnl_lock();
> err = raw_notifier_chain_register(&netdev_switch_notif_chain, nb);
>- mutex_unlock(&netdev_switch_mutex);
>+ rtnl_unlock();
> return err;
> }
> EXPORT_SYMBOL_GPL(register_netdev_switch_notifier);
>@@ -97,9 +97,9 @@ int unregister_netdev_switch_notifier(struct notifier_block *nb)
> {
> int err;
>
>- mutex_lock(&netdev_switch_mutex);
>+ rtnl_lock();
> err = raw_notifier_chain_unregister(&netdev_switch_notif_chain, nb);
>- mutex_unlock(&netdev_switch_mutex);
>+ rtnl_unlock();
> return err;
> }
> EXPORT_SYMBOL_GPL(unregister_netdev_switch_notifier);
>@@ -113,16 +113,17 @@ EXPORT_SYMBOL_GPL(unregister_netdev_switch_notifier);
> * Call all network notifier blocks. This should be called by driver
> * when it needs to propagate hardware event.
> * Return values are same as for atomic_notifier_call_chain().
>+ * rtnl_lock must be held.
> */
> int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
> struct netdev_switch_notifier_info *info)
> {
> int err;
>
>+ ASSERT_RTNL();
>+
> info->dev = dev;
>- mutex_lock(&netdev_switch_mutex);
> err = raw_notifier_call_chain(&netdev_switch_notif_chain, val, info);
>- mutex_unlock(&netdev_switch_mutex);
> return err;
> }
> EXPORT_SYMBOL_GPL(call_netdev_switch_notifiers);
>--
>2.5.0
>
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html