linux-kernel - Re: [PATCH 0/6] Address race condition with Device::drvdata()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aWUUkvdKsRVJqfE2@google.com>
Date: Mon, 12 Jan 2026 15:34:42 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Danilo Krummrich <dakr@...nel.org>
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 07, 2026 at 05:40:20PM +0100, Danilo Krummrich wrote:
> 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.

so your private data is just the first devres resource ;)

Ok. I'm worried that when you fix Devres, you have to undo changes you
made here. But I guess that's not the end of the world (and maybe you
don't have to).

Concept SGTM. I have not yet reviewed patches in details, but

Acked-by: Alice Ryhl <aliceryhl@...gle.com>

Alice