Skip to content

Commit c9287fa

Browse files
mszyprowFelipe Balbi
authored andcommitted
usb: gadget: u_ether: fix unsafe list iteration
list_for_each_entry_safe() is not safe for deleting entries from the list if the spin lock, which protects it, is released and reacquired during the list iteration. Fix this issue by replacing this construction with a simple check if list is empty and removing the first entry in each iteration. This is almost equivalent to a revert of the commit mentioned in the Fixes: tag. This patch fixes following issue: --->8--- Unable to handle kernel NULL pointer dereference at virtual address 00000104 pgd = (ptrval) [00000104] *pgd=00000000 Internal error: Oops: 817 [#1] PREEMPT SMP ARM Modules linked in: CPU: 1 PID: 84 Comm: kworker/1:1 Not tainted 4.20.0-rc2-next-20181114-00009-g8266b35ec404 #1061 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: events eth_work PC is at rx_fill+0x60/0xac LR is at _raw_spin_lock_irqsave+0x50/0x5c pc : [<c065fee0>] lr : [<c0a056b8>] psr: 80000093 sp : ee7fbee8 ip : 00000100 fp : 00000000 r10: 006000c0 r9 : c10b0ab0 r8 : ee7eb5c0 r7 : ee7eb614 r6 : ee7eb5ec r5 : 000000dc r4 : ee12ac00 r3 : ee12ac24 r2 : 00000200 r1 : 60000013 r0 : ee7eb5ec Flags: Nzcv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 6d5dc04a DAC: 00000051 Process kworker/1:1 (pid: 84, stack limit = 0x(ptrval)) Stack: (0xee7fbee8 to 0xee7fc000) ... [<c065fee0>] (rx_fill) from [<c0143b7c>] (process_one_work+0x200/0x738) [<c0143b7c>] (process_one_work) from [<c0144118>] (worker_thread+0x2c/0x4c8) [<c0144118>] (worker_thread) from [<c014a8a4>] (kthread+0x128/0x164) [<c014a8a4>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xee7fbfb0 to 0xee7fbff8) ... ---[ end trace 64480bc835eba7d6 ]--- Fixes: fea14e6 ("usb: gadget: u_ether: use better list accessors") Signed-off-by: Marek Szyprowski <[email protected]> Signed-off-by: Felipe Balbi <[email protected]>
1 parent 069caf5 commit c9287fa

File tree

1 file changed

+6
-5
lines changed

1 file changed

+6
-5
lines changed

drivers/usb/gadget/function/u_ether.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -401,12 +401,12 @@ static int alloc_requests(struct eth_dev *dev, struct gether *link, unsigned n)
401401
static void rx_fill(struct eth_dev *dev, gfp_t gfp_flags)
402402
{
403403
struct usb_request *req;
404-
struct usb_request *tmp;
405404
unsigned long flags;
406405

407406
/* fill unused rxq slots with some skb */
408407
spin_lock_irqsave(&dev->req_lock, flags);
409-
list_for_each_entry_safe(req, tmp, &dev->rx_reqs, list) {
408+
while (!list_empty(&dev->rx_reqs)) {
409+
req = list_first_entry(&dev->rx_reqs, struct usb_request, list);
410410
list_del_init(&req->list);
411411
spin_unlock_irqrestore(&dev->req_lock, flags);
412412

@@ -1125,7 +1125,6 @@ void gether_disconnect(struct gether *link)
11251125
{
11261126
struct eth_dev *dev = link->ioport;
11271127
struct usb_request *req;
1128-
struct usb_request *tmp;
11291128

11301129
WARN_ON(!dev);
11311130
if (!dev)
@@ -1142,7 +1141,8 @@ void gether_disconnect(struct gether *link)
11421141
*/
11431142
usb_ep_disable(link->in_ep);
11441143
spin_lock(&dev->req_lock);
1145-
list_for_each_entry_safe(req, tmp, &dev->tx_reqs, list) {
1144+
while (!list_empty(&dev->tx_reqs)) {
1145+
req = list_first_entry(&dev->tx_reqs, struct usb_request, list);
11461146
list_del(&req->list);
11471147

11481148
spin_unlock(&dev->req_lock);
@@ -1154,7 +1154,8 @@ void gether_disconnect(struct gether *link)
11541154

11551155
usb_ep_disable(link->out_ep);
11561156
spin_lock(&dev->req_lock);
1157-
list_for_each_entry_safe(req, tmp, &dev->rx_reqs, list) {
1157+
while (!list_empty(&dev->rx_reqs)) {
1158+
req = list_first_entry(&dev->rx_reqs, struct usb_request, list);
11581159
list_del(&req->list);
11591160

11601161
spin_unlock(&dev->req_lock);

0 commit comments

Comments
 (0)