* [PATCH] usb: host: oxu210hp-hcd: Fix deadlock in oxu_qh_alloc()
@ 2015-01-23 22:53 Alexey Khoroshilov
2015-01-24 15:52 ` Alan Stern
0 siblings, 1 reply; 2+ messages in thread
From: Alexey Khoroshilov @ 2015-01-23 22:53 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Alexey Khoroshilov, Rodolfo Giometti, linux-usb, linux-kernel,
ldv-project
oxu_qh_alloc() acquires oxu->mem_lock spinlock, finds free qh slot
and calls ehci_qtd_alloc() to initialize qh->dummy, but
ehci_qtd_alloc() acquires oxu->mem_lock as well.
That means an unavoidable deadlock in oxu_qh_alloc().
The patch fixes the issue by introduction of unlocked version
of ehci_qtd_alloc().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/usb/host/oxu210hp-hcd.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index 036924e640f5..15d4cf2c35cd 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -573,13 +573,11 @@ static inline void oxu_qtd_free(struct oxu_hcd *oxu, struct ehci_qtd *qtd)
spin_unlock(&oxu->mem_lock);
}
-static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
+static struct ehci_qtd *ehci_qtd_alloc_unlocked(struct oxu_hcd *oxu)
{
int i;
struct ehci_qtd *qtd = NULL;
- spin_lock(&oxu->mem_lock);
-
for (i = 0; i < QTD_NUM; i++)
if (!oxu->qtd_used[i])
break;
@@ -598,6 +596,17 @@ static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
oxu->qtd_used[i] = 1;
}
+ return qtd;
+}
+
+static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
+{
+ struct ehci_qtd *qtd;
+
+ spin_lock(&oxu->mem_lock);
+
+ qtd = ehci_qtd_alloc_unlocked(oxu);
+
spin_unlock(&oxu->mem_lock);
return qtd;
@@ -651,7 +660,7 @@ static struct ehci_qh *oxu_qh_alloc(struct oxu_hcd *oxu)
INIT_LIST_HEAD(&qh->qtd_list);
/* dummy td enables safe urb queuing */
- qh->dummy = ehci_qtd_alloc(oxu);
+ qh->dummy = ehci_qtd_alloc_unlocked(oxu);
if (qh->dummy == NULL) {
oxu_dbg(oxu, "no dummy td\n");
oxu->qh_used[i] = 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] usb: host: oxu210hp-hcd: Fix deadlock in oxu_qh_alloc()
2015-01-23 22:53 [PATCH] usb: host: oxu210hp-hcd: Fix deadlock in oxu_qh_alloc() Alexey Khoroshilov
@ 2015-01-24 15:52 ` Alan Stern
0 siblings, 0 replies; 2+ messages in thread
From: Alan Stern @ 2015-01-24 15:52 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Greg Kroah-Hartman, Rodolfo Giometti, linux-usb, linux-kernel,
ldv-project
On Sat, 24 Jan 2015, Alexey Khoroshilov wrote:
> oxu_qh_alloc() acquires oxu->mem_lock spinlock, finds free qh slot
> and calls ehci_qtd_alloc() to initialize qh->dummy, but
> ehci_qtd_alloc() acquires oxu->mem_lock as well.
> That means an unavoidable deadlock in oxu_qh_alloc().
>
> The patch fixes the issue by introduction of unlocked version
> of ehci_qtd_alloc().
This deadlock would affect anybody the moment they plugged a USB device
into one of these controllers, right?
Which means nobody can possibly be using them under Linux. So wouldn't
it be better to remove the driver entirely?
Alan Stern
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
> drivers/usb/host/oxu210hp-hcd.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
> index 036924e640f5..15d4cf2c35cd 100644
> --- a/drivers/usb/host/oxu210hp-hcd.c
> +++ b/drivers/usb/host/oxu210hp-hcd.c
> @@ -573,13 +573,11 @@ static inline void oxu_qtd_free(struct oxu_hcd *oxu, struct ehci_qtd *qtd)
> spin_unlock(&oxu->mem_lock);
> }
>
> -static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
> +static struct ehci_qtd *ehci_qtd_alloc_unlocked(struct oxu_hcd *oxu)
> {
> int i;
> struct ehci_qtd *qtd = NULL;
>
> - spin_lock(&oxu->mem_lock);
> -
> for (i = 0; i < QTD_NUM; i++)
> if (!oxu->qtd_used[i])
> break;
> @@ -598,6 +596,17 @@ static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
> oxu->qtd_used[i] = 1;
> }
>
> + return qtd;
> +}
> +
> +static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
> +{
> + struct ehci_qtd *qtd;
> +
> + spin_lock(&oxu->mem_lock);
> +
> + qtd = ehci_qtd_alloc_unlocked(oxu);
> +
> spin_unlock(&oxu->mem_lock);
>
> return qtd;
> @@ -651,7 +660,7 @@ static struct ehci_qh *oxu_qh_alloc(struct oxu_hcd *oxu)
> INIT_LIST_HEAD(&qh->qtd_list);
>
> /* dummy td enables safe urb queuing */
> - qh->dummy = ehci_qtd_alloc(oxu);
> + qh->dummy = ehci_qtd_alloc_unlocked(oxu);
> if (qh->dummy == NULL) {
> oxu_dbg(oxu, "no dummy td\n");
> oxu->qh_used[i] = 0;
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-01-24 15:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-23 22:53 [PATCH] usb: host: oxu210hp-hcd: Fix deadlock in oxu_qh_alloc() Alexey Khoroshilov
2015-01-24 15:52 ` Alan Stern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.