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

[Danilo Krummrich]

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@xxxxxxxxxx>

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.