IB/ipoib: Fix deadlock between ipoib_stop and mcast join flow

Before calling ipoib_stop, rtnl_lock should be taken, then
the flow clears the IPOIB_FLAG_ADMIN_UP and IPOIB_FLAG_OPER_UP
flags, and waits for mcast completion if IPOIB_MCAST_FLAG_BUSY
is set.

On the other hand, the flow of multicast join task initializes
a mcast completion, sets the IPOIB_MCAST_FLAG_BUSY and calls
ipoib_mcast_join. If IPOIB_FLAG_OPER_UP flag is not set, this
call returns EINVAL without setting the mcast completion and
leads to a deadlock.

    ipoib_stop                          |
        |                               |
    clear_bit(IPOIB_FLAG_ADMIN_UP)      |
        |                               |
    Context Switch                      |
        |                       ipoib_mcast_join_task
        |                               |
        |                       spin_lock_irq(lock)
        |                               |
        |                       init_completion(mcast)
        |                               |
        |                       set_bit(IPOIB_MCAST_FLAG_BUSY)
        |                               |
        |                       Context Switch
        |                               |
    clear_bit(IPOIB_FLAG_OPER_UP)       |
        |                               |
    spin_lock_irqsave(lock)             |
        |                               |
    Context Switch                      |
        |                       ipoib_mcast_join
        |                       return (-EINVAL)
        |                               |
        |                       spin_unlock_irq(lock)
        |                               |
        |                       Context Switch
        |                               |
    ipoib_mcast_dev_flush               |
    wait_for_completion(mcast)          |

ipoib_stop will wait for mcast completion for ever, and will
not release the rtnl_lock. As a result panic occurs with the
following trace:

    [13441.639268] Call Trace:
    [13441.640150]  [<ffffffff8168b579>] schedule+0x29/0x70
    [13441.641038]  [<ffffffff81688fc9>] schedule_timeout+0x239/0x2d0
    [13441.641914]  [<ffffffff810bc017>] ? complete+0x47/0x50
    [13441.642765]  [<ffffffff810a690d>] ? flush_workqueue_prep_pwqs+0x16d/0x200
    [13441.643580]  [<ffffffff8168b956>] wait_for_completion+0x116/0x170
    [13441.644434]  [<ffffffff810c4ec0>] ? wake_up_state+0x20/0x20
    [13441.645293]  [<ffffffffa05af170>] ipoib_mcast_dev_flush+0x150/0x190 [ib_ipoib]
    [13441.646159]  [<ffffffffa05ac967>] ipoib_ib_dev_down+0x37/0x60 [ib_ipoib]
    [13441.647013]  [<ffffffffa05a4805>] ipoib_stop+0x75/0x150 [ib_ipoib]

Fixes: 08bc327629cb ("IB/ipoib: fix for rare multicast join race condition")
Signed-off-by: Feras Daoud <ferasda@mellanox.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
Signed-off-by: Doug Ledford <dledford@redhat.com>

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 8da1955e00030892f53fd36bbafe3885df3f7c3e..5433de83cdab9b71a1366e10a25b7f01696fcb53 100644 (file)
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -491,6 +491,9 @@ static int ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast)
            !test_bit(IPOIB_FLAG_OPER_UP, &priv->flags))
                return -EINVAL;
 
+       init_completion(&mcast->done);
+       set_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags);
+
        ipoib_dbg_mcast(priv, "joining MGID %pI6\n", mcast->mcmember.mgid.raw);
 
        rec.mgid     = mcast->mcmember.mgid;
@@ -649,8 +652,6 @@ void ipoib_mcast_join_task(struct work_struct *work)
                        if (mcast->backoff == 1 ||
                            time_after_eq(jiffies, mcast->delay_until)) {
                                /* Found the next unjoined group */
-                               init_completion(&mcast->done);
-                               set_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags);
                                if (ipoib_mcast_join(dev, mcast)) {
                                        spin_unlock_irq(&priv->lock);
                                        return;
@@ -670,11 +671,9 @@ out:
                queue_delayed_work(priv->wq, &priv->mcast_task,
                                   delay_until - jiffies);
        }
-       if (mcast) {
-               init_completion(&mcast->done);
-               set_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags);
+       if (mcast)
                ipoib_mcast_join(dev, mcast);
-       }
+
        spin_unlock_irq(&priv->lock);
 }