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

[sashiko-bot@kernel.org]

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:

	port = idr_find(&dbc_tty_minors, tty->index);
	...
	if (!port)
		return -ENXIO;

	tty->driver_data = port;

	return tty_port_install(&port->port, driver, tty);

If xhci_dbc_tty_register_device() fails later (for example, during
xhci_dbc_alloc_requests()), the error path frees the kfifo and destroys the
tty_port.

Because tty_port_tty_vhangup() is not called here to terminate the
concurrent TTY session, does this allow userspace to subsequently call
write() and access the freed xmit_fifo and destroyed tty_port?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009095834.561578-1-mathias.nyman@linux.intel.com?part=6