* [PATCH] usb: core: deattach child device from typec connector on port unbind
@ 2026-07-21 20:22 Marco Tormento
2026-07-22 16:02 ` Heikki Krogerus
0 siblings, 1 reply; 6+ messages in thread
From: Marco Tormento @ 2026-07-21 20:22 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Heikki Krogerus, Alan Stern, linux-usb, linux-kernel,
Marco Tormento
connector_bind() tells the Type-C connector about a port's already
attached child via typec_attach(), but connector_unbind() has no
mirror image: it drops port_dev->connector without deattaching a
still-present child first. When that happens, port->usb2_dev (or
usb3_dev) in the Type-C port is left pointing at a device that is
about to be torn down, and typec_partner_deattach() never runs for
it.
This shows up on hardware where a single Type-C connector's component
aggregate spans ports on more than one USB root hub sharing an xHCI
controller (e.g. a Thunderbolt-attached hub exposing both a USB-2 and
a USB-3 root-hub port through the same connector). Unbinding the
connector from one root hub's ports also unbinds it for the other's,
even though the other root hub's child device is still attached and
gets disconnected later, by which point the connector is already
gone. The result is a "kernfs: can not remove 'typec', no directory"
warning the first time, "sysfs: cannot create duplicate filename"
the next time the device is reattached, and eventually a general
protection fault in typec_unregister_partner() when it dereferences
port->usb2_dev/usb3_dev after the device it points to has been freed.
Fix connector_unbind() to mirror connector_bind(): deattach the
child from the connector before dropping the reference to it. This
closes the race regardless of which order the parent USB controllers
happen to be torn down in.
Reported on a Lenovo Thinkpad T480s (BenQ EX3501R monitor with an
integrated USB hub, connected via the Thunderbolt-capable Type-C
port, alongside a USB-C power delivery brick on the other Type-C
port). Ran the reported plug/unplug sequence multiple times on that
hardware with this patch applied and could not reproduce the failure;
the same kernel build without the patch reproduced it on the first
attempt. Supersedes an earlier attempt that reordered
typec_deattach() in usb_disconnect() instead; that approach worked
around the same race but only for one ordering of controller teardown,
and inverted the normal teardown-mirrors-setup convention.
The original investigation, including the v1 patch and the hardware
reproduction that led to identifying this bug, is my own work. The
connector_unbind() fix approach below was proposed by Claude (Anthropic,
Sonnet 5) during code review of the v1 patch. Hardware reproduction and
validation of this fix were carried out by me, on the same hardware, per
the note above.
Link: https://lore.kernel.org/r/20250720210847.30998-1-mtormento80@gmail.com
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Marco Tormento <mtormento80@gmail.com>
---
drivers/usb/core/port.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index b1364f0c384c..3b258246bb3f 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -738,6 +738,14 @@ static void connector_unbind(struct device *dev, struct device *connector, void
{
struct usb_port *port_dev = to_usb_port(dev);
+ /*
+ * If a USB device is still connected to the port, let the
+ * Type-C connector know it's going away before we drop our
+ * reference to it.
+ */
+ if (port_dev->child)
+ typec_deattach(data, &port_dev->child->dev);
+
sysfs_remove_link(&connector->kobj, dev_name(dev));
sysfs_remove_link(&dev->kobj, "connector");
port_dev->connector = NULL;
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] usb: core: deattach child device from typec connector on port unbind
2026-07-21 20:22 [PATCH] usb: core: deattach child device from typec connector on port unbind Marco Tormento
@ 2026-07-22 16:02 ` Heikki Krogerus
2026-07-22 16:25 ` Marco Tormento
0 siblings, 1 reply; 6+ messages in thread
From: Heikki Krogerus @ 2026-07-22 16:02 UTC (permalink / raw)
To: Marco Tormento; +Cc: Greg Kroah-Hartman, Alan Stern, linux-usb, linux-kernel
On Tue, Jul 21, 2026 at 10:22:01PM +0200, Marco Tormento wrote:
> connector_bind() tells the Type-C connector about a port's already
> attached child via typec_attach(), but connector_unbind() has no
> mirror image: it drops port_dev->connector without deattaching a
> still-present child first. When that happens, port->usb2_dev (or
> usb3_dev) in the Type-C port is left pointing at a device that is
> about to be torn down, and typec_partner_deattach() never runs for
> it.
>
> This shows up on hardware where a single Type-C connector's component
> aggregate spans ports on more than one USB root hub sharing an xHCI
> controller (e.g. a Thunderbolt-attached hub exposing both a USB-2 and
> a USB-3 root-hub port through the same connector). Unbinding the
> connector from one root hub's ports also unbinds it for the other's,
> even though the other root hub's child device is still attached and
> gets disconnected later, by which point the connector is already
> gone. The result is a "kernfs: can not remove 'typec', no directory"
> warning the first time, "sysfs: cannot create duplicate filename"
> the next time the device is reattached, and eventually a general
> protection fault in typec_unregister_partner() when it dereferences
> port->usb2_dev/usb3_dev after the device it points to has been freed.
>
> Fix connector_unbind() to mirror connector_bind(): deattach the
> child from the connector before dropping the reference to it. This
> closes the race regardless of which order the parent USB controllers
> happen to be torn down in.
>
> Reported on a Lenovo Thinkpad T480s (BenQ EX3501R monitor with an
> integrated USB hub, connected via the Thunderbolt-capable Type-C
> port, alongside a USB-C power delivery brick on the other Type-C
> port). Ran the reported plug/unplug sequence multiple times on that
> hardware with this patch applied and could not reproduce the failure;
> the same kernel build without the patch reproduced it on the first
> attempt. Supersedes an earlier attempt that reordered
> typec_deattach() in usb_disconnect() instead; that approach worked
> around the same race but only for one ordering of controller teardown,
> and inverted the normal teardown-mirrors-setup convention.
>
> The original investigation, including the v1 patch and the hardware
> reproduction that led to identifying this bug, is my own work. The
> connector_unbind() fix approach below was proposed by Claude (Anthropic,
> Sonnet 5) during code review of the v1 patch. Hardware reproduction and
> validation of this fix were carried out by me, on the same hardware, per
> the note above.
>
> Link: https://lore.kernel.org/r/20250720210847.30998-1-mtormento80@gmail.com
>
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Marco Tormento <mtormento80@gmail.com>
There is already a fix for this:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-linus&id=e0b291fe117964037e0ba382eff4bb365d531c3a
Thanks,
> ---
> drivers/usb/core/port.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index b1364f0c384c..3b258246bb3f 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -738,6 +738,14 @@ static void connector_unbind(struct device *dev, struct device *connector, void
> {
> struct usb_port *port_dev = to_usb_port(dev);
>
> + /*
> + * If a USB device is still connected to the port, let the
> + * Type-C connector know it's going away before we drop our
> + * reference to it.
> + */
> + if (port_dev->child)
> + typec_deattach(data, &port_dev->child->dev);
> +
> sysfs_remove_link(&connector->kobj, dev_name(dev));
> sysfs_remove_link(&dev->kobj, "connector");
> port_dev->connector = NULL;
> --
> 2.55.0
--
heikki
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] usb: core: deattach child device from typec connector on port unbind
2026-07-22 16:02 ` Heikki Krogerus
@ 2026-07-22 16:25 ` Marco Tormento
2026-07-27 11:13 ` Heikki Krogerus
0 siblings, 1 reply; 6+ messages in thread
From: Marco Tormento @ 2026-07-22 16:25 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: Greg Kroah-Hartman, Alan Stern, linux-usb, linux-kernel
On Wed, 22 Jul 2026 at 18:02, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
> There is already a fix for this:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-linus&id=e0b291fe117964037e0ba382eff4bb365d531c3a
Sorry, I did not notice it.
Shouldn't the typec deattach come before the sysfs removals though,
reversing what happens in connector_bind?
Regards,
Marco
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: core: deattach child device from typec connector on port unbind
2026-07-22 16:25 ` Marco Tormento
@ 2026-07-27 11:13 ` Heikki Krogerus
2026-07-27 16:51 ` Marco Tormento
0 siblings, 1 reply; 6+ messages in thread
From: Heikki Krogerus @ 2026-07-27 11:13 UTC (permalink / raw)
To: Marco Tormento; +Cc: Greg Kroah-Hartman, Alan Stern, linux-usb, linux-kernel
Hi Marco,
On Wed, Jul 22, 2026 at 06:25:30PM +0200, Marco Tormento wrote:
> On Wed, 22 Jul 2026 at 18:02, Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> > There is already a fix for this:
> > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-linus&id=e0b291fe117964037e0ba382eff4bb365d531c3a
>
> Sorry, I did not notice it.
> Shouldn't the typec deattach come before the sysfs removals though,
> reversing what happens in connector_bind?
We should not need to reverse anything. We can't avoid calling
typec_attach() separately from connector_bind() because we can't
prevent USB devices from being enumerated before the typec drivers are
loaded for example during bootup. But that does not mean we need to
call typec_deattach() in connector_unbind().
After the USB device is "attached" to the typec connector, the
connector device should be pinned down for as long as the USB device
exists, but that is not happening. So what both you guys are doing, is
working around a problem with the reference count.
I can see that most of your commit message is written by some LLM, and
the parts that you've added don't usually belong to the commit
message. That makes me a bit concerned about how much you actually
understand what the code that you are modifying does.
Use of LLMs is fine, but you still have understand the code that you
modify.
Br,
--
heikki
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: core: deattach child device from typec connector on port unbind
2026-07-27 11:13 ` Heikki Krogerus
@ 2026-07-27 16:51 ` Marco Tormento
2026-08-05 11:49 ` Heikki Krogerus
0 siblings, 1 reply; 6+ messages in thread
From: Marco Tormento @ 2026-07-27 16:51 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: Greg Kroah-Hartman, Alan Stern, linux-usb, linux-kernel
Hi Heikki,
On Mon, 27 Jul 2026 at 13:13, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> We should not need to reverse anything. We can't avoid calling
> typec_attach() separately from connector_bind() because we can't
> prevent USB devices from being enumerated before the typec drivers are
> loaded for example during bootup. But that does not mean we need to
> call typec_deattach() in connector_unbind().
>
> After the USB device is "attached" to the typec connector, the
> connector device should be pinned down for as long as the USB device
> exists, but that is not happening. So what both you guys are doing, is
> working around a problem with the reference count.
>
> I can see that most of your commit message is written by some LLM, and
> the parts that you've added don't usually belong to the commit
> message. That makes me a bit concerned about how much you actually
> understand what the code that you are modifying does.
>
> Use of LLMs is fine, but you still have understand the code that you
> modify.
First of all, thank you for the explanation.
The commit message definitely sounded over confident, my understanding
of the issue is summed up here in my first patch submission a year
ago: https://lore.kernel.org/all/20250720210847.30998-1-mtormento80@gmail.com/
As I said back then, my experience on kernel code was nearly zero, and
I was well aware there might be better ways to fix the issue: that's
also true today.
What I did a few days ago was to ask AI to analyse my first patch and
see if it could find a better mitigation.
It came up with this patch, I read the explanation and it made sense
for my level of understanding of the matter (which is arguably low,
unfortunately).
I checked the code and tested for a while with success, so I submitted
a new patch and here we are.
Do you have any suspects yet on where the underlying issue might be?
Regards,
Marco
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: core: deattach child device from typec connector on port unbind
2026-07-27 16:51 ` Marco Tormento
@ 2026-08-05 11:49 ` Heikki Krogerus
0 siblings, 0 replies; 6+ messages in thread
From: Heikki Krogerus @ 2026-08-05 11:49 UTC (permalink / raw)
To: Marco Tormento; +Cc: Greg Kroah-Hartman, Alan Stern, linux-usb, linux-kernel
On Mon, Jul 27, 2026 at 06:51:36PM +0200, Marco Tormento wrote:
> Hi Heikki,
>
> On Mon, 27 Jul 2026 at 13:13, Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > We should not need to reverse anything. We can't avoid calling
> > typec_attach() separately from connector_bind() because we can't
> > prevent USB devices from being enumerated before the typec drivers are
> > loaded for example during bootup. But that does not mean we need to
> > call typec_deattach() in connector_unbind().
> >
> > After the USB device is "attached" to the typec connector, the
> > connector device should be pinned down for as long as the USB device
> > exists, but that is not happening. So what both you guys are doing, is
> > working around a problem with the reference count.
> >
> > I can see that most of your commit message is written by some LLM, and
> > the parts that you've added don't usually belong to the commit
> > message. That makes me a bit concerned about how much you actually
> > understand what the code that you are modifying does.
> >
> > Use of LLMs is fine, but you still have understand the code that you
> > modify.
>
> First of all, thank you for the explanation.
>
> The commit message definitely sounded over confident, my understanding
> of the issue is summed up here in my first patch submission a year
> ago: https://lore.kernel.org/all/20250720210847.30998-1-mtormento80@gmail.com/
>
> As I said back then, my experience on kernel code was nearly zero, and
> I was well aware there might be better ways to fix the issue: that's
> also true today.
>
> What I did a few days ago was to ask AI to analyse my first patch and
> see if it could find a better mitigation.
> It came up with this patch, I read the explanation and it made sense
> for my level of understanding of the matter (which is arguably low,
> unfortunately).
> I checked the code and tested for a while with success, so I submitted
> a new patch and here we are.
>
> Do you have any suspects yet on where the underlying issue might be?
Right now my gut feeling is that this should be fixed in
drivers/usb/typec/class.c, but I would need to be able to study this
more carefully.
Thanks,
--
heikki
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-05 11:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21 20:22 [PATCH] usb: core: deattach child device from typec connector on port unbind Marco Tormento
2026-07-22 16:02 ` Heikki Krogerus
2026-07-22 16:25 ` Marco Tormento
2026-07-27 11:13 ` Heikki Krogerus
2026-07-27 16:51 ` Marco Tormento
2026-08-05 11:49 ` Heikki Krogerus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox