All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Xu Yang <xu.yang_2@nxp.com>, Sean Rhodes <sean@starlabs.systems>,
	Kees Cook <kees@kernel.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb: core: port: Deattach Type-C connector on component unbind
Date: Mon, 13 Jul 2026 11:38:33 +0300	[thread overview]
Message-ID: <alSkCazrPX8U_9t4@kuha> (raw)
In-Reply-To: <20260611071201.1235545-1-acelan.kao@canonical.com>

On Thu, Jun 11, 2026 at 03:12:01PM +0800, Chia-Lin Kao (AceLan) wrote:
> connector_unbind() is the mirror of connector_bind(), but it is missing
> the symmetric call to typec_deattach() that connector_bind() makes via:
> 
>     if (port_dev->child)
>         typec_attach(port_dev->connector, &port_dev->child->dev);
> 
> When a Thunderbolt dock is unplugged, two teardown paths race:
> 
> 1. The component framework calls connector_unbind() first, which sets
>    port_dev->connector = NULL without calling typec_deattach().  This
>    leaves port->usb2_dev/port->usb3_dev in struct typec_port pointing at
>    the USB device that is about to be freed.
> 
> 2. usb_disconnect() then calls typec_deattach(port_dev->connector, ...),
>    but port_dev->connector is already NULL, so the call is a no-op and
>    port->usb2_dev is never cleared.
> 
> 3. Concurrently, UCSI detects a PD partner-disconnect event and calls
>    typec_unregister_partner(), which reads port->usb2_dev (now a dangling
>    pointer to freed memory) and passes it to typec_partner_unlink_device()
>    -> sysfs_remove_link() -> dev_name() on the freed device, corrupting
>    the typec/UCSI partner state.
> 
> This corruption leaves the Thunderbolt tunnel in an inconsistent state on
> the next dock hot-plug.  On affected hardware the dock's I225/igc NIC fails
> to enumerate: AER fires a slot reset while the igc driver is still
> initialising ("PCIe link lost"), and the subsequent igc_reset attempt hits
> igc_rd32 on an already-detached device:
> 
>     igc 0000:2e:00.0 eth0: PCIe link lost, device now detached
>     igc: Failed to read reg 0x0!
>     WARNING: CPU: 9 PID: 129 at drivers/net/ethernet/intel/igc/igc_main.c:7005
>              igc_rd32+0xa4/0xc0 [igc]
>     Call Trace:
>      igc_disable_pcie_master+0x16/0xa0 [igc]
>      igc_reset_hw_base+0x14/0x170 [igc]
>      igc_reset+0x63/0x110 [igc]
>      igc_io_slot_reset+0x9e/0xd0 [igc]
>      report_slot_reset+0x5d/0xc0
>      pcie_do_recovery+0x209/0x400
>      aer_isr_one_error_type+0x235/0x430
>      aer_isr+0x4e/0x80
>      irq_thread+0xf4/0x1f0
> 
> 4. UCSI later handles the PD partner-disconnect and calls
>    typec_unregister_partner(), which still sees the stale port->usb2_dev
>    and tries to remove its sysfs link a second time:
> 
>    kernfs: can not remove 'typec', no directory
>    WARNING: CPU: 6 PID: 55 at fs/kernfs/dir.c:1706 kernfs_remove_by_name_ns+0xe9/0xf0
>    Workqueue: events ucsi_handle_connector_change [typec_ucsi]
>    Call Trace:
>     sysfs_remove_link+0x19/0x50
>     typec_unregister_partner+0x6e/0x120 [typec]
>     ucsi_unregister_partner+0x107/0x150 [typec_ucsi]
>     ucsi_handle_connector_change+0x3ec/0x490 [typec_ucsi]
>     process_one_work+0x18e/0x3e0
>     worker_thread+0x2e3/0x420
>     kthread+0x10a/0x230
>     ret_from_fork+0x121/0x140
>     ret_from_fork_asm+0x1a/0x30
> 
>    With worse timing the same stale pointer is dereferenced after the
>    backing memory is freed, turning the warning into a use-after-free.
> 
> Fix the asymmetry: call typec_deattach() before clearing
> port_dev->connector, matching what connector_bind() does on the bind side.
> typec_partner_deattach() is already protected by port->partner_link_lock,
> so it serialises safely with the concurrent typec_unregister_partner() path.
> 
> Fixes: 11110783f5ea ("usb: Inform the USB Type-C class about enumerated devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>

FWIW:

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/core/port.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index b1364f0c384ce..b4452b665f591 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -740,6 +740,8 @@ static void connector_unbind(struct device *dev, struct device *connector, void
>  
>  	sysfs_remove_link(&connector->kobj, dev_name(dev));
>  	sysfs_remove_link(&dev->kobj, "connector");
> +	if (port_dev->child)
> +		typec_deattach(port_dev->connector, &port_dev->child->dev);
>  	port_dev->connector = NULL;
>  }
>  
> -- 
> 2.53.0
> 

-- 
heikki

      parent reply	other threads:[~2026-07-13  8:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-11  7:12 [PATCH] usb: core: port: Deattach Type-C connector on component unbind Chia-Lin Kao (AceLan)
2026-07-02  2:26 ` AceLan Kao
2026-07-02  5:50   ` Greg Kroah-Hartman
2026-07-13  8:38 ` Heikki Krogerus [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alSkCazrPX8U_9t4@kuha \
    --to=heikki.krogerus@linux.intel.com \
    --cc=acelan.kao@canonical.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=sean@starlabs.systems \
    --cc=xu.yang_2@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.