From patchwork Fri Feb 15 08:13:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Kroah-Hartman X-Patchwork-Id: 10814297 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 C6FE917E9 for ; Fri, 15 Feb 2019 08:20:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 874FD2EA55 for ; Fri, 15 Feb 2019 08:20:10 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7B68A2EA64; Fri, 15 Feb 2019 08:20:10 +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=-7.7 required=2.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,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 092582EA55 for ; Fri, 15 Feb 2019 08:20:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390676AbfBOIUI (ORCPT ); Fri, 15 Feb 2019 03:20:08 -0500 Received: from mail.kernel.org ([198.145.29.99]:35786 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388144AbfBOIUI (ORCPT ); Fri, 15 Feb 2019 03:20:08 -0500 Received: from localhost (5356596B.cm-6-7b.dynamic.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id EC805204EC; Fri, 15 Feb 2019 08:20:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1550218803; bh=ZDSmblI/Gjf+qsKEmGDe/kr6HfOVtkJvS+C0/vN8zNk=; h=Subject:To:From:Date:From; b=w9icNfV7rMfecmY/d6FITrtI51mhNs5MT+LwPQfHQlE4eg1/bdxEPB6ksGe9Le1lx tTO2nHiuo1a48ingk4TJsiO95VWEGY5kABVo3C2ct+y9BKzHqhwITKMSU0OwKCf3kC ZG2xZ6cHxbct4HZt4EzWP5Wuhd/xfFiAi8+uCKnk= Subject: patch "usb: f_fs: Avoid crash due to out-of-scope stack ptr access" added to usb-testing To: john.stultz@linaro.org, andrzej.p@samsung.com, balbi@kernel.org, chenyu56@huawei.com, felipe.balbi@linux.intel.com, gregkh@linuxfoundation.org, jackp@codeaurora.org, lars@metafoo.de, linux-usb@vger.kernel.org, plr.vincent@gmail.com, prime.zeng@hisilicon.com, stern@rowland.harvard.edu, thinh.nguyen@synopsys.com, zhangjerry@google.com From: Date: Fri, 15 Feb 2019 09:13:13 +0100 Message-ID: <155021839320114@kroah.com> MIME-Version: 1.0 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 This is a note to let you know that I've just added the patch titled usb: f_fs: Avoid crash due to out-of-scope stack ptr access to my usb git tree which can be found at git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git in the usb-testing branch. The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.) The patch will be merged to the usb-next branch sometime soon, after it passes testing, and the merge window is open. If you have any questions about this process, please let me know. From 54f64d5c983f939901dacc8cfc0983727c5c742e Mon Sep 17 00:00:00 2001 From: John Stultz Date: Tue, 5 Feb 2019 10:24:40 -0800 Subject: usb: f_fs: Avoid crash due to out-of-scope stack ptr access Since the 5.0 merge window opened, I've been seeing frequent crashes on suspend and reboot with the trace: [ 36.911170] Unable to handle kernel paging request at virtual address ffffff801153d660 [ 36.912769] Unable to handle kernel paging request at virtual address ffffff800004b564 ... [ 36.950666] Call trace: [ 36.950670] queued_spin_lock_slowpath+0x1cc/0x2c8 [ 36.950681] _raw_spin_lock_irqsave+0x64/0x78 [ 36.950692] complete+0x28/0x70 [ 36.950703] ffs_epfile_io_complete+0x3c/0x50 [ 36.950713] usb_gadget_giveback_request+0x34/0x108 [ 36.950721] dwc3_gadget_giveback+0x50/0x68 [ 36.950723] dwc3_thread_interrupt+0x358/0x1488 [ 36.950731] irq_thread_fn+0x30/0x88 [ 36.950734] irq_thread+0x114/0x1b0 [ 36.950739] kthread+0x104/0x130 [ 36.950747] ret_from_fork+0x10/0x1c I isolated this down to in ffs_epfile_io(): https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/gadget/function/f_fs.c#n1065 Where the completion done is setup on the stack: DECLARE_COMPLETION_ONSTACK(done); Then later we setup a request and queue it, and wait for it: if (unlikely(wait_for_completion_interruptible(&done))) { /* * To avoid race condition with ffs_epfile_io_complete, * dequeue the request first then check * status. usb_ep_dequeue API should guarantee no race * condition with req->complete callback. */ usb_ep_dequeue(ep->ep, req); interrupted = ep->status < 0; } The problem is, that we end up being interrupted, dequeue the request, and exit. But then the irq triggers and we try calling complete() on the context pointer which points to now random stack space, which results in the panic. Alan Stern pointed out there is a bug here, in that the snippet above "assumes that usb_ep_dequeue() waits until the request has been completed." And that: wait_for_completion(&done); Is needed right after the usb_ep_dequeue(). Thus this patch implements that change. With it I no longer see the crashes on suspend or reboot. This issue seems to have been uncovered by behavioral changes in the dwc3 driver in commit fec9095bdef4e ("usb: dwc3: gadget: remove wait_end_transfer"). Cc: Alan Stern Cc: Felipe Balbi Cc: Zeng Tao Cc: Jack Pham Cc: Thinh Nguyen Cc: Chen Yu Cc: Jerry Zhang Cc: Lars-Peter Clausen Cc: Vincent Pelletier Cc: Andrzej Pietrasiewicz Cc: Greg Kroah-Hartman Cc: Linux USB List Suggested-by: Alan Stern Signed-off-by: John Stultz Signed-off-by: Felipe Balbi --- drivers/usb/gadget/function/f_fs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 66a2c165b4b7..20413c276c61 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -1082,6 +1082,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) * condition with req->complete callback. */ usb_ep_dequeue(ep->ep, req); + wait_for_completion(&done); interrupted = ep->status < 0; }