Re: [BUG] usb: typec: ucsi: possible deadlock in ucsi_pr_swap() and ucsi_handle_connector_change()

[Jia-Ju Bai]

On 2022/2/9 22:30, Heikki Krogerus wrote:
> On Wed, Feb 09, 2022 at 11:50:57AM +0800, Jia-Ju Bai wrote:
> > Hello,
> >
> > My static analysis tool reports a possible deadlock in the ucsi driver in
> > Linux 5.16:
> >
> > ucsi_pr_swap()
> >    mutex_lock(&con->lock); --> Line 962 (Lock A)
> >    wait_for_completion_timeout(&con->complete, ...) --> Line 981 (Wait X)
> >
> > ucsi_handle_connector_change()
> >    mutex_lock(&con->lock); --> Line 763 (Lock A)
> >    complete(&con->complete); --> Line 782 (Wake X)
> >    complete(&con->complete); --> Line 807 (Wake X)
> >
> > When ucsi_pr_swap() is executed, "Wait X" is performed by holding "Lock A".
> > If ucsi_handle_connector_change() is executed at this time, "Wake X" cannot
> > be performed to wake up "Wait X" in ucsi_handle_connector_change(), because
> > "Lock A" has been already held by ucsi_handle_connector_change(), 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.
> > Any feedback would be appreciated, thanks :)
> This is probable a regression from commit ad74b8649bea ("usb: typec:
> ucsi: Preliminary support for alternate modes"). Can you test does
> this patch fix the issue (attached)?

Hi Heikki,

Thanks for the reply and patch.
After the patch is used, my tool does not report this deadlock in the 
ucsi driver.
Thus, I think this patch should be okay to fix the deadlock :)


Best wishes,
Jia-Ju Bai