From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-db3on0086.outbound.protection.outlook.com ([157.55.234.86]:51328 "EHLO emea01-db3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751383AbcCCMIX (ORCPT ); Thu, 3 Mar 2016 07:08:23 -0500 Date: Thu, 3 Mar 2016 11:35:31 +0200 From: Ido Schimmel To: Sasha Levin CC: , , Jiri Pirko , "David S. Miller" Subject: Re: [added to the 4.1 stable tree] switchdev: Require RTNL mutex to be held when sending FDB notifications Message-ID: <20160303093531.GA5957@colbert.mtl.com> References: <1456950258-513-1-git-send-email-sasha.levin@oracle.com> <1456950258-513-8-git-send-email-sasha.levin@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <1456950258-513-8-git-send-email-sasha.levin@oracle.com> Sender: stable-owner@vger.kernel.org List-ID: Hi Sasha, Wed, Mar 02, 2016 at 10:23:52PM IST, sasha.levin@oracle.com wrote: >From: Ido Schimmel > >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 >Signed-off-by: Jiri Pirko >Signed-off-by: David S. Miller >Signed-off-by: Sasha Levin >--- > 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 > #include > #include >+#include > #include > #include > >@@ -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 >