linux-kernel - Re: [PATCH 4.4 14/36] IB/ipoib: Fix deadlock between rmmod and set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1489717488.2852.54.camel@decadent.org.uk>
Date:   Fri, 17 Mar 2017 02:24:48 +0000
From:   Ben Hutchings <ben@...adent.org.uk>
To:     Feras Daoud <ferasda@...lanox.com>,
        Erez Shitrit <erezsh@...lanox.com>,
        Leon Romanovsky <leon@...nel.org>,
        Doug Ledford <dledford@...hat.com>
Cc:     stable@...r.kernel.org, Or Gerlitz <ogerlitz@...lanox.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4.4 14/36] IB/ipoib: Fix deadlock between rmmod and
 set_mode

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@...lanox.com>
> 
> 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.

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)