diff options
author | Erez Shitrit <erezsh@mellanox.com> | 2016-06-04 08:15:20 -0400 |
---|---|---|
committer | Doug Ledford <dledford@redhat.com> | 2016-06-07 09:50:53 -0400 |
commit | 198b12f77084244d310888dd5d643083cb5c2aa1 (patch) | |
tree | df3556483d4afdedfd1e92efcbfa1ed3b3a759af | |
parent | d7012467a95b767b4d3beb2e027aa24a83f12f0f (diff) |
IB/IPoIB: Fix race between ipoib_remove_one to sysfs functions
In ipoib_remove_one the driver holds the rtnl_lock and tries to do some
operation like dev_change_flags or unregister_netdev, while sysfs
callback like ipoib_vlan_delete holds sysfs mutex and tries to hold the
rtnl_lock via rtnl_trylock() and restart_syscall() if the lock is not
free, meanwhile ipoib_remove_one tries to get the sysfs lock in order to
free its sysfs directory, and we will get a->b, b->a deadlock.
Trace like the following:
schedule+0x37/0x80
schedule_preempt_disabled+0xe/0x10
__mutex_lock_slowpath+0xb5/0x120
mutex_lock+0x23/0x40
rtnl_lock+0x15/0x20
netdev_run_todo+0x17c/0x320
rtnl_unlock+0xe/0x10
ipoib_vlan_delete+0x11b/0x1b0 [ib_ipoib]
delete_child+0x54/0x80 [ib_ipoib]
dev_attr_store+0x18/0x30
sysfs_kf_write+0x37/0x40
mutex_lock+0x16/0x40
SyS_write+0x55/0xc0
entry_SYSCALL_64_fastpath+0x16/0x75
And
schedule+0x37/0x80
__kernfs_remove+0x1a8/0x260
? wake_atomic_t_function+0x60/0x60
kernfs_remove+0x25/0x40
sysfs_remove_dir+0x50/0x80
kobject_del+0x18/0x50
device_del+0x19f/0x260
netdev_unregister_kobject+0x6a/0x80
rollback_registered_many+0x1fd/0x340
rollback_registered+0x3c/0x70
unregister_netdevice_queue+0x55/0xc0
unregister_netdev+0x20/0x30
ipoib_remove_one+0x114/0x1b0 [ib_ipoib]
ib_unregister_client+0x4a/0x170 [ib_core]
? find_module_all+0x71/0xa0
ipoib_cleanup_module+0x10/0x94 [ib_ipoib]
SyS_delete_module+0x1b5/0x210
entry_SYSCALL_64_fastpath+0x16/0x75
The fix is by checking the flag IPOIB_FLAG_INTF_ON_DESTROY in order to
get out from the sysfs function.
Fixes: 862096a8bbf8 ("IB/ipoib: Add more rtnl_link_ops callbacks")
Fixes: 9baa0b036410 ("IB/ipoib: Add rtnl_link_ops support")
Signed-off-by: Erez Shitrit <erezsh@mellanox.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
Signed-off-by: Doug Ledford <dledford@redhat.com>
-rw-r--r-- | drivers/infiniband/ulp/ipoib/ipoib.h | 1 | ||||
-rw-r--r-- | drivers/infiniband/ulp/ipoib/ipoib_cm.c | 4 | ||||
-rw-r--r-- | drivers/infiniband/ulp/ipoib/ipoib_main.c | 3 | ||||
-rw-r--r-- | drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 6 |
4 files changed, 14 insertions, 0 deletions
diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h index bab7db6fa9ab..4f7d9b48df64 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib.h +++ b/drivers/infiniband/ulp/ipoib/ipoib.h | |||
@@ -94,6 +94,7 @@ enum { | |||
94 | IPOIB_NEIGH_TBL_FLUSH = 12, | 94 | IPOIB_NEIGH_TBL_FLUSH = 12, |
95 | IPOIB_FLAG_DEV_ADDR_SET = 13, | 95 | IPOIB_FLAG_DEV_ADDR_SET = 13, |
96 | IPOIB_FLAG_DEV_ADDR_CTRL = 14, | 96 | IPOIB_FLAG_DEV_ADDR_CTRL = 14, |
97 | IPOIB_FLAG_GOING_DOWN = 15, | ||
97 | 98 | ||
98 | IPOIB_MAX_BACKOFF_SECONDS = 16, | 99 | IPOIB_MAX_BACKOFF_SECONDS = 16, |
99 | 100 | ||
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c index b2f42835d76d..951d9abcca8b 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c | |||
@@ -1486,6 +1486,10 @@ static ssize_t set_mode(struct device *d, struct device_attribute *attr, | |||
1486 | { | 1486 | { |
1487 | struct net_device *dev = to_net_dev(d); | 1487 | struct net_device *dev = to_net_dev(d); |
1488 | int ret; | 1488 | int ret; |
1489 | struct ipoib_dev_priv *priv = netdev_priv(dev); | ||
1490 | |||
1491 | if (test_bit(IPOIB_FLAG_GOING_DOWN, &priv->flags)) | ||
1492 | return -EPERM; | ||
1489 | 1493 | ||
1490 | if (!rtnl_trylock()) | 1494 | if (!rtnl_trylock()) |
1491 | return restart_syscall(); | 1495 | return restart_syscall(); |
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 2d7c16346648..0aa52c2f9438 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c | |||
@@ -2141,6 +2141,9 @@ static void ipoib_remove_one(struct ib_device *device, void *client_data) | |||
2141 | ib_unregister_event_handler(&priv->event_handler); | 2141 | ib_unregister_event_handler(&priv->event_handler); |
2142 | flush_workqueue(ipoib_workqueue); | 2142 | flush_workqueue(ipoib_workqueue); |
2143 | 2143 | ||
2144 | /* mark interface in the middle of destruction */ | ||
2145 | set_bit(IPOIB_FLAG_GOING_DOWN, &priv->flags); | ||
2146 | |||
2144 | rtnl_lock(); | 2147 | rtnl_lock(); |
2145 | dev_change_flags(priv->dev, priv->dev->flags & ~IFF_UP); | 2148 | dev_change_flags(priv->dev, priv->dev->flags & ~IFF_UP); |
2146 | rtnl_unlock(); | 2149 | rtnl_unlock(); |
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c index 64a35595eab8..a2f9f29c6ab5 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c | |||
@@ -131,6 +131,9 @@ int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey) | |||
131 | 131 | ||
132 | ppriv = netdev_priv(pdev); | 132 | ppriv = netdev_priv(pdev); |
133 | 133 | ||
134 | if (test_bit(IPOIB_FLAG_GOING_DOWN, &ppriv->flags)) | ||
135 | return -EPERM; | ||
136 | |||
134 | snprintf(intf_name, sizeof intf_name, "%s.%04x", | 137 | snprintf(intf_name, sizeof intf_name, "%s.%04x", |
135 | ppriv->dev->name, pkey); | 138 | ppriv->dev->name, pkey); |
136 | priv = ipoib_intf_alloc(intf_name); | 139 | priv = ipoib_intf_alloc(intf_name); |
@@ -183,6 +186,9 @@ int ipoib_vlan_delete(struct net_device *pdev, unsigned short pkey) | |||
183 | 186 | ||
184 | ppriv = netdev_priv(pdev); | 187 | ppriv = netdev_priv(pdev); |
185 | 188 | ||
189 | if (test_bit(IPOIB_FLAG_GOING_DOWN, &ppriv->flags)) | ||
190 | return -EPERM; | ||
191 | |||
186 | if (!rtnl_trylock()) | 192 | if (!rtnl_trylock()) |
187 | return restart_syscall(); | 193 | return restart_syscall(); |
188 | 194 | ||