From patchwork Thu Aug 30 07:01:30 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick McHardy X-Patchwork-Id: 1386691 X-Patchwork-Delegate: roland@digitalvampire.org Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id F1A1DDF215 for ; Thu, 30 Aug 2012 07:11:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752522Ab2H3HL2 (ORCPT ); Thu, 30 Aug 2012 03:11:28 -0400 Received: from stinky.trash.net ([213.144.137.162]:47492 "EHLO stinky.trash.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752477Ab2H3HL1 (ORCPT ); Thu, 30 Aug 2012 03:11:27 -0400 X-Greylist: delayed 595 seconds by postgrey-1.27 at vger.kernel.org; Thu, 30 Aug 2012 03:11:27 EDT Received: from localhost (localhost [127.0.0.1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by stinky.trash.net (Postfix) with ESMTPS id 72B87B2C40; Thu, 30 Aug 2012 09:01:31 +0200 (MEST) Date: Thu, 30 Aug 2012 09:01:30 +0200 (MEST) From: Patrick McHardy To: Roland Dreier cc: Sean Hefty , Hal Rosenstock , linux-rdma@vger.kernel.org Subject: [PATCH] IB/ipoib: Fix crash resulted as of use after free for multicast object Message-ID: MIME-Version: 1.0 Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org 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 --- 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 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);