From nobody Fri Apr 19 08:10:36 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail(p=none dis=none) header.from=canonical.com ARC-Seal: i=1; a=rsa-sha256; t=1595581179; cv=none; d=zohomail.com; s=zohoarc; b=PZsUbN0HySuazoxSP7feRKfHd9xpiXOd4zITCc4Q5UcFXAwXK3srkRsp2mGjcp2FiKSaXq6DqsGNYqC2XiXhL4k87+wkWGozdBs1XpySNnbUjqt3g7qwYT/DX6NTrvu8+kswE0Uqn9rHsBfbGqsYwbEKYzmYFlfclVZAdf+EahM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1595581179; h=Content-Type:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=Hjub13SPe2wEgoDjP8LKlCX8Yl7alX0oGHdU3qzYDPM=; b=V2HSoda8d5lRjYBae0oFP6Wc8jhBSvkKJ01P688cLjXuRk8IYnxX+/NLUNI39Bu7KREr6TYRvIN/dZAiQWDJF+zbNEPTydLlCaBkbiyU/52Y85G3wUBpoQ6/YpMybWpPocQcR/ydpAu90bsV48goNY3IX+z27XEdalmKyd2as4k= ARC-Authentication-Results: i=1; mx.zohomail.com; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1595581179978141.6314662869645; Fri, 24 Jul 2020 01:59:39 -0700 (PDT) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jytXx-0004Nq-0G; Fri, 24 Jul 2020 08:59:17 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jytXv-0004Nj-9k for xen-devel@lists.xenproject.org; Fri, 24 Jul 2020 08:59:15 +0000 Received: from youngberry.canonical.com (unknown [91.189.89.112]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id f2665624-cd8b-11ea-87e7-bc764e2007e4; Fri, 24 Jul 2020 08:59:14 +0000 (UTC) Received: from mail-ej1-f72.google.com ([209.85.218.72]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jytXt-0003cC-Ig for xen-devel@lists.xenproject.org; Fri, 24 Jul 2020 08:59:13 +0000 Received: by mail-ej1-f72.google.com with SMTP id q11so3434066eja.3 for ; Fri, 24 Jul 2020 01:59:13 -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) X-Inumbo-ID: f2665624-cd8b-11ea-87e7-bc764e2007e4 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=n/3zDNavFBwyidp5mK9uTIUnZfbHeSKJP4k6Md3Q+MmdkMWQ8FQ2ZbQoZ9SwA3NWvc EqtOXntSZ9x4rmez3Jw1+S/UWUPdrNW3an2O+mEV7odoE4wZzNDZ+X3zGsXQmmSSDML1 HUBGuHZ3Fqz6BZLdd1A6yR6oULhBm5YpLuYv6THiBFs3tEbC56nM9xQSbfZHFkQzeDE9 1CgAPQu5Aj+br1BdM0coAM1wKpocTKt3vQliydYhLgPhghicxYr0ml5+PJVXD7Ra6R+b oEITKCk0MfjzDcbkG4/CQsqTHla7AATJ0QGL0fxnsCRipvNErj59Y9tCgRj7LjHJ5V+B 5RMA== X-Gm-Message-State: AOAM533Jplrd6i3wjxEFqCFSpPFrzyDmGHKm3O5F+UGrSbqV2R4/TW6V TT6zn2/vYZUAZlbukFS4W+OF336yydgi6S0D8iQrD8GmW/yDuwKIAxjJ3bdKF0D56zJkyB+q1oA daE2tgRMNZdk22OWl4WIN+q9UkHsrmHS/IkoEACie2jlV X-Received: by 2002:a17:906:c40d:: with SMTP id u13mr8037436ejz.519.1595581152669; Fri, 24 Jul 2020 01:59:12 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyLghkjxyAHSXRSJUM7yxIeHsXsl1Sx8URoFGpqj60bEA7IlZyIWZIUznr715CpupWlbmdO2A== X-Received: by 2002:a17:906:c40d:: with SMTP id u13mr8037418ejz.519.1595581152278; Fri, 24 Jul 2020 01:59:12 -0700 (PDT) Date: Fri, 24 Jul 2020 10:59:10 +0200 From: Andrea Righi To: Boris Ostrovsky , Juergen Gross , Stefano Stabellini Subject: [PATCH v2] xen-netfront: fix potential deadlock in xennet_remove() Message-ID: <20200724085910.GA1043801@xps-13> MIME-Version: 1.0 Content-Disposition: inline X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Jakub Kicinski , netdev@vger.kernel.org, "David S. Miller" , linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" There's a potential race in xennet_remove(); this is what the driver is doing upon unregistering a network device: 1. state =3D 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 =3D=3D "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, 0= 644); MODULE_PARM_DESC(max_queues, "Maximum number of queues per virtual interface"); =20 +#define XENNET_TIMEOUT (5 * HZ) + static const struct ethtool_ops xennet_ethtool_ops; =20 struct netfront_cb { @@ -1334,12 +1336,15 @@ static struct net_device *xennet_create_dev(struct = xenbus_device *dev) =20 netif_carrier_off(netdev); =20 - xenbus_switch_state(dev, XenbusStateInitialising); - wait_event(module_wq, - xenbus_read_driver_state(dev->otherend) !=3D - XenbusStateClosed && - xenbus_read_driver_state(dev->otherend) !=3D - XenbusStateUnknown); + do { + xenbus_switch_state(dev, XenbusStateInitialising); + err =3D wait_event_timeout(module_wq, + xenbus_read_driver_state(dev->otherend) !=3D + XenbusStateClosed && + xenbus_read_driver_state(dev->otherend) !=3D + XenbusStateUnknown, XENNET_TIMEOUT); + } while (!err); + return netdev; =20 exit: @@ -2139,28 +2144,43 @@ static const struct attribute_group xennet_dev_grou= p =3D { }; #endif /* CONFIG_SYSFS */ =20 -static int xennet_remove(struct xenbus_device *dev) +static void xennet_bus_close(struct xenbus_device *dev) { - struct netfront_info *info =3D dev_get_drvdata(&dev->dev); - - dev_dbg(&dev->dev, "%s\n", dev->nodename); + int ret; =20 - if (xenbus_read_driver_state(dev->otherend) !=3D XenbusStateClosed) { + if (xenbus_read_driver_state(dev->otherend) =3D=3D XenbusStateClosed) + return; + do { xenbus_switch_state(dev, XenbusStateClosing); - wait_event(module_wq, - xenbus_read_driver_state(dev->otherend) =3D=3D - XenbusStateClosing || - xenbus_read_driver_state(dev->otherend) =3D=3D - XenbusStateUnknown); + ret =3D wait_event_timeout(module_wq, + xenbus_read_driver_state(dev->otherend) =3D=3D + XenbusStateClosing || + xenbus_read_driver_state(dev->otherend) =3D=3D + XenbusStateClosed || + xenbus_read_driver_state(dev->otherend) =3D=3D + XenbusStateUnknown, + XENNET_TIMEOUT); + } while (!ret); + + if (xenbus_read_driver_state(dev->otherend) =3D=3D XenbusStateClosed) + return; =20 + do { xenbus_switch_state(dev, XenbusStateClosed); - wait_event(module_wq, - xenbus_read_driver_state(dev->otherend) =3D=3D - XenbusStateClosed || - xenbus_read_driver_state(dev->otherend) =3D=3D - XenbusStateUnknown); - } + ret =3D wait_event_timeout(module_wq, + xenbus_read_driver_state(dev->otherend) =3D=3D + XenbusStateClosed || + xenbus_read_driver_state(dev->otherend) =3D=3D + XenbusStateUnknown, + XENNET_TIMEOUT); + } while (!ret); +} + +static int xennet_remove(struct xenbus_device *dev) +{ + struct netfront_info *info =3D dev_get_drvdata(&dev->dev); =20 + xennet_bus_close(dev); xennet_disconnect_backend(info); =20 if (info->netdev->reg_state =3D=3D NETREG_REGISTERED) --=20 2.25.1