| From fac19a5bcbb6afa63667c37576ce0f086ec7413f Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Fri, 24 Jul 2020 10:59:10 +0200 |
| Subject: xen-netfront: fix potential deadlock in xennet_remove() |
| |
| From: Andrea Righi <andrea.righi@canonical.com> |
| |
| [ Upstream commit c2c633106453611be07821f53dff9e93a9d1c3f0 ] |
| |
| There's a potential race in xennet_remove(); this is what the driver is |
| doing upon unregistering a network device: |
| |
| 1. state = read bus state |
| 2. if state is not "Closed": |
| 3. request to set state to "Closing" |
| 4. wait for state to be set to "Closing" |
| 5. request to set state to "Closed" |
| 6. wait for state to be set to "Closed" |
| |
| If the state changes to "Closed" immediately after step 1 we are stuck |
| forever in step 4, because the state will never go back from "Closed" to |
| "Closing". |
| |
| Make sure to check also for state == "Closed" in step 4 to prevent the |
| deadlock. |
| |
| Also add a 5 sec timeout any time we wait for the bus state to change, |
| to avoid getting stuck forever in wait_event(). |
| |
| Signed-off-by: Andrea Righi <andrea.righi@canonical.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/net/xen-netfront.c | 64 +++++++++++++++++++++++++------------- |
| 1 file changed, 42 insertions(+), 22 deletions(-) |
| |
| diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c |
| index 6b4675a9494b2..c8e84276e6397 100644 |
| --- a/drivers/net/xen-netfront.c |
| +++ b/drivers/net/xen-netfront.c |
| @@ -63,6 +63,8 @@ module_param_named(max_queues, xennet_max_queues, uint, 0644); |
| MODULE_PARM_DESC(max_queues, |
| "Maximum number of queues per virtual interface"); |
| |
| +#define XENNET_TIMEOUT (5 * HZ) |
| + |
| static const struct ethtool_ops xennet_ethtool_ops; |
| |
| struct netfront_cb { |
| @@ -1337,12 +1339,15 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev) |
| |
| netif_carrier_off(netdev); |
| |
| - xenbus_switch_state(dev, XenbusStateInitialising); |
| - wait_event(module_wq, |
| - xenbus_read_driver_state(dev->otherend) != |
| - XenbusStateClosed && |
| - xenbus_read_driver_state(dev->otherend) != |
| - XenbusStateUnknown); |
| + do { |
| + xenbus_switch_state(dev, XenbusStateInitialising); |
| + err = wait_event_timeout(module_wq, |
| + xenbus_read_driver_state(dev->otherend) != |
| + XenbusStateClosed && |
| + xenbus_read_driver_state(dev->otherend) != |
| + XenbusStateUnknown, XENNET_TIMEOUT); |
| + } while (!err); |
| + |
| return netdev; |
| |
| exit: |
| @@ -2142,28 +2147,43 @@ static const struct attribute_group xennet_dev_group = { |
| }; |
| #endif /* CONFIG_SYSFS */ |
| |
| -static int xennet_remove(struct xenbus_device *dev) |
| +static void xennet_bus_close(struct xenbus_device *dev) |
| { |
| - struct netfront_info *info = dev_get_drvdata(&dev->dev); |
| - |
| - dev_dbg(&dev->dev, "%s\n", dev->nodename); |
| + int ret; |
| |
| - if (xenbus_read_driver_state(dev->otherend) != XenbusStateClosed) { |
| + if (xenbus_read_driver_state(dev->otherend) == XenbusStateClosed) |
| + return; |
| + do { |
| xenbus_switch_state(dev, XenbusStateClosing); |
| - wait_event(module_wq, |
| - xenbus_read_driver_state(dev->otherend) == |
| - XenbusStateClosing || |
| - xenbus_read_driver_state(dev->otherend) == |
| - XenbusStateUnknown); |
| + ret = wait_event_timeout(module_wq, |
| + xenbus_read_driver_state(dev->otherend) == |
| + XenbusStateClosing || |
| + xenbus_read_driver_state(dev->otherend) == |
| + XenbusStateClosed || |
| + xenbus_read_driver_state(dev->otherend) == |
| + XenbusStateUnknown, |
| + XENNET_TIMEOUT); |
| + } while (!ret); |
| + |
| + if (xenbus_read_driver_state(dev->otherend) == XenbusStateClosed) |
| + return; |
| |
| + do { |
| xenbus_switch_state(dev, XenbusStateClosed); |
| - wait_event(module_wq, |
| - xenbus_read_driver_state(dev->otherend) == |
| - XenbusStateClosed || |
| - xenbus_read_driver_state(dev->otherend) == |
| - XenbusStateUnknown); |
| - } |
| + ret = wait_event_timeout(module_wq, |
| + xenbus_read_driver_state(dev->otherend) == |
| + XenbusStateClosed || |
| + xenbus_read_driver_state(dev->otherend) == |
| + XenbusStateUnknown, |
| + XENNET_TIMEOUT); |
| + } while (!ret); |
| +} |
| + |
| +static int xennet_remove(struct xenbus_device *dev) |
| +{ |
| + struct netfront_info *info = dev_get_drvdata(&dev->dev); |
| |
| + xennet_bus_close(dev); |
| xennet_disconnect_backend(info); |
| |
| if (info->netdev->reg_state == NETREG_REGISTERED) |
| -- |
| 2.25.1 |
| |