Received: by 2002:a25:e74b:0:0:0:0:0 with SMTP id e72csp2007748ybh; Fri, 24 Jul 2020 02:00:13 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzw5pao/GNYDIW6MWw/tu5qonrv1S/DZeolU+0ZbdvZCFfbLqxAi2rBKR6naE4NAkvpNgEw X-Received: by 2002:a50:eac5:: with SMTP id u5mr8203225edp.6.1595581213047; Fri, 24 Jul 2020 02:00:13 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1595581213; cv=none; d=google.com; s=arc-20160816; b=cOj1PJ51us56pvJ53xbjyyVFnEQrJoehb+JiiLqtn4jiSDlWyB/l7PZFhMS0VB3ikL oPguxYgTdKjDv51RMTJeO2o8D9pJe5NdHtcvmpQX3dqsAiI3X2rJ+pc6FSvwlMcOWNzh JvIUUz/G1yEsgnKnzlPSlVjposVAP+rcnZYOrC4ONBf2G0ydBByILR9ZXfyUGgtnE6SM S0dJoDkgh/ueuZOFwQfPvQUwsy6xmsjoHXj9V5hS5UE3Ror9LW+bNOfJdDsA9s80DXBv LYiaXefByZYe7zMChqBRIlBl3mYJ0tzm3nb0TP5Ag8GL3UpuYjXGyMwVBN/VxjloL1c4 2zWQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-disposition:mime-version :message-id:subject:cc:to:from:date; bh=Hjub13SPe2wEgoDjP8LKlCX8Yl7alX0oGHdU3qzYDPM=; b=kyzUS1GC5d2F/VCu7lDKWHYDALBnEQHqafmDYWpvQzJFQt1no+DVwNul+B+/GBSBh3 XmbqTFhgJ+n8fF0CV7K3VrUNGSY9gAo7Vu/e21PJHyYGLIiRvG5sEEKLlnOcyaPt6bAO C7Ci7s+KHjM3uSq4du7boWHPYOuJHQKREeloWeMq9lX4QvviyeF1TBvoXStuaXmPQ5Nx WBiCkWgq29AVuNR6yaAxhuw/HD0wsNCBwYNtMqdqxTxd65s+0qcvsY7i7yU/gAyYNaY8 9i0bEDMHTKp1bfg+NL3KM46yleYSDnoLGNLVHFd9sYHMWmC0iDQMyUGGDSXSfEY3758+ /csA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=canonical.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id o11si176413ejx.693.2020.07.24.01.59.49; Fri, 24 Jul 2020 02:00:13 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=canonical.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726952AbgGXI7Q (ORCPT + 99 others); Fri, 24 Jul 2020 04:59:16 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:57288 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726572AbgGXI7P (ORCPT ); Fri, 24 Jul 2020 04:59:15 -0400 Received: from mail-ej1-f71.google.com ([209.85.218.71]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jytXt-0003cG-LP for linux-kernel@vger.kernel.org; Fri, 24 Jul 2020 08:59:13 +0000 Received: by mail-ej1-f71.google.com with SMTP id z14so1753398eje.19 for ; Fri, 24 Jul 2020 01:59:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition; bh=Hjub13SPe2wEgoDjP8LKlCX8Yl7alX0oGHdU3qzYDPM=; b=jBgCutwikEpD3UHAPtd1CterNSTZQh8a4747chpYJNt7Askl107Plj5NT93ex6zmND rovhAiss88fsWt3AME/16eh/rAg1zfHfgnVbxKr7yayiaiBK9rzlJUYr3EL7/VFMItpD Atb3sw+1bmYa7uwisBSZBT2dmXWnxfHUZndmGPbhkWk4aIzt697BJlVYo8x9hi1G62I4 MhM79+v8SLdRBA10YdDPJ8ViEUrLINcOVGWdv04w+rVYfkotNLgBgAcaAdsKKPHljJ4Y qaXbeJtBPVtobeTCozEysAP3AasI/iMZMqISgLaIW/z42oQDTOCbt8UlMp6Y8Yfmg675 R4Ow== X-Gm-Message-State: AOAM533HQ0uo6v47pGL4B4UvbTzqcIzSw/LdigOY5TNLjVhRNNKlJMZR Tbwnm7u78Xfx+dJ2t+Y6uwhrcBK7slc1F4OKeGIkHv4kYibzenX6uhL0UiqIKPgwUNBNj/eaLQN aHkiNbA2CJQh8s3GJObysIRlmAKV2vmVmGx8I0UG7eA== X-Received: by 2002:a17:906:c40d:: with SMTP id u13mr8037439ejz.519.1595581152670; Fri, 24 Jul 2020 01:59:12 -0700 (PDT) X-Received: by 2002:a17:906:c40d:: with SMTP id u13mr8037418ejz.519.1595581152278; Fri, 24 Jul 2020 01:59:12 -0700 (PDT) Received: from localhost (host-87-11-131-192.retail.telecomitalia.it. [87.11.131.192]) by smtp.gmail.com with ESMTPSA id y22sm302547edl.84.2020.07.24.01.59.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Jul 2020 01:59:11 -0700 (PDT) Date: Fri, 24 Jul 2020 10:59:10 +0200 From: Andrea Righi To: Boris Ostrovsky , Juergen Gross , Stefano Stabellini Cc: "David S. Miller" , Jakub Kicinski , xen-devel@lists.xenproject.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] xen-netfront: fix potential deadlock in xennet_remove() Message-ID: <20200724085910.GA1043801@xps-13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- Changes in v2: - remove all dev_dbg() calls (as suggested by David Miller) 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 482c6c8b0fb7..88280057e032 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 { @@ -1334,12 +1336,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: @@ -2139,28 +2144,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