linux-kernel - Re: [BUG] usb: typec: ucsi: possible deadlock in ucsi_pr_swap() and ucsi_handle_connector

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <43d6c3b5-17e6-63a8-21fa-3ff9f478ada7@gmail.com>
Date:   Thu, 10 Feb 2022 10:24:42 +0800
From:   Jia-Ju Bai <baijiaju1990@...il.com>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Greg KH <gregkh@...uxfoundation.org>, kyletso@...gle.com,
        jackp@...eaurora.org, andy.shevchenko@...il.com,
        unixbhaskar@...il.com, subbaram@...eaurora.org,
        mrana@...eaurora.org,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [BUG] usb: typec: ucsi: possible deadlock in ucsi_pr_swap() and
 ucsi_handle_connector_change()



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