From patchwork Tue May 21 13:18:40 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kloetzke Jan X-Patchwork-Id: 10953673 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E5D0D17D2 for ; Tue, 21 May 2019 13:19:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D470820174 for ; Tue, 21 May 2019 13:19:02 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BE18528942; Tue, 21 May 2019 13:19:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1BFF120174 for ; Tue, 21 May 2019 13:19:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728244AbfEUNTA (ORCPT ); Tue, 21 May 2019 09:19:00 -0400 Received: from mail01.preh.com ([80.149.130.22]:42938 "EHLO mail01.preh.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726692AbfEUNS7 (ORCPT ); Tue, 21 May 2019 09:18:59 -0400 From: Kloetzke Jan To: Oliver Neukum , David Miller CC: =?utf-8?q?Jan_Kl=C3=B6tzke?= , "netdev@vger.kernel.org" , "linux-usb@vger.kernel.org" , Kloetzke Jan Subject: [PATCH v3] usbnet: fix kernel crash after disconnect Thread-Topic: [PATCH v3] usbnet: fix kernel crash after disconnect Thread-Index: AQHVD9e0yfwldPJKrkOozCggfUiRSQ== Date: Tue, 21 May 2019 13:18:40 +0000 Message-ID: <20190521131826.30475-1-Jan.Kloetzke@preh.de> References: <1558438944.12672.13.camel@suse.com> In-Reply-To: <1558438944.12672.13.camel@suse.com> Accept-Language: de-DE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-tm-snts-smtp: 0057CAAE3B5C9ED69887D59E986334810166454D7D941CDCF57AE75E0021741E2000:8 x-exclaimer-md-config: 142fe46c-4d13-4ac1-9970-1f36f118897a Content-ID: <7AD7AA868E63D944A5F07209F4FA09B3@preh.de> MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; d=preh.de; s=key1; c=relaxed/relaxed; h=from:to:cc:subject:date:message-id:references:content-type:mime-version; bh=1jmU4MUGKrogGk+Rb1I7p4KEjxRh1MCE6RhCh5YD+AE=; b=oGxj932i+4q9UjJ+ntwTPXCb3OmPnml8cTWwz1aAeK2QbO6XHlUsIninKjo35XbrCH6bYweeILxD cz5W4UCRG8nERMxbzRaD+7CxoLJHfLn7LAlg5bFi0MogzdtTwlhDb7cKqfPJm46/ahlqlLp7p1vO rp1li6d6Q2EFQjG2EeWe5R+Ts0dwRbYUS4D2zznPV0nnj1k7OlTRs+nqgd1NKx49T4KuOT46TApZ oITopd5xIieL9lvgqQ23fr4yZBzLpJDR/LKXgnbuzW0PlOYEjiag4/mv/u+jCyjNOoFv9C8Yk1D/ GpsrZR2XkDJh2sZVRORhhweGg7QtiaEMLdDn0g== Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When disconnecting cdc_ncm the kernel sporadically crashes shortly after the disconnect: [ 57.868812] Unable to handle kernel NULL pointer dereference at virtual address 00000000 ... [ 58.006653] PC is at 0x0 [ 58.009202] LR is at call_timer_fn+0xec/0x1b4 [ 58.013567] pc : [<0000000000000000>] lr : [] pstate: 00000145 [ 58.020976] sp : ffffff8008003da0 [ 58.024295] x29: ffffff8008003da0 x28: 0000000000000001 [ 58.029618] x27: 000000000000000a x26: 0000000000000100 [ 58.034941] x25: 0000000000000000 x24: ffffff8008003e68 [ 58.040263] x23: 0000000000000000 x22: 0000000000000000 [ 58.045587] x21: 0000000000000000 x20: ffffffc68fac1808 [ 58.050910] x19: 0000000000000100 x18: 0000000000000000 [ 58.056232] x17: 0000007f885aff8c x16: 0000007f883a9f10 [ 58.061556] x15: 0000000000000001 x14: 000000000000006e [ 58.066878] x13: 0000000000000000 x12: 00000000000000ba [ 58.072201] x11: ffffffc69ff1db30 x10: 0000000000000020 [ 58.077524] x9 : 8000100008001000 x8 : 0000000000000001 [ 58.082847] x7 : 0000000000000800 x6 : ffffff8008003e70 [ 58.088169] x5 : ffffffc69ff17a28 x4 : 00000000ffff138b [ 58.093492] x3 : 0000000000000000 x2 : 0000000000000000 [ 58.098814] x1 : 0000000000000000 x0 : 0000000000000000 ... [ 58.205800] [< (null)>] (null) [ 58.210521] [] expire_timers+0xa0/0x14c [ 58.215937] [] run_timer_softirq+0xe8/0x128 [ 58.221702] [] __do_softirq+0x298/0x348 [ 58.227118] [] irq_exit+0x74/0xbc [ 58.232009] [] __handle_domain_irq+0x78/0xac [ 58.237857] [] gic_handle_irq+0x80/0xac ... The crash happens roughly 125..130ms after the disconnect. This correlates with the 'delay' timer that is started on certain USB tx/rx errors in the URB completion handler. The problem is a race of usbnet_stop() with usbnet_start_xmit(). In usbnet_stop() we call usbnet_terminate_urbs() to cancel all URBs in flight. This only makes sense if no new URBs are submitted concurrently, though. But the usbnet_start_xmit() can run at the same time on another CPU which almost unconditionally submits an URB. The error callback of the new URB will then schedule the timer after it was already stopped. The fix adds a check if the tx queue is stopped after the tx list lock has been taken. This should reliably prevent the submission of new URBs while usbnet_terminate_urbs() does its job. The same thing is done on the rx side even though it might be safe due to other flags that are checked there. Signed-off-by: Jan Klötzke --- v3: removed WARN_ON() because the race is expected to happen v2: add WARN_ON() as of Olivers suggestion drivers/net/usb/usbnet.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 504282af27e5..921cc0571bd0 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -506,6 +506,7 @@ static int rx_submit (struct usbnet *dev, struct urb *urb, gfp_t flags) if (netif_running (dev->net) && netif_device_present (dev->net) && + test_bit(EVENT_DEV_OPEN, &dev->flags) && !test_bit (EVENT_RX_HALT, &dev->flags) && !test_bit (EVENT_DEV_ASLEEP, &dev->flags)) { switch (retval = usb_submit_urb (urb, GFP_ATOMIC)) { @@ -1431,6 +1432,11 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb, spin_unlock_irqrestore(&dev->txq.lock, flags); goto drop; } + if (netif_queue_stopped(net)) { + usb_autopm_put_interface_async(dev->intf); + spin_unlock_irqrestore(&dev->txq.lock, flags); + goto drop; + } #ifdef CONFIG_PM /* if this triggers the device is still a sleep */