From patchwork Mon Jun 27 14:05:27 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Neukum X-Patchwork-Id: 12896688 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3B85BC43334 for ; Mon, 27 Jun 2022 14:05:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236585AbiF0OFh (ORCPT ); Mon, 27 Jun 2022 10:05:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43788 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236653AbiF0OFg (ORCPT ); Mon, 27 Jun 2022 10:05:36 -0400 Received: from EUR05-AM6-obe.outbound.protection.outlook.com (mail-am6eur05on2067.outbound.protection.outlook.com [40.107.22.67]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 48903120B4 for ; Mon, 27 Jun 2022 07:05:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Q1o5TInqio7ZSHhO/tjebiqWQTXl/sxtjO0cs/Yi6YrjdctQ7rPL6kbFHTDdEMfv5poVwMBqKQuIvphn8NG1ZuEtp7rn3CoPpe7pGH4yau3bdpsFcOG21LPMU/UpWIgVB7M7Q6uaIxGUuTFihJgq8S8i5D/5c+rvFQbkb3HKmEI94X508UpUMFobWfR6dgYyOYwa5se2eSKvpNA+xjM0CNuumpM3BsIQ83fr8hi+97ps2QjnSYeqCzRRQuYD5clWdNZZe0F0qhhm5bT39+HiXSwkT67RqUmoyis/oxqrr54GLYhbrcraGN5rfEh0Pos5jeEZRLU8zNhkaeg0+4ysfg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ood1FBUVkQpCy+rzeSWEHZT4Hubj3preSnjzT3Uy4yY=; b=Vlk+0rYc82DxZuSWZ/2Sc0yVEicKTdP2JHkbC7v9yKuILpUyfMG678QuXj2DlUVkURiK1Wbdf81rMw+7+pxHCvmmlzgwEfqYAiys5vwJf2XyswWmAi+VrZ90TjljCn73uM9bt7t1cyyDCk9A0Cwmb62Fdy9wN53SoULJ22QRynHQWIRQDkqo0GI3DZqOZvrfcLvQTS5non+TBm2zjTXnXVHMWTS3NO46njQj7Z15FCX1ukMrBa57lDVa01UT2hwN2cwQsb2EERha5zb3PgmZdHeGag5t/5pgdNAcpgQwQMWkjlVcKKK/6jRihXh3HWc6ZWXw9RYt081437ktW2d2eQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ood1FBUVkQpCy+rzeSWEHZT4Hubj3preSnjzT3Uy4yY=; b=kwkWjINf6cNEwbR/GWue1CMA4HGNmAS7B99MfMhJsVv8d2eyHPmwqIHFupPnSpUVgzZo460BkD+mTe0KFHI7sdrPVariaJLv5k1NZ0SjeTPTLLmsFyVMIurrO5FjrNNzWSr+enTUY2AVbji3X3u9m2bvgpYUa7i8aAmBa9QsKHeAHpqgC30WGq3SyiPjgJJZlazZUD+9NXTCJgDYkSkoNaHPW8pZ80JDjHuSra1fV4HXytN6QN5Y+MM4AfJN4Hj+wqQPWb4Qli3eN1pk1F/7IhZhY8fvLdyQEZWL+HjV8bROtqTB+ulp6gE6oUf8a8hxz6cVg1W/oSnNSPrwTtOnYQ== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com; Received: from VI1PR0401MB2526.eurprd04.prod.outlook.com (2603:10a6:800:58::16) by DB8PR04MB5835.eurprd04.prod.outlook.com (2603:10a6:10:a6::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5373.18; Mon, 27 Jun 2022 14:05:29 +0000 Received: from VI1PR0401MB2526.eurprd04.prod.outlook.com ([fe80::21d5:a855:6e65:cf5d]) by VI1PR0401MB2526.eurprd04.prod.outlook.com ([fe80::21d5:a855:6e65:cf5d%12]) with mapi id 15.20.5373.018; Mon, 27 Jun 2022 14:05:29 +0000 Message-ID: <24312261-6c84-cd2a-9ab4-c92eb700507f@suse.com> Date: Mon, 27 Jun 2022 16:05:27 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 To: Lukas Wunner Content-Language: en-US Cc: "netdev@vger.kernel.org" From: Oliver Neukum Subject: How to safely disconnect an usbnet device X-ClientProxiedBy: AM5PR0502CA0013.eurprd05.prod.outlook.com (2603:10a6:203:91::23) To VI1PR0401MB2526.eurprd04.prod.outlook.com (2603:10a6:800:58::16) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 95dad8a2-ae65-41cf-1c01-08da584617b2 X-MS-TrafficTypeDiagnostic: DB8PR04MB5835:EE_ X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: vQm7JpVFl9QQrjZ05FN/m7zSb57XSuzbt+snwPcbUD3Mtz6NQKkOO0a3y7sc1F5ryJzCiKYWc7oILIDrfxYi5DpahEhQllp1cS8lgZtbMH2wT2W04iWGSRxj0Xb8grh1jeY1EpyLGoOlt1C39JK/d3cCzev/dHz95SaTpV63f2piklLtjHJVpjAJjIVZ0j3mCiLWmpjgUiG8ozc4okYqTe844NC3ZEx1j0lsoyuFBxru4Up0O90d+UfnPvXBWOajfjXMm4KOmPUHJAZGG8dK/w3vnGWr/tGbB7OokEjqTF74cvChsqWZAi4X2dvIFLy325SQ/KuxAaOUjIwAHm4mXOwU+aMofrcgv8qD4cXgXkU3ib/pR/jnb+F+YYqQOgoZujVLSzoOFDGVc2opjWwpHVAlwSft3ah2KD2o1jlAiRE3X3/QoGKV3Q8E9NXVSnb5bBj0rhC9QzeCLGRRp2E32S8XILh9kE71d+wQ5mgtHE1GAnMsXLY7uVzyDkQg+6hQMqywXtOXnGxp0pjy486QTWdNd2Z6cHg4dLoPmtJmE8pHydtgXmeEMEEAs+ZKYp+BqwgHCnC/hkbuB8R775keo1qdVfcqbpYJ4Dd+hfI4icnTiBP7ClGqWzAx2nL2jtfJl/E/gZYYToi6hCSxbyrYszxlUnBeJVRLTxOfvKRYpuHghmebf3KduhG9OXJWJ0VYkYxERX/Ei2wyNW0DFg4zAmTHK9PoJupy3EZmyscxNmZEHv5dQQDieagHJcNrM1jwerBMVXfyg88GlvRhfPBLb4Wrdi2+2PotvPNopMdhd98EczL/Kn4WXSfwnAWSp8AV X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR0401MB2526.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230016)(366004)(39860400002)(346002)(396003)(136003)(376002)(38100700002)(8676002)(2906002)(186003)(6486002)(66946007)(86362001)(8936002)(36756003)(31686004)(31696002)(316002)(235185007)(33964004)(6916009)(5660300002)(2616005)(478600001)(4326008)(66556008)(41300700001)(6506007)(66476007)(6512007)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?q?7Ak+I788E9rwNwKKiOwZBzYWFqwC?= =?utf-8?q?pdc3+TPxsbU/YOq2LmMXUCg5w5V8UJt2hmSUtAZMKPThZ9tw23pek+v+3sNy4ctlM?= =?utf-8?q?0/nfXcoDf6K9B2teZDYJZD+gqv3GJvaiOc2ognBTEtD4r2g5jkrePuW4ULFHg1S+n?= =?utf-8?q?SeVgJZNMZa+uNoWLnLFQ8ZNk1cR2wv7LaNXbeQ/27A/n7V2TNlKEafapycGHvCEu9?= =?utf-8?q?+8/XLL8UgAVNDlSJNYUxjuPbSY2WA7SDjQgJU8+gh6EMRzyfbG3kor+yXZISaPGde?= =?utf-8?q?5ZOUEaiQcz/ICiHqx8zzDtrBuyb+MOUNHxpD0kC1jcb4KA4Y9iQgB7fa2eA2Jxbtd?= =?utf-8?q?JQ++B0WNItLPQMK9ey+0hPUlAPsMhuStkC9QKxF/UToWgM5njflt/uecOAVWBJvS5?= =?utf-8?q?Bnzha61Kzq8L5Ldv8uUq6jpinUx1iwFCWfkx318sZfkLfPNzOZeoE53BIKsq9H/Rd?= =?utf-8?q?SGgjeYsiXIXg+ItkaO4i4Q2Ma6vyb5j8iEOw23F9ACa+PJmIfIcwIS3eUl7kGj+KN?= =?utf-8?q?7otY6AFtlwLsQeFVo5+1sWxBoKv+CUtKc7vhePcui2auzl6MWVevN/EjLGCvJPrF+?= =?utf-8?q?MziAacZDxdJh6LEGYFDG1/nHm/WiscxDNkwm7Ps94XeXl0p9wwBWkxuTHPCyAO1Hn?= =?utf-8?q?AWn1gqjfqYKcA6qlHk8eEbAeVf9kC60ool3Y6vSh7q0t1zibbJ5l+02N/NzI8O1re?= =?utf-8?q?2iQ9QgoZSo3RR2H6DSqNQOza+ktQp/mnluwP3yuOzFSbRCUQg6CG9eqPhmDzfKDW6?= =?utf-8?q?kWfgLjvWBXL/yE1UasHJYiK3whUjf00+3UW52k+VegqXlMe4A0k3adfjBpJS6QE0b?= =?utf-8?q?SqcnLDB55hHcFaCqs/LUo69sdBdGfAKw0/8RFY7a3x8fJCxWWfghTqvwHRM+oSA8U?= =?utf-8?q?Mb2m7qf4OeVp0UL/HlIHYhqADyiRr7glcPkRQrBfBQLeFVM14SYzicoAOzko2ieFj?= =?utf-8?q?BcfCpjscjAgyq/1XFtey3Fh+INpE5gjZcWjHGh/enHuQW86v1nMBByJcKxfc7PYUq?= =?utf-8?q?k3t4XQVBoIQazq000DJT14OpCC80yshGAPWr2CqzopUw+jhftEc+aK4QotcFJc5sO?= =?utf-8?q?ZG0DoG/PIY+3yBc7bBgBS3XWSYuLsQUdLxXUmmav2mQjQruEypWP0ooQqBHQ1pD6Z?= =?utf-8?q?nO0DzD3Sgr4K6XbHWHUtIO+UGNgyAAiwwbACCCv/XZDqIP04dmiDQ+jUvPHuH64ba?= =?utf-8?q?Abmv+IUZJuuyyESvgpNkP6ZGiY2gDrJwGkea+LgA2of3gI398FHtXs2qDaSUHOlGz?= =?utf-8?q?fRSRjMhhtfJCUPyDdzHHoD/aEPSizaizOzR2O0SQHLPaZk/ezGlP4YJMqcPLaMenp?= =?utf-8?q?WX8rfctjnJfv9tUo10iJ9JC5WutElWiQpMeorGIf/NxjqsqcE3gCJ+QKMM4Vtqe/+?= =?utf-8?q?g19Mc/5aHHwHN4jRaJ1R9sLGGo1s1YjvgnWlOP35Ba3JC15YR2GK4egJ1Q3zbJKlf?= =?utf-8?q?eT305up8+CsIWDXZtPv3wKQz4SMOmj1poLPs38A4mrI/hWKHLdmU5wb57E0GgrUQo?= =?utf-8?q?mlMBnTAY5E9XcdGVerZdBCHS4chDzY4wPQgIsIxwE6aq865EGe1PolH8C2PTp+yfl?= =?utf-8?q?PdtVjwwKc+I?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: 95dad8a2-ae65-41cf-1c01-08da584617b2 X-MS-Exchange-CrossTenant-AuthSource: VI1PR0401MB2526.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jun 2022 14:05:29.6724 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 7zazpKq6dbQP/Kx4h/U4tRNZi5Wqkn3T+LhPjRG+zT1YxkyZpSMyxWhr9AD9g1U/17zSewgkzFKrexim3bvS1g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB8PR04MB5835 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi, looking at the race conditions with respect to works, timers and bottom halves it looks to me like me it needs the following algorithm (in pseudocode): 1. Set a flag 2. Cancel everything 3. Recancel everything while checking for the flag while every time a work is scheduled, a timer armed or something similar. By that logic anything that escapes the first round will be caught in the second round. What do you think of his patch? Regards Oliver From 15c537de5e6e9499b964f8337b22ae183742cbb5 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Mon, 27 Jun 2022 16:03:37 +0200 Subject: [PATCH] usbnet: fix cyclical race on disconnect There is a cyclical dependency that must be broken. Signed-off-by: Oliver Neukum --- drivers/net/usb/usbnet.c | 28 +++++++++++++++++++++++----- include/linux/usb/usbnet.h | 18 ++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 7835890df6d3..fcf1a3b9db12 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -501,7 +501,8 @@ static int rx_submit (struct usbnet *dev, struct urb *urb, gfp_t flags) skb = __netdev_alloc_skb_ip_align(dev->net, size, flags); if (!skb) { netif_dbg(dev, rx_err, dev->net, "no rx skb\n"); - usbnet_defer_kevent (dev, EVENT_RX_MEMORY); + if (!usbnet_going_away(dev)) + usbnet_defer_kevent (dev, EVENT_RX_MEMORY); usb_free_urb (urb); return -ENOMEM; } @@ -518,6 +519,7 @@ static int rx_submit (struct usbnet *dev, struct urb *urb, gfp_t flags) if (netif_running (dev->net) && netif_device_present (dev->net) && + !usbnet_going_away(dev) && test_bit(EVENT_DEV_OPEN, &dev->flags) && !test_bit (EVENT_RX_HALT, &dev->flags) && !test_bit (EVENT_DEV_ASLEEP, &dev->flags)) { @@ -854,6 +856,16 @@ int usbnet_stop (struct net_device *net) del_timer_sync (&dev->delay); tasklet_kill (&dev->bh); cancel_work_sync(&dev->kevent); + + /* + * we have cyclic dependencies. Those calls are needed + * to break a cycle. We cannot fall into the gaps because + * we have a flag + */ + tasklet_kill (&dev->bh); + del_timer_sync (&dev->delay); + cancel_work_sync(&dev->kevent); + if (!pm) usb_autopm_put_interface(dev->intf); @@ -1179,7 +1191,8 @@ usbnet_deferred_kevent (struct work_struct *work) status); } else { clear_bit (EVENT_RX_HALT, &dev->flags); - tasklet_schedule (&dev->bh); + if (!usbnet_going_away(dev)) + tasklet_schedule (&dev->bh); } } @@ -1201,10 +1214,13 @@ usbnet_deferred_kevent (struct work_struct *work) } if (rx_submit (dev, urb, GFP_KERNEL) == -ENOLINK) resched = 0; - usb_autopm_put_interface(dev->intf); fail_lowmem: - if (resched) + usb_autopm_put_interface(dev->intf); + if (resched && !usbnet_going_away(dev)) { + set_bit (EVENT_RX_MEMORY, &dev->flags); + tasklet_schedule (&dev->bh); + } } } @@ -1217,13 +1233,13 @@ usbnet_deferred_kevent (struct work_struct *work) if (status < 0) goto skip_reset; if(info->link_reset && (retval = info->link_reset(dev)) < 0) { - usb_autopm_put_interface(dev->intf); skip_reset: netdev_info(dev->net, "link reset failed (%d) usbnet usb-%s-%s, %s\n", retval, dev->udev->bus->bus_name, dev->udev->devpath, info->description); + usb_autopm_put_interface(dev->intf); } else { usb_autopm_put_interface(dev->intf); } @@ -1560,6 +1576,7 @@ static void usbnet_bh (struct timer_list *t) } else if (netif_running (dev->net) && netif_device_present (dev->net) && netif_carrier_ok(dev->net) && + !usbnet_going_away(dev) && !timer_pending(&dev->delay) && !test_bit(EVENT_RX_PAUSED, &dev->flags) && !test_bit(EVENT_RX_HALT, &dev->flags)) { @@ -1606,6 +1623,7 @@ void usbnet_disconnect (struct usb_interface *intf) usb_set_intfdata(intf, NULL); if (!dev) return; + usbnet_mark_going_away(dev); xdev = interface_to_usbdev (intf); diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h index 1b4d72d5e891..8b568ae65366 100644 --- a/include/linux/usb/usbnet.h +++ b/include/linux/usb/usbnet.h @@ -84,8 +84,26 @@ struct usbnet { # define EVENT_LINK_CHANGE 11 # define EVENT_SET_RX_MODE 12 # define EVENT_NO_IP_ALIGN 13 +/* + * this one is special, as it indicates that the device is going away + * there are cyclic dependencies between tasklet, timer and bh + * that must be broken + */ +# define EVENT_UNPLUG 31 }; +static inline bool usbnet_going_away(struct usbnet *ubn) +{ + smp_mb__before_atomic(); + return test_bit(EVENT_UNPLUG, &ubn->flags); +} + +static inline void usbnet_mark_going_away(struct usbnet *ubn) +{ + set_bit(EVENT_UNPLUG, &ubn->flags); + smp_mb__after_atomic(); +} + static inline struct usb_driver *driver_of(struct usb_interface *intf) { return to_usb_driver(intf->dev.driver); -- 2.35.3