public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Alice Ryhl" <aliceryhl@google.com>
Cc: <gregkh@linuxfoundation.org>, <rafael@kernel.org>,
	<igor.korotin.linux@gmail.com>, <ojeda@kernel.org>,
	<boqun.feng@gmail.com>, <gary@garyguo.net>,
	<bjorn3_gh@protonmail.com>, <lossin@kernel.org>,
	<a.hindborg@kernel.org>, <tmgross@umich.edu>,
	<david.m.ertman@intel.com>, <ira.weiny@intel.com>,
	<leon@kernel.org>, <bhelgaas@google.com>,
	<kwilczynski@kernel.org>, <wsa+renesas@sang-engineering.com>,
	<linux-kernel@vger.kernel.org>, <rust-for-linux@vger.kernel.org>,
	<linux-pci@vger.kernel.org>, <linux-usb@vger.kernel.org>,
	<linux-i2c@vger.kernel.org>
Subject: Re: [PATCH 0/6] Address race condition with Device::drvdata()
Date: Wed, 07 Jan 2026 17:40:20 +0100	[thread overview]
Message-ID: <DFII83QY76O0.2PKZ73WCTVGPR@kernel.org> (raw)
In-Reply-To: <aV6BHw-Liv0SVAwO@google.com>

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

  reply	other threads:[~2026-01-07 16:40 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-07 10:34 [PATCH 0/6] Address race condition with Device::drvdata() Danilo Krummrich
2026-01-07 10:35 ` [PATCH 1/6] rust: i2c: do not drop device private data on shutdown() Danilo Krummrich
2026-01-07 10:35 ` [PATCH 2/6] rust: auxiliary: add Driver::unbind() callback Danilo Krummrich
2026-01-07 10:35 ` [PATCH 3/6] rust: driver: introduce a common Driver trait Danilo Krummrich
2026-01-14 19:40   ` Igor Korotin
2026-01-07 10:35 ` [PATCH 4/6] rust: driver: add DEVICE_DRIVER_OFFSET to the " Danilo Krummrich
2026-01-07 10:35 ` [PATCH 5/6] rust: driver: add DriverData type to the generic " Danilo Krummrich
2026-01-07 10:35 ` [PATCH 6/6] rust: driver: drop device private data post unbind Danilo Krummrich
2026-01-07 12:22   ` Greg KH
2026-01-07 12:50     ` Danilo Krummrich
2026-01-07 14:54       ` Greg KH
2026-01-12 14:27         ` Danilo Krummrich
2026-01-12 15:03           ` Greg KH
2026-01-07 15:51 ` [PATCH 0/6] Address race condition with Device::drvdata() Alice Ryhl
2026-01-07 16:40   ` Danilo Krummrich [this message]
2026-01-12 15:34     ` Alice Ryhl
2026-01-12 15:47       ` Danilo Krummrich
2026-01-14 19:50 ` Igor Korotin
2026-01-16  0:23 ` Danilo Krummrich

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=DFII83QY76O0.2PKZ73WCTVGPR@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=david.m.ertman@intel.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=igor.korotin.linux@gmail.com \
    --cc=ira.weiny@intel.com \
    --cc=kwilczynski@kernel.org \
    --cc=leon@kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --cc=wsa+renesas@sang-engineering.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox