summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohn Stultz <john.stultz@linaro.org>2019-02-05 10:24:40 -0800
committerDongwoo Lee <dwoo08.lee@samsung.com>2019-11-26 11:23:22 +0900
commiteaff878115683630b5248a95f29d4fad87f46b53 (patch)
tree2461d03cbfd7ff78a29d621691f4eee8a97b6a82
parenteca04186b50dea816fccd930185e49ef57f63814 (diff)
downloadlinux-4.9-exynos9110-eaff878115683630b5248a95f29d4fad87f46b53.tar.gz
linux-4.9-exynos9110-eaff878115683630b5248a95f29d4fad87f46b53.tar.bz2
linux-4.9-exynos9110-eaff878115683630b5248a95f29d4fad87f46b53.zip
usb: f_fs: Avoid crash due to out-of-scope stack ptr access
[ Upstream commit 54f64d5c983f939901dacc8cfc0983727c5c742e ] 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 <stern@rowland.harvard.edu> Cc: Felipe Balbi <balbi@kernel.org> Cc: Zeng Tao <prime.zeng@hisilicon.com> Cc: Jack Pham <jackp@codeaurora.org> Cc: Thinh Nguyen <thinh.nguyen@synopsys.com> Cc: Chen Yu <chenyu56@huawei.com> Cc: Jerry Zhang <zhangjerry@google.com> Cc: Lars-Peter Clausen <lars@metafoo.de> Cc: Vincent Pelletier <plr.vincent@gmail.com> Cc: Andrzej Pietrasiewicz <andrzej.p@samsung.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Linux USB List <linux-usb@vger.kernel.org> Suggested-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: John Stultz <john.stultz@linaro.org> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Signed-off-by: Sasha Levin <sashal@kernel.org> [dwoo08.lee: cherry-pick linux-4.9.y stable commit 49145924e804 to stablize f_fs] Signed-off-by: Dongwoo Lee <dwoo08.lee@samsung.com> Change-Id: I5a786991ea9ceef96ff6c6b65dc807f842356fd6
-rw-r--r--drivers/usb/gadget/function/f_fs.c1
1 files changed, 1 insertions, 0 deletions
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index a29dfe848c4c..82a7f2a9995f 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1009,6 +1009,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;
}