Commit c9287fa6 authored by Marek Szyprowski's avatar Marek Szyprowski Committed by Felipe Balbi

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: fea14e68 ("usb: gadget: u_ether: use better list accessors")
Signed-off-by: default avatarMarek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
parent 069caf59
...@@ -401,12 +401,12 @@ static int alloc_requests(struct eth_dev *dev, struct gether *link, unsigned n) ...@@ -401,12 +401,12 @@ static int alloc_requests(struct eth_dev *dev, struct gether *link, unsigned n)
static void rx_fill(struct eth_dev *dev, gfp_t gfp_flags) static void rx_fill(struct eth_dev *dev, gfp_t gfp_flags)
{ {
struct usb_request *req; struct usb_request *req;
struct usb_request *tmp;
unsigned long flags; unsigned long flags;
/* fill unused rxq slots with some skb */ /* fill unused rxq slots with some skb */
spin_lock_irqsave(&dev->req_lock, flags); spin_lock_irqsave(&dev->req_lock, flags);
list_for_each_entry_safe(req, tmp, &dev->rx_reqs, list) { while (!list_empty(&dev->rx_reqs)) {
req = list_first_entry(&dev->rx_reqs, struct usb_request, list);
list_del_init(&req->list); list_del_init(&req->list);
spin_unlock_irqrestore(&dev->req_lock, flags); spin_unlock_irqrestore(&dev->req_lock, flags);
...@@ -1125,7 +1125,6 @@ void gether_disconnect(struct gether *link) ...@@ -1125,7 +1125,6 @@ void gether_disconnect(struct gether *link)
{ {
struct eth_dev *dev = link->ioport; struct eth_dev *dev = link->ioport;
struct usb_request *req; struct usb_request *req;
struct usb_request *tmp;
WARN_ON(!dev); WARN_ON(!dev);
if (!dev) if (!dev)
...@@ -1142,7 +1141,8 @@ void gether_disconnect(struct gether *link) ...@@ -1142,7 +1141,8 @@ void gether_disconnect(struct gether *link)
*/ */
usb_ep_disable(link->in_ep); usb_ep_disable(link->in_ep);
spin_lock(&dev->req_lock); spin_lock(&dev->req_lock);
list_for_each_entry_safe(req, tmp, &dev->tx_reqs, list) { while (!list_empty(&dev->tx_reqs)) {
req = list_first_entry(&dev->tx_reqs, struct usb_request, list);
list_del(&req->list); list_del(&req->list);
spin_unlock(&dev->req_lock); spin_unlock(&dev->req_lock);
...@@ -1154,7 +1154,8 @@ void gether_disconnect(struct gether *link) ...@@ -1154,7 +1154,8 @@ void gether_disconnect(struct gether *link)
usb_ep_disable(link->out_ep); usb_ep_disable(link->out_ep);
spin_lock(&dev->req_lock); spin_lock(&dev->req_lock);
list_for_each_entry_safe(req, tmp, &dev->rx_reqs, list) { while (!list_empty(&dev->rx_reqs)) {
req = list_first_entry(&dev->rx_reqs, struct usb_request, list);
list_del(&req->list); list_del(&req->list);
spin_unlock(&dev->req_lock); spin_unlock(&dev->req_lock);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment