Re: [PATCH 06/14] xhci: dbc: lock the minor IDR on registration failure

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 10/9/26 13:52, Greg KH wrote:
> Resending with the proper people in cc...
> 
> On Fri, Oct 09, 2026 at 12:51:41PM +0200, Greg KH wrote:
>> On Fri, Oct 09, 2026 at 10:13:10AM +0000, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>> - [High] usb: xhci: dbc: UAF due to premature IDR exposure and missing TTY hangup on error path
>>> --
>>>
>>> commit 99c18a1b8449e8389ccac896ceadcc979dbd6a57
>>> Author: Sang-Hoon Choi <csh0052@gmail.com>
>>>
>>> xhci: dbc: lock the minor IDR on registration failure
>>>
>>> This commit fixes a race condition during DbC TTY registration failure
>>> by adding mutex protection around the removal of the minor IDR entry.
>>>
>>>> diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
>>>> index 3d51e8d82659d..2249cc16800cc 100644
>>>> --- a/drivers/usb/host/xhci-dbgtty.c
>>>> +++ b/drivers/usb/host/xhci-dbgtty.c
>>>> @@ -535,7 +535,9 @@ static int xhci_dbc_tty_register_device(struct xhci_dbc *dbc)
>>>>   err_free_fifo:
>>>>   	kfifo_free(&port->port.xmit_fifo);
>>>>   err_exit_port:
>>>> +	mutex_lock(&dbc_tty_minors_lock);
>>>>   	idr_remove(&dbc_tty_minors, port->minor);
>>>> +	mutex_unlock(&dbc_tty_minors_lock);
>>>>   err_idr:
>>>>   	xhci_dbc_tty_exit_port(port);
>>>
>>> [Severity: High]
>>> Does this error path leave concurrent TTY sessions pointing to freed memory?
>>>
>>> Earlier in xhci_dbc_tty_register_device(), the port is added to the globally
>>> visible dbc_tty_minors IDR before internal structures are fully allocated:
>>>
>>> 	mutex_lock(&dbc_tty_minors_lock);
>>> 	port->minor = idr_alloc(&dbc_tty_minors, port, 0, 64, GFP_KERNEL);
>>> 	mutex_unlock(&dbc_tty_minors_lock);
>>>
>>> A concurrent open() on the device node routes to dbc_tty_install(), which
>>> retrieves the port and installs the TTY without verifying port->registered:
>>
>> I don't think we need to worry about "concurrent open()" for this as
>> that's not a real use case that I know of and this _should_ be root only
>> accessable, right?
>>
>> Or can normal users open this device node?
>>
No risk of concurrent open as the tty device node is not yet created in this error
path.

This error path is taken if tty_port_register_device() fails, of some other failure
before it

-Mathias