Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752399AbdFAQ26 (ORCPT ); Thu, 1 Jun 2017 12:28:58 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:33122 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751997AbdFAPor (ORCPT ); Thu, 1 Jun 2017 11:44:47 -0400 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "Erez Shitrit" , "Doug Ledford" , "Feras Daoud" , "Or Gerlitz" , "Leon Romanovsky" Date: Thu, 01 Jun 2017 16:43:15 +0100 Message-ID: X-Mailer: LinuxStableQueue (scripts by bwh) Subject: [PATCH 3.16 035/212] IB/ipoib: Fix deadlock between rmmod and set_mode In-Reply-To: X-SA-Exim-Connect-IP: 82.70.136.246 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4096 Lines: 110 3.16.44-rc1 review patch. If anyone has any objections, please let me know. ------------------ From: Feras Daoud 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. set_mod: [] ? check_preempt_curr+0x6d/0x90 [] __mutex_lock_slowpath+0x13e/0x180 [] ? __rtnl_unlock+0x15/0x20 [] mutex_lock+0x2b/0x50 [] rtnl_lock+0x15/0x20 [] ipoib_set_mode+0x97/0x160 [ib_ipoib] [] set_mode+0x3b/0x80 [ib_ipoib] [] dev_attr_store+0x20/0x30 [] sysfs_write_file+0xe5/0x170 [] vfs_write+0xb8/0x1a0 [] sys_write+0x51/0x90 [] system_call_fastpath+0x16/0x1b rmmod: [] ? put_dec+0x10c/0x110 [] ? number+0x2ee/0x320 [] schedule_timeout+0x215/0x2e0 [] ? vsnprintf+0x484/0x5f0 [] ? string+0x40/0x100 [] wait_for_common+0x123/0x180 [] ? default_wake_function+0x0/0x20 [] ? ifind_fast+0x5e/0xb0 [] wait_for_completion+0x1d/0x20 [] sysfs_addrm_finish+0x228/0x270 [] sysfs_remove_dir+0xa3/0xf0 [] kobject_del+0x16/0x40 [] device_del+0x184/0x1e0 [] netdev_unregister_kobject+0xab/0xc0 [] rollback_registered+0xae/0x130 [] unregister_netdevice+0x22/0x70 [] unregister_netdev+0x1e/0x30 [] ipoib_remove_one+0xe0/0x120 [ib_ipoib] [] ib_unregister_device+0x4f/0x100 [ib_core] [] mlx4_ib_remove+0x41/0x180 [mlx4_ib] [] mlx4_remove_device+0x71/0x90 [mlx4_core] Fixes: 862096a8bbf8 ("IB/ipoib: Add more rtnl_link_ops callbacks") Cc: Or Gerlitz Signed-off-by: Feras Daoud Signed-off-by: Erez Shitrit Signed-off-by: Leon Romanovsky Signed-off-by: Doug Ledford Signed-off-by: Ben Hutchings --- drivers/infiniband/ulp/ipoib/ipoib_cm.c | 12 +++++++----- drivers/infiniband/ulp/ipoib/ipoib_main.c | 6 ++---- 2 files changed, 9 insertions(+), 9 deletions(-) --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c @@ -1496,12 +1496,14 @@ static ssize_t set_mode(struct device *d ret = ipoib_set_mode(dev, buf); - rtnl_unlock(); + /* The assumption is that the function ipoib_set_mode returned + * with the rtnl held by it, if not the value -EBUSY returned, + * then no need to rtnl_unlock + */ + if (ret != -EBUSY) + rtnl_unlock(); - if (!ret) - return count; - - return ret; + return (!ret || ret == -EBUSY) ? count : ret; } static DEVICE_ATTR(mode, S_IWUSR | S_IRUGO, show_mode, set_mode); --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -243,8 +243,7 @@ int ipoib_set_mode(struct net_device *de priv->tx_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")) { @@ -253,8 +252,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;