Message-Id: <DFII83QY76O0.2PKZ73WCTVGPR@kernel.org>
Date: Wed, 07 Jan 2026 17:40:20 +0100
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Alice Ryhl" <aliceryhl@...gle.com>
Cc: <gregkh@...uxfoundation.org>, <rafael@...nel.org>,
 <igor.korotin.linux@...il.com>, <ojeda@...nel.org>, <boqun.feng@...il.com>,
 <gary@...yguo.net>, <bjorn3_gh@...tonmail.com>, <lossin@...nel.org>,
 <a.hindborg@...nel.org>, <tmgross@...ch.edu>, <david.m.ertman@...el.com>,
 <ira.weiny@...el.com>, <leon@...nel.org>, <bhelgaas@...gle.com>,
 <kwilczynski@...nel.org>, <wsa+renesas@...g-engineering.com>,
 <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>,
 <linux-pci@...r.kernel.org>, <linux-usb@...r.kernel.org>,
 <linux-i2c@...r.kernel.org>
Subject: Re: [PATCH 0/6] Address race condition with Device::drvdata()

On Wed Jan 7, 2026 at 4:51 PM CET, Alice Ryhl wrote:
> If a &Device<Bound> lets you access a given value, then we must not
> destroy that value until after the last &Device<Bound> has expired.
>
> A &Device<Bound> lets you access the driver private data. And a
> &Device<Bound> lets you access the contents of a Devres<T>.
>
> Thus, the last &Device<Bound> must expire before we destroy driver
> private data or values inside of Devres<T>. Etc.

Yes, the last &Device<Bound> must expire before we destroy the device private
data. This is exactly what is achieved by this patch. The device private data is
destroyed after all devres callbacks have been processed, which guarantees that
there can't be any contexts left that provide a &Device<Bound>.

As for the values inside of a Devres<T>, this is exactly what I refer to in my
paragraph above talking about the unsoundness of the devres cleanup ordering in
Rust.

I also mention that I'm already working on a solution and it is in fact pretty
close to the solution you propose below, i.e. a generic mechanism to support
multiple devres domains (which I also see advantages for in C code).

As mentioned, this will also help with getting the required synchronize_rcu()
calls down to exactly one per device unbind.

Technically, we could utilize such a devres domain for dropping the device
private data, but there is no need to have a separate domain just for this, we
already have a distinct place for dropping and freeing the device private data
after the device has been fully unbound, which is much simpler than a separate
devres domain.

Now, you may argue we don't need a separate devres domain, and that we could use
the non-early devres domain. However, this would have the following implication:

In the destructor of the device private data, drivers could still try to use
device resources stored in the device private data through try_access(), which
may or may not succeed depending on whether the corresponding Devres<T>
containers are part of the device private data initializer or whether they have
been allocated separately.

Or in other words it would leave room for drivers to abuse this behavior.

Therefore, the desired order is:

  1. Driver::unbind() (A place for drivers to tear down the device;
     registrations are up - unless explicitly revoked by the driver (this is a
     semantic choice) - and device resources are accessible.)

  2. devm_early_* (Drop all devres guarded registrations.)

  3. No more &Device<Bound> left.

  4. devm_* (Drop all device resources.)

  5. No more device resources left.

  6. Drop and free device private data. (try_access() will never succeed in the
     destructor of the device private data.

- Danilo

Powered by blists - more mailing lists