Re: [BUG] bus: mhi: possible deadlock in mhi_pm_disable_transition() and mhi_async_power_up()

[Daniel Thompson]

On Sat, Jan 29, 2022 at 10:56:30AM +0800, Jia-Ju Bai wrote:
> Hello,
> 
> My static analysis tool reports a possible deadlock in the mhi driver in
> Linux 5.10:
> 
> mhi_async_power_up()
>   mutex_lock(&mhi_cntrl->pm_mutex); --> Line 933 (Lock A)
>   wait_event_timeout(mhi_cntrl->state_event, ...) --> Line 985 (Wait X)
>   mutex_unlock(&mhi_cntrl->pm_mutex); --> Line 1040 (Unlock A)
> 
> mhi_pm_disable_transition()
>   mutex_lock(&mhi_cntrl->pm_mutex); --> Line 463 (Lock A)
>   wake_up_all(&mhi_cntrl->state_event); --> Line 474 (Wake X)
>   mutex_unlock(&mhi_cntrl->pm_mutex); --> Line 524 (Unlock A)
>   wake_up_all(&mhi_cntrl->state_event); --> Line 526 (Wake X)
> 
> When mhi_async_power_up() is executed, "Wait X" is performed by holding
> "Lock A". If mhi_pm_disable_transition() is concurrently executed at this
> time, "Wake X" cannot be performed to wake up "Wait X" in
> mhi_async_power_up(), because "Lock A" is already hold by
> mhi_async_power_up(), causing a possible deadlock.
> I find that "Wait X" is performed with a timeout, to relieve the possible
> deadlock; but I think this timeout can cause inefficient execution.
> 
> I am not quite sure whether this possible problem is real and how to fix it
> if it is real.
> Any feedback would be appreciated, thanks :)

Interesting find but I think it would be better to run your tool
against more recent kernels to confirm any problem reports. In this
case the code you mention looks like it was removed in v5.17-rc1
(and should eventually make its way to the stable kernels too).


Daniel.