diff mbox

IB/ipoib: Fix crash resulted as of use after free for multicast object

Message ID Pine.GSO.4.63.1208300859100.27363@stinky-local.trash.net (mailing list archive)
State Accepted, archived
Delegated to: Roland Dreier
Headers show

Commit Message

Patrick McHardy Aug. 30, 2012, 7:01 a.m. UTC
Fix a crash in ipoib_mcast_join_task(). With help from Or Gerlitz.
[PATCH] IB/ipoib: Fix crash resulted as of use after free for multicast object

Commit c8c2afe3 "IPoIB: Use rtnl lock/unlock when changing device flags" 
added a call to rtnl_lock() in ipoib_mcast_join_task(), which is run from 
the ipoib_workqueue, and hence the workqueue can't be flushed from the 
context of ipoib_stop. 

In the current code, ipoib_stop() which doesn't flush the workqueue, 
calls ipoib_mcast_dev_flush() which goes and deletes all the multicast 
entries, and this flow place without any synchronization with possible 
running instance of ipoib_mcast_join_task() which relate to the 
same ipoib device, leading to a crashes with as of kernel 
NULL pointer dereference in that task.

The fix takes the approach of making sure the the workqueue
is flushed before ipoib_mcast_dev_flush() is called, where for that 
end, we move the RTNL lock wrapped code to ipoib_mcast_join_finish().

Signed-off-by: Patrick McHardy <kaber@trash.net>


---
 drivers/infiniband/ulp/ipoib/ipoib_main.c      |    2 +-
 drivers/infiniband/ulp/ipoib/ipoib_multicast.c |   19 ++++++++++---------
 2 files changed, 11 insertions(+), 10 deletions(-)

-- 
1.7.1

Comments

Roland Dreier Sept. 18, 2012, 5:18 p.m. UTC | #1
On Thu, Aug 30, 2012 at 12:01 AM, Patrick McHardy <kaber@trash.net> wrote:
> Fix a crash in ipoib_mcast_join_task(). With help from Or Gerlitz.

Thanks, applied.  But srsly, read over the changelog in that attachment
and think about a bit more proofreading next time around.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index ad7a456..b9129fe 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -150,7 +150,7 @@  static int ipoib_stop(struct net_device *dev)
 
 	netif_stop_queue(dev);
 
-	ipoib_ib_dev_down(dev, 0);
+	ipoib_ib_dev_down(dev, 1);
 	ipoib_ib_dev_stop(dev, 0);
 
 	if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) {
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 7536724..cecb98a 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -175,7 +175,9 @@  static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast,
 
 	mcast->mcmember = *mcmember;
 
-	/* Set the cached Q_Key before we attach if it's the broadcast group */
+	/* Set the multicast MTU and cached Q_Key before we attach if it's
+	 * the broadcast group.
+	 */
 	if (!memcmp(mcast->mcmember.mgid.raw, priv->dev->broadcast + 4,
 		    sizeof (union ib_gid))) {
 		spin_lock_irq(&priv->lock);
@@ -183,10 +185,17 @@  static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast,
 			spin_unlock_irq(&priv->lock);
 			return -EAGAIN;
 		}
+		priv->mcast_mtu = IPOIB_UD_MTU(ib_mtu_enum_to_int(priv->broadcast->mcmember.mtu));
 		priv->qkey = be32_to_cpu(priv->broadcast->mcmember.qkey);
 		spin_unlock_irq(&priv->lock);
 		priv->tx_wr.wr.ud.remote_qkey = priv->qkey;
 		set_qkey = 1;
+
+		if (!ipoib_cm_admin_enabled(dev)) {
+			rtnl_lock();
+			dev_set_mtu(dev, min(priv->mcast_mtu, priv->admin_mtu));
+			rtnl_unlock();
+		}
 	}
 
 	if (!test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags)) {
@@ -574,14 +583,6 @@  void ipoib_mcast_join_task(struct work_struct *work)
 		return;
 	}
 
-	priv->mcast_mtu = IPOIB_UD_MTU(ib_mtu_enum_to_int(priv->broadcast->mcmember.mtu));
-
-	if (!ipoib_cm_admin_enabled(dev)) {
-		rtnl_lock();
-		dev_set_mtu(dev, min(priv->mcast_mtu, priv->admin_mtu));
-		rtnl_unlock();
-	}
-
 	ipoib_dbg_mcast(priv, "successfully joined all multicast groups\n");
 
 	clear_bit(IPOIB_MCAST_RUN, &priv->flags);