* [RE-RESEND PATCH 2/4] usb: musb: Fix NULL check on struct musb_request field
2021-01-23 14:24 [RE-RESEND PATCH 1/4] usb: musb: Fix runtime PM race in musb_queue_resume_work Paul Cercueil
@ 2021-01-23 14:25 ` Paul Cercueil
2021-01-23 14:25 ` [RE-RESEND PATCH 3/4] usb: musb: dma: Remove unused variable Paul Cercueil
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2021-01-23 14:25 UTC (permalink / raw)
To: Bin Liu, Greg Kroah-Hartman
Cc: Tony Lindgren, od, linux-mips, linux-usb, linux-kernel,
Paul Cercueil, Maarten ter Huurne
The 'request' variable is a pointer to the 'request' field of the
struct musb_request 'req' pointer. It only worked until now because
the 'request' field is the first one in the musb_request structure, but
as soon as that changes, the check will be invalid.
Fix it preventively by doing the NULL-check on the 'req' pointer
instead.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Suggested-by: Maarten ter Huurne <maarten@treewalker.org>
Acked-by: Tony Lindgren <tony@atomide.com>
---
drivers/usb/musb/musb_gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index f62ffaede1ab..ef374d4dd94a 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -451,7 +451,7 @@ void musb_g_tx(struct musb *musb, u8 epnum)
return;
}
- if (request) {
+ if (req) {
trace_musb_req_tx(req);
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RE-RESEND PATCH 3/4] usb: musb: dma: Remove unused variable
2021-01-23 14:24 [RE-RESEND PATCH 1/4] usb: musb: Fix runtime PM race in musb_queue_resume_work Paul Cercueil
2021-01-23 14:25 ` [RE-RESEND PATCH 2/4] usb: musb: Fix NULL check on struct musb_request field Paul Cercueil
@ 2021-01-23 14:25 ` Paul Cercueil
2021-01-23 14:25 ` [RE-RESEND PATCH 4/4] usb: musb: jz4740: Add missing CR to error strings Paul Cercueil
2021-01-23 16:41 ` [RE-RESEND PATCH 1/4] usb: musb: Fix runtime PM race in musb_queue_resume_work Sergei Shtylyov
3 siblings, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2021-01-23 14:25 UTC (permalink / raw)
To: Bin Liu, Greg Kroah-Hartman
Cc: Tony Lindgren, od, linux-mips, linux-usb, linux-kernel, Paul Cercueil
Remove unused-but-set devctl variable.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/usb/musb/musbhsdma.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
index 0aacfc8be5a1..7acd1635850d 100644
--- a/drivers/usb/musb/musbhsdma.c
+++ b/drivers/usb/musb/musbhsdma.c
@@ -321,8 +321,6 @@ irqreturn_t dma_controller_irq(int irq, void *private_data)
musb_channel->channel.status =
MUSB_DMA_STATUS_BUS_ABORT;
} else {
- u8 devctl;
-
addr = musb_read_hsdma_addr(mbase,
bchannel);
channel->actual_len = addr
@@ -336,8 +334,6 @@ irqreturn_t dma_controller_irq(int irq, void *private_data)
< musb_channel->len) ?
"=> reconfig 0" : "=> complete");
- devctl = musb_readb(mbase, MUSB_DEVCTL);
-
channel->status = MUSB_DMA_STATUS_FREE;
/* completed */
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RE-RESEND PATCH 4/4] usb: musb: jz4740: Add missing CR to error strings
2021-01-23 14:24 [RE-RESEND PATCH 1/4] usb: musb: Fix runtime PM race in musb_queue_resume_work Paul Cercueil
2021-01-23 14:25 ` [RE-RESEND PATCH 2/4] usb: musb: Fix NULL check on struct musb_request field Paul Cercueil
2021-01-23 14:25 ` [RE-RESEND PATCH 3/4] usb: musb: dma: Remove unused variable Paul Cercueil
@ 2021-01-23 14:25 ` Paul Cercueil
2021-01-23 16:41 ` [RE-RESEND PATCH 1/4] usb: musb: Fix runtime PM race in musb_queue_resume_work Sergei Shtylyov
3 siblings, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2021-01-23 14:25 UTC (permalink / raw)
To: Bin Liu, Greg Kroah-Hartman
Cc: Tony Lindgren, od, linux-mips, linux-usb, linux-kernel, Paul Cercueil
If you pass a string that is not terminated with a carriage return to
dev_err(), it will eventually be printed with a carriage return, but
not right away, since the kernel will wait for a pr_cont().
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/usb/musb/jz4740.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/musb/jz4740.c b/drivers/usb/musb/jz4740.c
index c4fe1f4cd17a..5b7d576bf6ee 100644
--- a/drivers/usb/musb/jz4740.c
+++ b/drivers/usb/musb/jz4740.c
@@ -116,13 +116,13 @@ static int jz4740_musb_init(struct musb *musb)
if (IS_ERR(musb->xceiv)) {
err = PTR_ERR(musb->xceiv);
if (err != -EPROBE_DEFER)
- dev_err(dev, "No transceiver configured: %d", err);
+ dev_err(dev, "No transceiver configured: %d\n", err);
return err;
}
glue->role_sw = usb_role_switch_register(dev, &role_sw_desc);
if (IS_ERR(glue->role_sw)) {
- dev_err(dev, "Failed to register USB role switch");
+ dev_err(dev, "Failed to register USB role switch\n");
return PTR_ERR(glue->role_sw);
}
@@ -205,26 +205,26 @@ static int jz4740_probe(struct platform_device *pdev)
pdata = of_device_get_match_data(dev);
if (!pdata) {
- dev_err(dev, "missing platform data");
+ dev_err(dev, "missing platform data\n");
return -EINVAL;
}
musb = platform_device_alloc("musb-hdrc", PLATFORM_DEVID_AUTO);
if (!musb) {
- dev_err(dev, "failed to allocate musb device");
+ dev_err(dev, "failed to allocate musb device\n");
return -ENOMEM;
}
clk = devm_clk_get(dev, "udc");
if (IS_ERR(clk)) {
- dev_err(dev, "failed to get clock");
+ dev_err(dev, "failed to get clock\n");
ret = PTR_ERR(clk);
goto err_platform_device_put;
}
ret = clk_prepare_enable(clk);
if (ret) {
- dev_err(dev, "failed to enable clock");
+ dev_err(dev, "failed to enable clock\n");
goto err_platform_device_put;
}
@@ -240,19 +240,19 @@ static int jz4740_probe(struct platform_device *pdev)
ret = platform_device_add_resources(musb, pdev->resource,
pdev->num_resources);
if (ret) {
- dev_err(dev, "failed to add resources");
+ dev_err(dev, "failed to add resources\n");
goto err_clk_disable;
}
ret = platform_device_add_data(musb, pdata, sizeof(*pdata));
if (ret) {
- dev_err(dev, "failed to add platform_data");
+ dev_err(dev, "failed to add platform_data\n");
goto err_clk_disable;
}
ret = platform_device_add(musb);
if (ret) {
- dev_err(dev, "failed to register musb device");
+ dev_err(dev, "failed to register musb device\n");
goto err_clk_disable;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RE-RESEND PATCH 1/4] usb: musb: Fix runtime PM race in musb_queue_resume_work
2021-01-23 14:24 [RE-RESEND PATCH 1/4] usb: musb: Fix runtime PM race in musb_queue_resume_work Paul Cercueil
` (2 preceding siblings ...)
2021-01-23 14:25 ` [RE-RESEND PATCH 4/4] usb: musb: jz4740: Add missing CR to error strings Paul Cercueil
@ 2021-01-23 16:41 ` Sergei Shtylyov
2021-01-24 8:38 ` Paul Cercueil
3 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2021-01-23 16:41 UTC (permalink / raw)
To: Paul Cercueil, Bin Liu, Greg Kroah-Hartman
Cc: Tony Lindgren, od, linux-mips, linux-usb, linux-kernel, stable
On 1/23/21 5:24 PM, Paul Cercueil wrote:
> musb_queue_resume_work() would call the provided callback if the runtime
> PM status was 'active'. Otherwise, it would enqueue the request if the
> hardware was still suspended (musb->is_runtime_suspended is true).
>
> This causes a race with the runtime PM handlers, as it is possible to be
> in the case where the runtime PM status is not yet 'active', but the
> hardware has been awaken (PM resume function has been called).
Awakened. :-)
> When hitting the race, the resume work was not enqueued, which probably
> triggered other bugs further down the stack. For instance, a telnet
> connection on Ingenic SoCs would result in a 50/50 chance of a
> segmentation fault somewhere in the musb code.
>
> Rework the code so that either we call the callback directly if
> (musb->is_runtime_suspended == 0), or enqueue the query otherwise.
>
> Fixes: ea2f35c01d5e ("usb: musb: Fix sleeping function called from invalid context for hdrc glue")
> Cc: stable@vger.kernel.org # v4.9+
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Reviewed-by: Tony Lindgren <tony@atomide.com>
> Tested-by: Tony Lindgren <tony@atomide.com>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RE-RESEND PATCH 1/4] usb: musb: Fix runtime PM race in musb_queue_resume_work
2021-01-23 16:41 ` [RE-RESEND PATCH 1/4] usb: musb: Fix runtime PM race in musb_queue_resume_work Sergei Shtylyov
@ 2021-01-24 8:38 ` Paul Cercueil
0 siblings, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2021-01-24 8:38 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Bin Liu, Greg Kroah-Hartman, Tony Lindgren, od, linux-mips,
linux-usb, linux-kernel, stable
Hi Sergei,
Le sam. 23 janv. 2021 à 19:41, Sergei Shtylyov
<sergei.shtylyov@gmail.com> a écrit :
> On 1/23/21 5:24 PM, Paul Cercueil wrote:
>
>> musb_queue_resume_work() would call the provided callback if the
>> runtime
>> PM status was 'active'. Otherwise, it would enqueue the request if
>> the
>> hardware was still suspended (musb->is_runtime_suspended is true).
>>
>> This causes a race with the runtime PM handlers, as it is possible
>> to be
>> in the case where the runtime PM status is not yet 'active', but the
>> hardware has been awaken (PM resume function has been called).
>
> Awakened. :-)
Oops. Hopefully Bin or Greg can fix it when merging (if I don't need to
v2, that is to say - feedback welcome).
Cheers,
-Paul
>> When hitting the race, the resume work was not enqueued, which
>> probably
>> triggered other bugs further down the stack. For instance, a telnet
>> connection on Ingenic SoCs would result in a 50/50 chance of a
>> segmentation fault somewhere in the musb code.
>>
>> Rework the code so that either we call the callback directly if
>> (musb->is_runtime_suspended == 0), or enqueue the query otherwise.
>>
>> Fixes: ea2f35c01d5e ("usb: musb: Fix sleeping function called from
>> invalid context for hdrc glue")
>> Cc: stable@vger.kernel.org # v4.9+
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> Reviewed-by: Tony Lindgren <tony@atomide.com>
>> Tested-by: Tony Lindgren <tony@atomide.com>
> [...]
>
>
> MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread