[PATCH v2] xhci: dbgtty: fix device unregister

[PATCH v2] xhci: dbgtty: fix device unregister

[Łukasz Bartosik]

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.

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()

---
 drivers/usb/host/xhci-dbgtty.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
index d894081d8d15..ad86f315c26d 100644
--- 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);
+
 	tty_unregister_device(dbc_tty_driver, port->minor);
 	xhci_dbc_tty_exit_port(port);
 	port->registered = false;
-- 
2.52.0.rc1.455.g30608eb744-goog

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

[Jiri Slaby]

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

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

[Mathias Nyman]

On 11/24/25 08:42, Jiri Slaby wrote:
> 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?

I recommended using tty_vhangup(), well actually tty_port_tty_vhangup() to solve
issue '2' you pointed out.
To me it looks like tty_vhangup() is synchronous so it won't schedule hangup work
and should be safe to call right before we destroy the port.

> 
>> --- 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.

v1 had the NULL check, I stated that tty_port_tty_vhangup() does the check for us
so no need for it here.

https://lore.kernel.org/linux-usb/d25feb0d-2ede-4722-a499-095139870c96@linux.intel.com/

looks like v2 removed the check but used tty_vhangup() instead of
tty_port_tty_vhangup()

> 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?

Does tty_port_tty_vhangup() work here? it
1. checks if tty is NULL
2. is synchronous and should be safe to call before tty_unregister_device()

tty internals are still a bit of mystery to me

Thanks
Mathias

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

[Jiri Slaby]

On 24. 11. 25, 8:48, Mathias Nyman wrote:
> On 11/24/25 08:42, Jiri Slaby wrote:
>> 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?
> 
> I recommended using tty_vhangup(), well actually tty_port_tty_vhangup() 
> to solve
> issue '2' you pointed out.
> To me it looks like tty_vhangup() is synchronous so it won't schedule 
> hangup work
> and should be safe to call right before we destroy the port.

Oops, right, my cscope DB was old and lead me to tty_hangup() instead 
(that schedules).

>> 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?
> 
> Does tty_port_tty_vhangup() work here? it
> 1. checks if tty is NULL
> 2. is synchronous and should be safe to call before tty_unregister_device()

Yes, this works for me.

thanks,
-- 
js
suse labs

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

[Łukasz Bartosik]

On Mon, Nov 24, 2025 at 10:11 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 24. 11. 25, 8:48, Mathias Nyman wrote:
> > On 11/24/25 08:42, Jiri Slaby wrote:
> >> Hmm, CCing TTY MAINTAINERS entry would not hurt.
> >>

Fair point. I will keep it in mind in the future.

> >> 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
> >>

Thanks for spotting this.

> >>> 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?
> >
> > I recommended using tty_vhangup(), well actually tty_port_tty_vhangup()
> > to solve
> > issue '2' you pointed out.
> > To me it looks like tty_vhangup() is synchronous so it won't schedule
> > hangup work
> > and should be safe to call right before we destroy the port.
>
> Oops, right, my cscope DB was old and lead me to tty_hangup() instead
> (that schedules).
>
> >> 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?
> >
> > Does tty_port_tty_vhangup() work here? it
> > 1. checks if tty is NULL
> > 2. is synchronous and should be safe to call before tty_unregister_device()
>
> Yes, this works for me.
>

Greg should I send v3 with typo fix  indifinitely -> indefinitely and
elaborate on
the tty_hangup() -> tty_vhangup() replacement in changes ?

Thank you,
Łukasz



> thanks,
> --
> js
> suse labs

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

[Greg Kroah-Hartman]

On Tue, Nov 25, 2025 at 08:53:17AM +0100, Łukasz Bartosik wrote:
> On Mon, Nov 24, 2025 at 10:11 AM Jiri Slaby <jirislaby@kernel.org> wrote:
> >
> > On 24. 11. 25, 8:48, Mathias Nyman wrote:
> > > On 11/24/25 08:42, Jiri Slaby wrote:
> > >> Hmm, CCing TTY MAINTAINERS entry would not hurt.
> > >>
> 
> Fair point. I will keep it in mind in the future.
> 
> > >> 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
> > >>
> 
> Thanks for spotting this.
> 
> > >>> 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?
> > >
> > > I recommended using tty_vhangup(), well actually tty_port_tty_vhangup()
> > > to solve
> > > issue '2' you pointed out.
> > > To me it looks like tty_vhangup() is synchronous so it won't schedule
> > > hangup work
> > > and should be safe to call right before we destroy the port.
> >
> > Oops, right, my cscope DB was old and lead me to tty_hangup() instead
> > (that schedules).
> >
> > >> 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?
> > >
> > > Does tty_port_tty_vhangup() work here? it
> > > 1. checks if tty is NULL
> > > 2. is synchronous and should be safe to call before tty_unregister_device()
> >
> > Yes, this works for me.
> >
> 
> Greg should I send v3 with typo fix  indifinitely -> indefinitely and
> elaborate on
> the tty_hangup() -> tty_vhangup() replacement in changes ?

As this is already in my tree, a fixup on top of that would be good.

thanks,

greg k-h

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

[Łukasz Bartosik]

On Tue, Nov 25, 2025 at 9:00 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Nov 25, 2025 at 08:53:17AM +0100, Łukasz Bartosik wrote:
> > On Mon, Nov 24, 2025 at 10:11 AM Jiri Slaby <jirislaby@kernel.org> wrote:
> > >
> > > On 24. 11. 25, 8:48, Mathias Nyman wrote:
> > > > On 11/24/25 08:42, Jiri Slaby wrote:
> > > >> Hmm, CCing TTY MAINTAINERS entry would not hurt.
> > > >>
> >
> > Fair point. I will keep it in mind in the future.
> >
> > > >> 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
> > > >>
> >
> > Thanks for spotting this.
> >
> > > >>> 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?
> > > >
> > > > I recommended using tty_vhangup(), well actually tty_port_tty_vhangup()
> > > > to solve
> > > > issue '2' you pointed out.
> > > > To me it looks like tty_vhangup() is synchronous so it won't schedule
> > > > hangup work
> > > > and should be safe to call right before we destroy the port.
> > >
> > > Oops, right, my cscope DB was old and lead me to tty_hangup() instead
> > > (that schedules).
> > >
> > > >> 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?
> > > >
> > > > Does tty_port_tty_vhangup() work here? it
> > > > 1. checks if tty is NULL
> > > > 2. is synchronous and should be safe to call before tty_unregister_device()
> > >
> > > Yes, this works for me.
> > >
> >
> > Greg should I send v3 with typo fix  indifinitely -> indefinitely and
> > elaborate on
> > the tty_hangup() -> tty_vhangup() replacement in changes ?
>
> As this is already in my tree, a fixup on top of that would be good.
>

I sent a fixup. I created it on top of your usb-linus branch.
Please let me know if you expect it to be created differently.

Thank you,
Łukasz

> thanks,
>
> greg k-h

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

[Jiri Slaby]

On 25. 11. 25, 15:39, Łukasz Bartosik wrote:
> I sent a fixup.

I don't see it on the stable or usb lists, nor in my inbox. Where did 
you send it to?

thanks,
-- 
js
suse labs

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

[Łukasz Bartosik]

On Wed, Nov 26, 2025 at 6:39 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 25. 11. 25, 15:39, Łukasz Bartosik wrote:
> > I sent a fixup.
>
> I don't see it on the stable or usb lists, nor in my inbox. Where did
> you send it to?
>

I sent it here https://lore.kernel.org/linux-usb/CALwA+NaXYDn1k-tVmM+-UQNJZQEZGiB4QmBfv1E6OeWyMicUig@mail.gmail.com/T/.

Thanks,
Łukasz

> thanks,
> --
> js
> suse labs