diff options
author | Patrick McHardy <kaber@trash.net> | 2012-08-30 03:01:30 -0400 |
---|---|---|
committer | Roland Dreier <roland@purestorage.com> | 2012-09-30 23:32:33 -0400 |
commit | bea1e22df494a729978e7f2c54f7bda328f74bc3 (patch) | |
tree | 89d9e7ce19232caf86b38833fdbacbe8517b1f97 | |
parent | 979570e02981d4a8fc20b3cc8fd651856c98ee9d (diff) |
IPoIB: Fix use-after-free of multicast object
Fix a crash in ipoib_mcast_join_task(). (with help from Or Gerlitz)
Commit c8c2afe360b7 ("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. This takes place without any synchronization with
a possible running instance of ipoib_mcast_join_task() for the same
ipoib device, leading to a crash due to NULL pointer dereference.
Fix this by making sure that the workqueue is flushed before
ipoib_mcast_dev_flush() is called. To make that possible, we move the
RTNL-lock wrapped code to ipoib_mcast_join_finish().
Signed-off-by: Patrick McHardy <kaber@trash.net>
Cc: <stable@vger.kernel.org>
Signed-off-by: Roland Dreier <roland@purestorage.com>
-rw-r--r-- | drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 | ||||
-rw-r--r-- | drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 19 |
2 files changed, 11 insertions, 10 deletions
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 1e19b5ae7c47..ea0dfc77a7f4 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) | |||
150 | 150 | ||
151 | netif_stop_queue(dev); | 151 | netif_stop_queue(dev); |
152 | 152 | ||
153 | ipoib_ib_dev_down(dev, 0); | 153 | ipoib_ib_dev_down(dev, 1); |
154 | ipoib_ib_dev_stop(dev, 0); | 154 | ipoib_ib_dev_stop(dev, 0); |
155 | 155 | ||
156 | if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) { | 156 | 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 75367249f447..cecb98a4c662 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, | |||
175 | 175 | ||
176 | mcast->mcmember = *mcmember; | 176 | mcast->mcmember = *mcmember; |
177 | 177 | ||
178 | /* Set the cached Q_Key before we attach if it's the broadcast group */ | 178 | /* Set the multicast MTU and cached Q_Key before we attach if it's |
179 | * the broadcast group. | ||
180 | */ | ||
179 | if (!memcmp(mcast->mcmember.mgid.raw, priv->dev->broadcast + 4, | 181 | if (!memcmp(mcast->mcmember.mgid.raw, priv->dev->broadcast + 4, |
180 | sizeof (union ib_gid))) { | 182 | sizeof (union ib_gid))) { |
181 | spin_lock_irq(&priv->lock); | 183 | spin_lock_irq(&priv->lock); |
@@ -183,10 +185,17 @@ static int ipoib_mcast_join_finish(struct ipoib_mcast *mcast, | |||
183 | spin_unlock_irq(&priv->lock); | 185 | spin_unlock_irq(&priv->lock); |
184 | return -EAGAIN; | 186 | return -EAGAIN; |
185 | } | 187 | } |
188 | priv->mcast_mtu = IPOIB_UD_MTU(ib_mtu_enum_to_int(priv->broadcast->mcmember.mtu)); | ||
186 | priv->qkey = be32_to_cpu(priv->broadcast->mcmember.qkey); | 189 | priv->qkey = be32_to_cpu(priv->broadcast->mcmember.qkey); |
187 | spin_unlock_irq(&priv->lock); | 190 | spin_unlock_irq(&priv->lock); |
188 | priv->tx_wr.wr.ud.remote_qkey = priv->qkey; | 191 | priv->tx_wr.wr.ud.remote_qkey = priv->qkey; |
189 | set_qkey = 1; | 192 | set_qkey = 1; |
193 | |||
194 | if (!ipoib_cm_admin_enabled(dev)) { | ||
195 | rtnl_lock(); | ||
196 | dev_set_mtu(dev, min(priv->mcast_mtu, priv->admin_mtu)); | ||
197 | rtnl_unlock(); | ||
198 | } | ||
190 | } | 199 | } |
191 | 200 | ||
192 | if (!test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags)) { | 201 | if (!test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags)) { |
@@ -574,14 +583,6 @@ void ipoib_mcast_join_task(struct work_struct *work) | |||
574 | return; | 583 | return; |
575 | } | 584 | } |
576 | 585 | ||
577 | priv->mcast_mtu = IPOIB_UD_MTU(ib_mtu_enum_to_int(priv->broadcast->mcmember.mtu)); | ||
578 | |||
579 | if (!ipoib_cm_admin_enabled(dev)) { | ||
580 | rtnl_lock(); | ||
581 | dev_set_mtu(dev, min(priv->mcast_mtu, priv->admin_mtu)); | ||
582 | rtnl_unlock(); | ||
583 | } | ||
584 | |||
585 | ipoib_dbg_mcast(priv, "successfully joined all multicast groups\n"); | 586 | ipoib_dbg_mcast(priv, "successfully joined all multicast groups\n"); |
586 | 587 | ||
587 | clear_bit(IPOIB_MCAST_RUN, &priv->flags); | 588 | clear_bit(IPOIB_MCAST_RUN, &priv->flags); |