Linux USB
 help / color / mirror / Atom feed
From: Paul Menzel <pmenzel@molgen.mpg.de>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2] usb: typec: class: do not pin the alt mode driver module
Date: Wed,  1 Jul 2026 12:28:26 +0200	[thread overview]
Message-ID: <20260701102831.34954-2-pmenzel@molgen.mpg.de> (raw)

Plugging or unplugging a USB-C dock on a Dell XPS 13 9370 running v7.1.1
triggers a WARN splat and, on unplug, a module refcount underflow:

    WARNING: drivers/usb/typec/class.c:311 at typec_altmode_update_active+0x100/0x110 [typec]
    Workqueue: USBC000:00-con3 ucsi_poll_worker [typec_ucsi]
    Call Trace:
     ucsi_altmode_update_active+0x101/0x1a0 [typec_ucsi]
     ucsi_check_altmodes+0x92/0xd0 [typec_ucsi]
     ucsi_poll_worker+0x3a/0x110 [typec_ucsi]
  
    WARNING: kernel/module/main.c:957 at module_put+0x96/0xa0
    Workqueue: events ucsi_handle_connector_change [typec_ucsi]
    Call Trace:
     typec_altmode_update_active+0x6f/0x110 [typec]
     typec_remove+0x85/0x90 [typec]
     device_release_driver_internal+0x19e/0x200
     ucsi_unregister_altmodes+0x49/0xb0 [typec_ucsi]
     ucsi_unregister_partner+0x9e/0x160 [typec_ucsi]
     ucsi_handle_connector_change+0x386/0x420 [typec_ucsi]

For a partner or plug alt mode, typec_altmode_update_active() takes a
reference on the bound alt mode driver's module when the mode is entered
and drops it when the mode is exited, to keep the driver from being
unloaded while a mode is active.

That reference counting is unbalanced.  The get and put are keyed on
adev->dev.driver, but the alt mode driver bind/unbind runs asynchronously
and is not serialised against the active state changes issued by the UCSI
poll and connector-change workers.  If the mode is reported active before
the driver has bound, no reference is taken but adev->active is set, and
the module_put() on unbind then underflows the refcount.  try_module_get()
can also fail when the module is already going away.  These are the splats
above, seen for years via Fedora ABRT on several Dell laptops.

The pin is not needed.  When the alt mode driver is unbound—including
on module unload, which unbinds first—typec_remove() puts the mode back
into TYPEC_STATE_SAFE, clears adev->active and adev->ops, and the driver's
own remove callback drains its work (cancel_work_sync() in
dp_altmode_remove()).  The mode is torn down cleanly without the extra
module reference; all it ever bought was making rmmod of an alt mode
driver fail with -EBUSY while a mode happened to be active.

Drop the module get/put instead of trying to balance it.

Fixes: 8a37d87d72f0 ("usb: typec: Bus type for alternate modes")
Link: https://retrace.fedoraproject.org/faf/reports/542503/
Link: https://retrace.fedoraproject.org/faf/reports/1278469/
Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
Changes in v2 (addressing review from Greg Kroah-Hartman and the
Sashiko automated review):
- Drop the alt mode driver module reference counting entirely instead
  of balancing it with a tracking flag; the unbind path already tears
  the mode down safely, so the pin is unnecessary (Greg).
- Drop the WARN_ON(), which could reboot panic_on_warn systems (Greg).
- Drop the large explanatory comment (Greg).
- No new struct altmode flag, so the unlocked-flag, wrong-module and
  double-put concerns from the Sashiko review no longer apply.

v1: https://lore.kernel.org/linux-usb/20260701062417.15314-2-pmenzel@molgen.mpg.de/
sashiko: https://sashiko.dev/#/patchset/20260701062417.15314-2-pmenzel%40molgen.mpg.de
 drivers/usb/typec/class.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 0977581ad1b6e..ca19eb2a3c645 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -304,13 +304,6 @@ void typec_altmode_update_active(struct typec_altmode *adev, bool active)
 	if (adev->active == active)
 		return;
 
-	if (!is_typec_port(adev->dev.parent) && adev->dev.driver) {
-		if (!active)
-			module_put(adev->dev.driver->owner);
-		else
-			WARN_ON(!try_module_get(adev->dev.driver->owner));
-	}
-
 	adev->active = active;
 	snprintf(dir, sizeof(dir), "mode%d", adev->mode);
 	sysfs_notify(&adev->dev.kobj, dir, "active");
-- 
2.54.0


             reply	other threads:[~2026-07-01 10:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01 10:28 Paul Menzel [this message]
2026-07-03 13:14 ` [PATCH v2] usb: typec: class: do not pin the alt mode driver module Heikki Krogerus
2026-07-03 21:29   ` Paul Menzel
2026-07-04  6:59     ` Greg Kroah-Hartman

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=20260701102831.34954-2-pmenzel@molgen.mpg.de \
    --to=pmenzel@molgen.mpg.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox