Re: [PATCH v4] usb: host: xhci-plat: fix possible kernel oops while resuming

[Sergey Shtylyov <s.shtylyov@omp.ru>]

On 9/11/23 3:53 PM, Mathias Nyman wrote:
[...]

>> If this driver enables the xHC clocks while resuming from sleep, it calls
>> clk_prepare_enable() without checking for errors and blithely goes on to
>> read/write the xHC's registers -- which, with the xHC not being clocked,
>> at least on ARM32 usually causes an imprecise external abort exceptions
>> which cause kernel oops.  Currently, the chips for which the driver does
>> the clock dance on suspend/resume seem to be the Broadcom STB SoCs, based
>> on ARM32 CPUs, as it seems...
>>
>> In order to fix this issue, add the result checks for clk_prepare_enable()
>> calls in xhci_plat_resume(), add conditional clk_disable_unprepare() calls
>> on the error path of xhci_plat_resume(); then factor out the common clock
>> disabling code from the suspend() and resume() driver PM methods into a
>> separate function to avoid code duplication.

> Minor nitpick, but not sure a separate function is helpful here.
> It's two lines of code called twice.

   Tried to save on the object code size...

>> Found by Linux Verification Center (linuxtesting.org) with the Svace static
>> analysis tool.
>>
>> Fixes: 8bd954c56197 ("usb: host: xhci-plat: suspend and resume clocks")
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> If I understood correctly this issue hasn't been seen in real life,

   Not yet?

> and this patch only changes how we fail?

   Yes, failing gracefully instead of a kernel oops...

> So I guess this would be more suitable for usb-next than usb-linus.

   Maybe. I'll recast and rebase...

[...]

> Thanks
> Mathias

MBR, Sergey