linux-kernel - Re: [PATCH] rust: devres: fix race condition due to nesting

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aYYVjT8OZTKYdaML@google.com>
Date: Fri, 6 Feb 2026 16:23:41 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: gregkh@...uxfoundation.org, rafael@...nel.org, ojeda@...nel.org, 
	boqun.feng@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com, 
	lossin@...nel.org, a.hindborg@...nel.org, tmgross@...ch.edu, 
	driver-core@...ts.linux.dev, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Boris Brezillon <boris.brezillon@...labora.com>, 
	Markus Probst <markus.probst@...teo.de>
Subject: Re: [PATCH] rust: devres: fix race condition due to nesting

On Fri, Feb 06, 2026 at 04:56:54PM +0100, Danilo Krummrich wrote:
> On Fri Feb 6, 2026 at 4:54 PM CET, Alice Ryhl wrote:
> > On Thu, Feb 05, 2026 at 11:25:15PM +0100, Danilo Krummrich wrote:
> >> Commit f5d3ef25d238 ("rust: devres: get rid of Devres' inner Arc") did
> >> attempt to optimize away the internal reference count of Devres.
> >> 
> >> However, without an internal reference count, we can't support cases
> >> where Devres is indirectly nested, resulting into a deadlock.
> >> 
> >> Such indirect nesting easily happens in the following way:
> >> 
> >> A registration object (which is guarded by devres) hold a reference
> >> count of an object that holds a device resource guarded by devres
> >> itself.
> >> 
> >> For instance a drm::Registration holds a reference of a drm::Device. The
> >> drm::Device itself holds a device resource in its private data.
> >> 
> >> When the drm::Registration is dropped by devres, and it happens that it
> >> did hold the last reference count of the drm::Device, it also drops the
> >> device resource, which is guarded by devres itself.
> >
> > Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>
> 
> Thanks for the review!
> 
> >> +        // Take additional reference count for `devm_add_action()`.
> >> +        core::mem::forget(data.clone());
> >
> > I'd feel better if you called .clone() prior to devm_add_action(). That
> > way, even if devm somehow runs the callback before we get to this call
> > to clone, the refcount has already been incremented.
> >
> > I know it's not really a problem because of the &Device<Bound> argument.
> 
> This is intentional, since, as you say, &Device<Bound> guarantees that this
> won't happen and since otherwise we'd explicitly need to drop the reference
> count again if devm_add_action() fails.

It wouldn't be explicit, as you can let destructor do it. But I'm not
going to force it. Feel free to land it as-is.

Alice