usb: dwc3: of-simple: fix use-after-free on remove

On 13/06/18 11:05, Felipe Balbi wrote:

Roger Quadros [off-list ref] writes:

quoted

Hi Johan,

On 31/05/18 17:45, Johan Hovold wrote:

quoted

The clocks have already been explicitly disabled and put as part of
remove() so the runtime suspend callback must not be run when balancing
the runtime PM usage count before returning.

Fixes: 16adc674d0d6 ("usb: dwc3: add generic OF glue layer")
Signed-off-by: Johan Hovold <johan@kernel.org>
---

Changes in v2
 - balance usage count only after disabling runtime PM to avoid racing
   with pm_runtime_suspend() as suggested by Alan


 drivers/usb/dwc3/dwc3-of-simple.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index cb2ee96fd3e8..048922d549dd 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c

@@ -165,8 +165,9 @@ static int dwc3_of_simple_remove(struct platform_device *pdev)
 
 	reset_control_put(simple->resets);
 
-	pm_runtime_put_sync(dev);

Wasn't this call there to balance out the pm_runtime_get_sync() call in probe()?
The pm_runtime_get_sync() call is still there in probe().

that's now balanced by put_noidle below

OK.

I'm still trying to get my head around this.

in probe() we do
{
	enable all clocks;
        pm_runtime_set_active(dev);
        pm_runtime_enable(dev);
        pm_runtime_get_sync(dev);
}

How will runtime suspend work at all?
We're holding a positive RPM count in probe().

This was pointed out by Johan as well in [1]

[1] https://lkml.org/lkml/2018/5/28/1705