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

[Felipe Balbi <balbi@kernel.org>]

Roger Quadros <rogerq@ti.com> writes:

> On 13/06/18 11:05, Felipe Balbi wrote:
>> Roger Quadros <rogerq@ti.com> writes:
>> 
>>> Hi Johan,
>>>
>>> On 31/05/18 17:45, Johan Hovold wrote:
>>>> 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().

echo auto > /path/to/dwc3/power/control

Granted, that get_sync() would've been better as a pm_runtime_forbid()