]>
Commit | Line | Data |
---|---|---|
04fd09d4 SL |
1 | From 0ad9f236a6d4bac2c8c12cbd5710b18646d3eca4 Mon Sep 17 00:00:00 2001 |
2 | From: John Stultz <john.stultz@linaro.org> | |
3 | Date: Tue, 5 Feb 2019 10:24:40 -0800 | |
4 | Subject: usb: f_fs: Avoid crash due to out-of-scope stack ptr access | |
5 | ||
6 | [ Upstream commit 54f64d5c983f939901dacc8cfc0983727c5c742e ] | |
7 | ||
8 | Since the 5.0 merge window opened, I've been seeing frequent | |
9 | crashes on suspend and reboot with the trace: | |
10 | ||
11 | [ 36.911170] Unable to handle kernel paging request at virtual address ffffff801153d660 | |
12 | [ 36.912769] Unable to handle kernel paging request at virtual address ffffff800004b564 | |
13 | ... | |
14 | [ 36.950666] Call trace: | |
15 | [ 36.950670] queued_spin_lock_slowpath+0x1cc/0x2c8 | |
16 | [ 36.950681] _raw_spin_lock_irqsave+0x64/0x78 | |
17 | [ 36.950692] complete+0x28/0x70 | |
18 | [ 36.950703] ffs_epfile_io_complete+0x3c/0x50 | |
19 | [ 36.950713] usb_gadget_giveback_request+0x34/0x108 | |
20 | [ 36.950721] dwc3_gadget_giveback+0x50/0x68 | |
21 | [ 36.950723] dwc3_thread_interrupt+0x358/0x1488 | |
22 | [ 36.950731] irq_thread_fn+0x30/0x88 | |
23 | [ 36.950734] irq_thread+0x114/0x1b0 | |
24 | [ 36.950739] kthread+0x104/0x130 | |
25 | [ 36.950747] ret_from_fork+0x10/0x1c | |
26 | ||
27 | I isolated this down to in ffs_epfile_io(): | |
28 | https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/gadget/function/f_fs.c#n1065 | |
29 | ||
30 | Where the completion done is setup on the stack: | |
31 | DECLARE_COMPLETION_ONSTACK(done); | |
32 | ||
33 | Then later we setup a request and queue it, and wait for it: | |
34 | if (unlikely(wait_for_completion_interruptible(&done))) { | |
35 | /* | |
36 | * To avoid race condition with ffs_epfile_io_complete, | |
37 | * dequeue the request first then check | |
38 | * status. usb_ep_dequeue API should guarantee no race | |
39 | * condition with req->complete callback. | |
40 | */ | |
41 | usb_ep_dequeue(ep->ep, req); | |
42 | interrupted = ep->status < 0; | |
43 | } | |
44 | ||
45 | The problem is, that we end up being interrupted, dequeue the | |
46 | request, and exit. | |
47 | ||
48 | But then the irq triggers and we try calling complete() on the | |
49 | context pointer which points to now random stack space, which | |
50 | results in the panic. | |
51 | ||
52 | Alan Stern pointed out there is a bug here, in that the snippet | |
53 | above "assumes that usb_ep_dequeue() waits until the request has | |
54 | been completed." And that: | |
55 | ||
56 | wait_for_completion(&done); | |
57 | ||
58 | Is needed right after the usb_ep_dequeue(). | |
59 | ||
60 | Thus this patch implements that change. With it I no longer see | |
61 | the crashes on suspend or reboot. | |
62 | ||
63 | This issue seems to have been uncovered by behavioral changes in | |
64 | the dwc3 driver in commit fec9095bdef4e ("usb: dwc3: gadget: | |
65 | remove wait_end_transfer"). | |
66 | ||
67 | Cc: Alan Stern <stern@rowland.harvard.edu> | |
68 | Cc: Felipe Balbi <balbi@kernel.org> | |
69 | Cc: Zeng Tao <prime.zeng@hisilicon.com> | |
70 | Cc: Jack Pham <jackp@codeaurora.org> | |
71 | Cc: Thinh Nguyen <thinh.nguyen@synopsys.com> | |
72 | Cc: Chen Yu <chenyu56@huawei.com> | |
73 | Cc: Jerry Zhang <zhangjerry@google.com> | |
74 | Cc: Lars-Peter Clausen <lars@metafoo.de> | |
75 | Cc: Vincent Pelletier <plr.vincent@gmail.com> | |
76 | Cc: Andrzej Pietrasiewicz <andrzej.p@samsung.com> | |
77 | Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
78 | Cc: Linux USB List <linux-usb@vger.kernel.org> | |
79 | Suggested-by: Alan Stern <stern@rowland.harvard.edu> | |
80 | Signed-off-by: John Stultz <john.stultz@linaro.org> | |
81 | Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> | |
82 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
83 | --- | |
84 | drivers/usb/gadget/function/f_fs.c | 1 + | |
85 | 1 file changed, 1 insertion(+) | |
86 | ||
87 | diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c | |
88 | index 52e6897fa35a..79900c0b4f3a 100644 | |
89 | --- a/drivers/usb/gadget/function/f_fs.c | |
90 | +++ b/drivers/usb/gadget/function/f_fs.c | |
91 | @@ -1009,6 +1009,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) | |
92 | * condition with req->complete callback. | |
93 | */ | |
94 | usb_ep_dequeue(ep->ep, req); | |
95 | + wait_for_completion(&done); | |
96 | interrupted = ep->status < 0; | |
97 | } | |
98 | ||
99 | -- | |
100 | 2.19.1 | |
101 |