All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions
@ 2026-08-11  0:47 Daniel Golle
  2026-08-11  2:33 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Golle @ 2026-08-11  0:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	driver-core, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Andrew Lunn, Vladimir Oltean
  Cc: netdev, linux-kernel, Miri Korenblit, linux-wireless,
	Marcel Holtmann, Luiz Augusto von Dentz, linux-bluetooth,
	Hans de Goede

Three in-tree drivers (iwlwifi, hci_h5, btintel_pcie) schedule a
deferred re-probe of their own device from a work item in module
text. The hand-rolled copies share two bug classes: the work function
ends with module_put(THIS_MODULE), racing a concurrent rmmod freeing
the module text (the race module_put_and_kthread_exit() exists to
close for kthreads), and nothing synchronizes the deferred detach
against device_shutdown() or an administrative unbind.

Patch 1 moves the deferred work into the driver core.
device_schedule_reprobe() runs builtin code, so no module reference
is needed; it checks under a single __device_driver_lock() hold that
the device is still bound to the driver that scheduled the re-probe,
and skips the detach once device_shutdown() has reached the device
(a new one-bit shutdown_done flag in struct device_private). Patches
2 and 3 are mechanical conversions. Patch 4 (btintel_pcie) also
removes that driver's remove()-from-own-work contract; it changes
more and can be dropped without affecting patches 1-3.

This grew out of review of the mxl862xx DSA series [1][2], which
copied the iwlwifi pattern. There ->shutdown() clears drvdata so that
->remove() becomes a no-op, a stale re-probe escalates to
use-after-free, and its v10 cover letter walks through why a driver
cannot fully close the race itself: device_shutdown() holds
device_lock() across ->shutdown() and device_reprobe() takes the same
lock, so any driver-side check remains a TOCTOU. The helper closes it
in the core; mxl862xx will convert once it lands.

Testing: checkpatch --strict and kernel-doc clean. Patches 1-3 were
runtime-tested (backported to 7.0.11) on Intel AX101 hardware with
PROVE_LOCKING and DEBUG_OBJECTS_WORK, driving iwlwifi's crash
escalation into the re-probe path: normal detach+rebind, an unbind
racing a pending re-probe (the unbind is not undone), rmmod with a
re-probe pending (now succeeds instead of EBUSY), and reboot with a
re-probe pending; no lockdep or debugobjects reports. hci_h5 and
btintel_pcie are compile-tested only.

[1] https://lore.kernel.org/all/cover.1786294649.git.daniel@makrotopia.org/
[2] https://sashiko.dev/#/patchset/cover.1786294649.git.daniel%40makrotopia.org

Daniel Golle (4):
  driver core: add device_schedule_reprobe()
  wifi: iwlwifi: use device_schedule_reprobe()
  Bluetooth: hci_h5: use device_schedule_reprobe()
  Bluetooth: btintel_pcie: use device_schedule_reprobe() after reset

 drivers/base/base.h                           |  5 ++
 drivers/base/core.c                           |  3 +
 drivers/base/dd.c                             | 82 +++++++++++++++++++
 drivers/bluetooth/btintel_pcie.c              | 48 ++++++-----
 drivers/bluetooth/hci_h5.c                    | 41 ++--------
 .../net/wireless/intel/iwlwifi/iwl-trans.c    | 40 +--------
 include/linux/device.h                        |  2 +
 7 files changed, 122 insertions(+), 99 deletions(-)

-- 
2.55.0

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

* Re: [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions
  2026-08-11  0:47 [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions Daniel Golle
@ 2026-08-11  2:33 ` Greg Kroah-Hartman
  2026-08-11  7:32   ` Hans de Goede
  0 siblings, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2026-08-11  2:33 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Rafael J. Wysocki, Danilo Krummrich, driver-core, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Andrew Lunn, Vladimir Oltean, netdev, linux-kernel,
	Miri Korenblit, linux-wireless, Marcel Holtmann,
	Luiz Augusto von Dentz, linux-bluetooth, Hans de Goede

On Tue, Aug 11, 2026 at 01:47:17AM +0100, Daniel Golle wrote:
> Three in-tree drivers (iwlwifi, hci_h5, btintel_pcie) schedule a
> deferred re-probe of their own device from a work item in module
> text.

That's a mess, why?  Why not fix that up to not do that?  Thousands of
other kernel drivers do not do that, what makes these so special?

> The hand-rolled copies share two bug classes: the work function
> ends with module_put(THIS_MODULE),

That's broken as-is.  a module should NEVER be calling
module_get(THIS_MODULE) either.

> racing a concurrent rmmod freeing
> the module text (the race module_put_and_kthread_exit() exists to
> close for kthreads), and nothing synchronizes the deferred detach
> against device_shutdown() or an administrative unbind.

yeah, that's a mess, don't do that.

Fix up the original drivers please, let's not encourage others to copy
this broken scheme.

Also, your patches were not threaded properly :(

thanks,

greg k-h

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

* Re: [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions
  2026-08-11  2:33 ` Greg Kroah-Hartman
@ 2026-08-11  7:32   ` Hans de Goede
  2026-08-11  8:07     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Hans de Goede @ 2026-08-11  7:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Daniel Golle
  Cc: Rafael J. Wysocki, Danilo Krummrich, driver-core, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Andrew Lunn, Vladimir Oltean, netdev, linux-kernel,
	Miri Korenblit, linux-wireless, Marcel Holtmann,
	Luiz Augusto von Dentz, linux-bluetooth

Hi Greg,

On 11-Aug-26 04:33, Greg Kroah-Hartman wrote:
> On Tue, Aug 11, 2026 at 01:47:17AM +0100, Daniel Golle wrote:
>> Three in-tree drivers (iwlwifi, hci_h5, btintel_pcie) schedule a
>> deferred re-probe of their own device from a work item in module
>> text.
> 
> That's a mess, why?  Why not fix that up to not do that?  Thousands of
> other kernel drivers do not do that, what makes these so special?

I can only speak for the hci_h5 driver where I added the reprobe code-path.

The problem is some of the Bluetooth HCI devices using hci_h5 loose all
state during system-suspend. This means that the HCI and the Bluetooth core
end up being out of sync.

So we basically need to tear down and re-build everything including
e.g. the firmware upload which happens at probe(). Doing a full reprobe
is by far the easiest way to do this.

I suspect the other 4 users + the pending driver which triggered
this are similar.

Sure we can do the whole tear-down + setup from some worker
scheduled at resume, while keep the driver attached but if we need
to duplicate that over 4 drivers + the pending driver which triggers
this then IMHO those 5 users are a pattern which deserves having
some helper to do this through the existing probe() + remove(),
rather then requiring those 5 drivers to open code this themselves.

Note that we already have device_reprobe(), which has 15 existing
users. This series just adds a helper to do a device_reprobe() from
a worker in a safe way.

Regards,

Hans




> 
>> The hand-rolled copies share two bug classes: the work function
>> ends with module_put(THIS_MODULE),
> 
> That's broken as-is.  a module should NEVER be calling
> module_get(THIS_MODULE) either.
> 
>> racing a concurrent rmmod freeing
>> the module text (the race module_put_and_kthread_exit() exists to
>> close for kthreads), and nothing synchronizes the deferred detach
>> against device_shutdown() or an administrative unbind.
> 
> yeah, that's a mess, don't do that.
> 
> Fix up the original drivers please, let's not encourage others to copy
> this broken scheme.
> 
> Also, your patches were not threaded properly :(
> 
> thanks,
> 
> greg k-h


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

* Re: [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions
  2026-08-11  7:32   ` Hans de Goede
@ 2026-08-11  8:07     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2026-08-11  8:07 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Daniel Golle, Rafael J. Wysocki, Danilo Krummrich, driver-core,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Andrew Lunn, Vladimir Oltean, netdev, linux-kernel,
	Miri Korenblit, linux-wireless, Marcel Holtmann,
	Luiz Augusto von Dentz, linux-bluetooth

On Tue, Aug 11, 2026 at 09:32:45AM +0200, Hans de Goede wrote:
> Hi Greg,
> 
> On 11-Aug-26 04:33, Greg Kroah-Hartman wrote:
> > On Tue, Aug 11, 2026 at 01:47:17AM +0100, Daniel Golle wrote:
> >> Three in-tree drivers (iwlwifi, hci_h5, btintel_pcie) schedule a
> >> deferred re-probe of their own device from a work item in module
> >> text.
> > 
> > That's a mess, why?  Why not fix that up to not do that?  Thousands of
> > other kernel drivers do not do that, what makes these so special?
> 
> I can only speak for the hci_h5 driver where I added the reprobe code-path.
> 
> The problem is some of the Bluetooth HCI devices using hci_h5 loose all
> state during system-suspend. This means that the HCI and the Bluetooth core
> end up being out of sync.
> 
> So we basically need to tear down and re-build everything including
> e.g. the firmware upload which happens at probe(). Doing a full reprobe
> is by far the easiest way to do this.
> 
> I suspect the other 4 users + the pending driver which triggered
> this are similar.

Full reprobe feels different than the THIS_MODULE stuff, which is what I
objected to here.

> Sure we can do the whole tear-down + setup from some worker
> scheduled at resume, while keep the driver attached but if we need
> to duplicate that over 4 drivers + the pending driver which triggers
> this then IMHO those 5 users are a pattern which deserves having
> some helper to do this through the existing probe() + remove(),
> rather then requiring those 5 drivers to open code this themselves.
> 
> Note that we already have device_reprobe(), which has 15 existing
> users. This series just adds a helper to do a device_reprobe() from
> a worker in a safe way.

That feels a bit better, as long as this code really is "safe" :)

So, how can this be tested and fixed up so it isn't a RFC anymore?

thanks,

greg k-h

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

end of thread, other threads:[~2026-08-11  8:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11  0:47 [RFC PATCH 0/4] device_schedule_reprobe(): core helper and conversions Daniel Golle
2026-08-11  2:33 ` Greg Kroah-Hartman
2026-08-11  7:32   ` Hans de Goede
2026-08-11  8:07     ` Greg Kroah-Hartman

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.