Re: [PATCH v2] xhci: dbgtty: fix device unregister

[Jiri Slaby <jirislaby@kernel.org>]

Hmm, CCing TTY MAINTAINERS entry would not hurt.

On 19. 11. 25, 22:29, Łukasz Bartosik wrote:
> From: Łukasz Bartosik <ukaszb@chromium.org>
> 
> When DbC is disconnected then xhci_dbc_tty_unregister_device()
> is called. However if there is any user space process blocked
> on write to DbC terminal device then it will never be signalled
> and thus stay blocked indifinitely.

indefinitely

> This fix adds a tty_vhangup() call in xhci_dbc_tty_unregister_device().
> The tty_vhangup() wakes up any blocked writers and causes subsequent
> write attempts to DbC terminal device to fail.
> 
> Cc: stable@vger.kernel.org
> Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
> Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
> ---
> Changes in v2:
> - Replaced tty_hangup() with tty_vhangup()

Why exactly?

> --- a/drivers/usb/host/xhci-dbgtty.c
> +++ b/drivers/usb/host/xhci-dbgtty.c
> @@ -535,6 +535,12 @@ static void xhci_dbc_tty_unregister_device(struct xhci_dbc *dbc)
>   
>   	if (!port->registered)
>   		return;
> +	/*
> +	 * Hang up the TTY. This wakes up any blocked
> +	 * writers and causes subsequent writes to fail.
> +	 */
> +	tty_vhangup(port->port.tty);

This is wrong IMO:
1) what if there is no tty currently open? Ie. tty is NULL.
2) you schedule a tty hangup work and destroy the port right after:
>   	tty_unregister_device(dbc_tty_driver, port->minor);
>   	xhci_dbc_tty_exit_port(port);
>   	port->registered = false;
You should to elaborate how this is supposed to work?

thanks,
-- 
js
suse labs