linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xhci: dbgtty: fix device unregister: fixup typo
@ 2025-11-25 14:25 Łukasz Bartosik
  2025-11-26  7:15 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 10+ messages in thread
From: Łukasz Bartosik @ 2025-11-25 14:25 UTC (permalink / raw)
  To: Mathias Nyman, Greg Kroah-Hartman; +Cc: linux-usb, Ł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 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.

Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
Link: https://patch.msgid.link/20251119212910.1245694-1-ukaszb@google.com

Fix typo indifinitely->indefinitely

--
2.52.0.460.gd25c4c69ec-goog

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xhci: dbgtty: fix device unregister: fixup typo
  2025-11-25 14:25 [PATCH] xhci: dbgtty: fix device unregister: fixup typo Łukasz Bartosik
@ 2025-11-26  7:15 ` Greg Kroah-Hartman
  2025-11-26  7:58   ` Łukasz Bartosik
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2025-11-26  7:15 UTC (permalink / raw)
  To: Łukasz Bartosik; +Cc: Mathias Nyman, linux-usb

On Tue, Nov 25, 2025 at 02:25:31PM +0000, Ł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 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.
> 
> Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
> Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
> Link: https://patch.msgid.link/20251119212910.1245694-1-ukaszb@google.com
> 
> Fix typo indifinitely->indefinitely
> 
> --
> 2.52.0.460.gd25c4c69ec-goog
> 

I see no patch here :(

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xhci: dbgtty: fix device unregister: fixup typo
  2025-11-26  7:15 ` Greg Kroah-Hartman
@ 2025-11-26  7:58   ` Łukasz Bartosik
  2025-11-26  9:15     ` Mathias Nyman
  2025-11-26 10:14     ` Greg Kroah-Hartman
  0 siblings, 2 replies; 10+ messages in thread
From: Łukasz Bartosik @ 2025-11-26  7:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Mathias Nyman, linux-usb

On Wed, Nov 26, 2025 at 8:15 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Nov 25, 2025 at 02:25:31PM +0000, Ł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 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.
> >
> > Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
> > Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
> > Link: https://patch.msgid.link/20251119212910.1245694-1-ukaszb@google.com
> >
> > Fix typo indifinitely->indefinitely
> >
> > --
> > 2.52.0.460.gd25c4c69ec-goog
> >
>
> I see no patch here :(

Hi Greg,

The typo I fixed was in the commit message indifinitely->indefinitely.

Would you please elaborate what is your expectation for a fixup in this case ?

Thanks,
Łukasz

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xhci: dbgtty: fix device unregister: fixup typo
  2025-11-26  7:58   ` Łukasz Bartosik
@ 2025-11-26  9:15     ` Mathias Nyman
  2025-11-26  9:50       ` Łukasz Bartosik
  2025-11-26 10:14     ` Greg Kroah-Hartman
  1 sibling, 1 reply; 10+ messages in thread
From: Mathias Nyman @ 2025-11-26  9:15 UTC (permalink / raw)
  To: Łukasz Bartosik, Greg Kroah-Hartman
  Cc: Mathias Nyman, linux-usb, Jiri Slaby

On 11/26/25 09:58, Łukasz Bartosik wrote:
> On Wed, Nov 26, 2025 at 8:15 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>>
>> On Tue, Nov 25, 2025 at 02:25:31PM +0000, Ł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 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.
>>>
>>> Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
>>> Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
>>> Link: https://patch.msgid.link/20251119212910.1245694-1-ukaszb@google.com
>>>
>>> Fix typo indifinitely->indefinitely
>>>
>>> --
>>> 2.52.0.460.gd25c4c69ec-goog
>>>
>>
>> I see no patch here :(
> 
> Hi Greg,
> 
> The typo I fixed was in the commit message indifinitely->indefinitely.
> 
> Would you please elaborate what is your expectation for a fixup in this case ?
> 

A fixup patch is just a new patch that is applied on top of the first.

If the typo was in the commit message then it can't be helped.

I still think we need a fixup patch for the missing null check

-	tty_vhangup(port->port.tty);
+	if (port->port.tty)
+		tty_vhangup(port->port.tty);

or just use tty_port_tty_vhangup()

Remember to add Jiri Slaby and stable to CC, and a new Fixes tag
Fixes: 1f73b8b56cf3 ("xhci: dbgtty: fix device unregister")

Thanks
Mathias



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xhci: dbgtty: fix device unregister: fixup typo
  2025-11-26  9:15     ` Mathias Nyman
@ 2025-11-26  9:50       ` Łukasz Bartosik
  2025-11-26  9:56         ` Jiri Slaby
  0 siblings, 1 reply; 10+ messages in thread
From: Łukasz Bartosik @ 2025-11-26  9:50 UTC (permalink / raw)
  To: Mathias Nyman; +Cc: Greg Kroah-Hartman, Mathias Nyman, linux-usb, Jiri Slaby

On Wed, Nov 26, 2025 at 10:15 AM Mathias Nyman
<mathias.nyman@linux.intel.com> wrote:
>
> On 11/26/25 09:58, Łukasz Bartosik wrote:
> > On Wed, Nov 26, 2025 at 8:15 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> >>
> >> On Tue, Nov 25, 2025 at 02:25:31PM +0000, Ł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 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.
> >>>
> >>> Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
> >>> Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
> >>> Link: https://patch.msgid.link/20251119212910.1245694-1-ukaszb@google.com
> >>>
> >>> Fix typo indifinitely->indefinitely
> >>>
> >>> --
> >>> 2.52.0.460.gd25c4c69ec-goog
> >>>
> >>
> >> I see no patch here :(
> >
> > Hi Greg,
> >
> > The typo I fixed was in the commit message indifinitely->indefinitely.
> >
> > Would you please elaborate what is your expectation for a fixup in this case ?
> >
>
> A fixup patch is just a new patch that is applied on top of the first.
>

Thank you for the clarification.

> If the typo was in the commit message then it can't be helped.
>

:(

> I still think we need a fixup patch for the missing null check
>
> -       tty_vhangup(port->port.tty);
> +       if (port->port.tty)
> +               tty_vhangup(port->port.tty);
>
> or just use tty_port_tty_vhangup()
>

IMHO it looks good because tty_vhangup calls two functions inside:
1) tty_debug_hangup - which handles the case when tty is null
2) __tty_hangup - which also checks tty for null

Thanks,
Łukasz

> Remember to add Jiri Slaby and stable to CC, and a new Fixes tag
> Fixes: 1f73b8b56cf3 ("xhci: dbgtty: fix device unregister")
>
> Thanks
> Mathias
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xhci: dbgtty: fix device unregister: fixup typo
  2025-11-26  9:50       ` Łukasz Bartosik
@ 2025-11-26  9:56         ` Jiri Slaby
  2025-11-26 12:26           ` Łukasz Bartosik
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Slaby @ 2025-11-26  9:56 UTC (permalink / raw)
  To: Łukasz Bartosik, Mathias Nyman
  Cc: Greg Kroah-Hartman, Mathias Nyman, linux-usb

On 26. 11. 25, 10:50, Łukasz Bartosik wrote:
>> I still think we need a fixup patch for the missing null check
>>
>> -       tty_vhangup(port->port.tty);
>> +       if (port->port.tty)
>> +               tty_vhangup(port->port.tty);
>>
>> or just use tty_port_tty_vhangup()
>>
> 
> IMHO it looks good because tty_vhangup calls two functions inside:
> 1) tty_debug_hangup - which handles the case when tty is null
> 2) __tty_hangup - which also checks tty for null

Is it still good when someone closes the TTY right after the "if (!tty)" 
checks?

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xhci: dbgtty: fix device unregister: fixup typo
  2025-11-26  7:58   ` Łukasz Bartosik
  2025-11-26  9:15     ` Mathias Nyman
@ 2025-11-26 10:14     ` Greg Kroah-Hartman
  1 sibling, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2025-11-26 10:14 UTC (permalink / raw)
  To: Łukasz Bartosik; +Cc: Mathias Nyman, linux-usb

On Wed, Nov 26, 2025 at 08:58:10AM +0100, Łukasz Bartosik wrote:
> On Wed, Nov 26, 2025 at 8:15 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Nov 25, 2025 at 02:25:31PM +0000, Ł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 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.
> > >
> > > Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver")
> > > Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
> > > Link: https://patch.msgid.link/20251119212910.1245694-1-ukaszb@google.com
> > >
> > > Fix typo indifinitely->indefinitely
> > >
> > > --
> > > 2.52.0.460.gd25c4c69ec-goog
> > >
> >
> > I see no patch here :(
> 
> Hi Greg,
> 
> The typo I fixed was in the commit message indifinitely->indefinitely.
> 
> Would you please elaborate what is your expectation for a fixup in this case ?

Ah, I thought it was a change to the diff.  We can't change changelogs
after they are in the tree, sorry.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xhci: dbgtty: fix device unregister: fixup typo
  2025-11-26  9:56         ` Jiri Slaby
@ 2025-11-26 12:26           ` Łukasz Bartosik
  2025-11-26 14:05             ` Łukasz Bartosik
  0 siblings, 1 reply; 10+ messages in thread
From: Łukasz Bartosik @ 2025-11-26 12:26 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Mathias Nyman, Greg Kroah-Hartman, Mathias Nyman, linux-usb

On Wed, Nov 26, 2025 at 10:56 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 26. 11. 25, 10:50, Łukasz Bartosik wrote:
> >> I still think we need a fixup patch for the missing null check
> >>
> >> -       tty_vhangup(port->port.tty);
> >> +       if (port->port.tty)
> >> +               tty_vhangup(port->port.tty);
> >>
> >> or just use tty_port_tty_vhangup()
> >>
> >
> > IMHO it looks good because tty_vhangup calls two functions inside:
> > 1) tty_debug_hangup - which handles the case when tty is null
> > 2) __tty_hangup - which also checks tty for null
>
> Is it still good when someone closes the TTY right after the "if (!tty)"
> checks?
>

Hi Jiri,

Will putting tty_vhangup around get and put reference as follows suffice:
tty = tty_port_tty_get(port);
if (tty) {
    tty_vhangup(tty);
    tty_kref_put(tty);
}

Thanks,
Łukasz

> thanks,
> --
> js
> suse labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xhci: dbgtty: fix device unregister: fixup typo
  2025-11-26 12:26           ` Łukasz Bartosik
@ 2025-11-26 14:05             ` Łukasz Bartosik
  2025-11-27 14:19               ` Łukasz Bartosik
  0 siblings, 1 reply; 10+ messages in thread
From: Łukasz Bartosik @ 2025-11-26 14:05 UTC (permalink / raw)
  To: Jiri Slaby, Greg Kroah-Hartman; +Cc: Mathias Nyman, Mathias Nyman, linux-usb

On Wed, Nov 26, 2025 at 1:26 PM Łukasz Bartosik <ukaszb@chromium.org> wrote:
>
> On Wed, Nov 26, 2025 at 10:56 AM Jiri Slaby <jirislaby@kernel.org> wrote:
> >
> > On 26. 11. 25, 10:50, Łukasz Bartosik wrote:
> > >> I still think we need a fixup patch for the missing null check
> > >>
> > >> -       tty_vhangup(port->port.tty);
> > >> +       if (port->port.tty)
> > >> +               tty_vhangup(port->port.tty);
> > >>
> > >> or just use tty_port_tty_vhangup()
> > >>
> > >
> > > IMHO it looks good because tty_vhangup calls two functions inside:
> > > 1) tty_debug_hangup - which handles the case when tty is null
> > > 2) __tty_hangup - which also checks tty for null
> >
> > Is it still good when someone closes the TTY right after the "if (!tty)"
> > checks?
> >
>
> Hi Jiri,
>
> Will putting tty_vhangup around get and put reference as follows suffice:
> tty = tty_port_tty_get(port);
> if (tty) {
>     tty_vhangup(tty);
>     tty_kref_put(tty);
> }
>

Ah I missed what already Mathias pointed out to me in the comment
https://lore.kernel.org/all/6171754f-1b84-47e0-a4da-0d045ea7546e@linux.intel.com/
that there is a helper tty_port_tty_vhangup which takes care of the
locking as well.

I will send a fixup which switches tty_vhangup -> tty_port_tty_vhangup.

Thanks,
Łukasz

> Thanks,
> Łukasz
>
> > thanks,
> > --
> > js
> > suse labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xhci: dbgtty: fix device unregister: fixup typo
  2025-11-26 14:05             ` Łukasz Bartosik
@ 2025-11-27 14:19               ` Łukasz Bartosik
  0 siblings, 0 replies; 10+ messages in thread
From: Łukasz Bartosik @ 2025-11-27 14:19 UTC (permalink / raw)
  To: Jiri Slaby, Greg Kroah-Hartman, Mathias Nyman; +Cc: Mathias Nyman, linux-usb

On Wed, Nov 26, 2025 at 3:05 PM Łukasz Bartosik <ukaszb@chromium.org> wrote:
>
> On Wed, Nov 26, 2025 at 1:26 PM Łukasz Bartosik <ukaszb@chromium.org> wrote:
> >
> > On Wed, Nov 26, 2025 at 10:56 AM Jiri Slaby <jirislaby@kernel.org> wrote:
> > >
> > > On 26. 11. 25, 10:50, Łukasz Bartosik wrote:
> > > >> I still think we need a fixup patch for the missing null check
> > > >>
> > > >> -       tty_vhangup(port->port.tty);
> > > >> +       if (port->port.tty)
> > > >> +               tty_vhangup(port->port.tty);
> > > >>
> > > >> or just use tty_port_tty_vhangup()
> > > >>
> > > >
> > > > IMHO it looks good because tty_vhangup calls two functions inside:
> > > > 1) tty_debug_hangup - which handles the case when tty is null
> > > > 2) __tty_hangup - which also checks tty for null
> > >
> > > Is it still good when someone closes the TTY right after the "if (!tty)"
> > > checks?
> > >
> >
> > Hi Jiri,
> >
> > Will putting tty_vhangup around get and put reference as follows suffice:
> > tty = tty_port_tty_get(port);
> > if (tty) {
> >     tty_vhangup(tty);
> >     tty_kref_put(tty);
> > }
> >
>
> Ah I missed what already Mathias pointed out to me in the comment
> https://lore.kernel.org/all/6171754f-1b84-47e0-a4da-0d045ea7546e@linux.intel.com/
> that there is a helper tty_port_tty_vhangup which takes care of the
> locking as well.
>
> I will send a fixup which switches tty_vhangup -> tty_port_tty_vhangup.
>

I sent the fixup
https://lore.kernel.org/linux-usb/20251127111644.3161386-1-ukaszb@google.com/T/

Thank you,
Łukasz

> Thanks,
> Łukasz
>
> > Thanks,
> > Łukasz
> >
> > > thanks,
> > > --
> > > js
> > > suse labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-11-27 14:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-25 14:25 [PATCH] xhci: dbgtty: fix device unregister: fixup typo Łukasz Bartosik
2025-11-26  7:15 ` Greg Kroah-Hartman
2025-11-26  7:58   ` Łukasz Bartosik
2025-11-26  9:15     ` Mathias Nyman
2025-11-26  9:50       ` Łukasz Bartosik
2025-11-26  9:56         ` Jiri Slaby
2025-11-26 12:26           ` Łukasz Bartosik
2025-11-26 14:05             ` Łukasz Bartosik
2025-11-27 14:19               ` Łukasz Bartosik
2025-11-26 10:14     ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).