Re: [PATCH 4.4 14/36] IB/ipoib: Fix deadlock between rmmod and set_mode

[Ben Hutchings]

On Mon, 2017-03-13 at 16:39 +0800, Greg Kroah-Hartman wrote:
> 4.4-stable review patch.ÂÂIf anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Feras Daoud <ferasda@xxxxxxxxxxxx>
> 
> commit 0a0007f28304cb9fc87809c86abb80ec71317f20 upstream.
> 
> When calling set_mode from sys/fs, the call flow locks the sys/fs lock
> first and then tries to lock rtnl_lock (when calling ipoib_set_mod).
> On the other hand, the rmmod call flow takes the rtnl_lock first
> (when calling unregister_netdev) and then tries to take the sys/fs
> lock. Deadlock a->b, b->a.
> 
> The problem starts when ipoib_set_mod frees it's rtnl_lck and tries
> to get it after that.
[...]
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -464,8 +464,7 @@ int ipoib_set_mode(struct net_device *de
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂpriv->tx_wr.wr.send_flags &= ~IB_SEND_IP_CSUM;
> Â
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂipoib_flush_paths(dev);
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂrtnl_lock();
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn 0;
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn (!rtnl_trylock()) ? -EBUSY : 0;
> ÂÂÂÂÂÂÂÂ}
> Â
> ÂÂÂÂÂÂÂÂif (!strcmp(buf, "datagram\n")) {
> @@ -474,8 +473,7 @@ int ipoib_set_mode(struct net_device *de
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂdev_set_mtu(dev, min(priv->mcast_mtu, dev->mtu));
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂrtnl_unlock();
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂipoib_flush_paths(dev);
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂrtnl_lock();
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn 0;
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn (!rtnl_trylock()) ? -EBUSY : 0;
> ÂÂÂÂÂÂÂÂ}
> Â
> ÂÂÂÂÂÂÂÂreturn -EINVAL;

Since you didn't change ipoib_changelink() to handle this, that now has
a potential lock imbalance.

Ben.

-- 
Ben Hutchings
Hoare's Law of Large Problems:
ÂÂÂÂÂÂÂÂInside every large problem is a small problem struggling to get
out.

Attachment: signature.asc
Description: This is a digitally signed message part