linux-kernel - Re: [PATCH 1/7] rust: pci: pass driver data by value to `unbind`

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: <DEZPV445WAYI.13J8N3Y8TLSZI@nvidia.com>
Date: Tue, 16 Dec 2025 23:38:52 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Danilo Krummrich" <dakr@...nel.org>, "Alexandre Courbot"
 <acourbot@...dia.com>
Cc: "Alice Ryhl" <aliceryhl@...gle.com>, "Simona Vetter" <simona@...ll.ch>,
 "Bjorn Helgaas" <bhelgaas@...gle.com>,
 Krzysztof Wilczyński <kwilczynski@...nel.org>, "Miguel
 Ojeda" <ojeda@...nel.org>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
 <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Trevor Gross" <tmgross@...ch.edu>,
 "Alistair Popple" <apopple@...dia.com>, "Joel Fernandes"
 <joelagnelf@...dia.com>, "Eliot Courtney" <ecourtney@...dia.com>,
 <nouveau@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>,
 <linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH 1/7] rust: pci: pass driver data by value to `unbind`

On Tue Dec 16, 2025 at 9:14 PM JST, Danilo Krummrich wrote:
> On Tue Dec 16, 2025 at 6:13 AM CET, Alexandre Courbot wrote:
>> When unbinding a PCI driver, the `T::unbind` callback is invoked by the
>> driver framework, passing the driver data as a `Pin<&T>`.
>>
>> This artificially restricts what the driver can do, as it cannot mutate
>> any state on the data. This becomes a problem in e.g. Nova, which needs
>> to invoke mutable methods when unbinding.
>>
>> `remove_callback` retrieves the driver data by value, and drops it right
>> after the call to `T::unbind`, meaning it is the only reference to the
>> driver data by the time `T::unbind` is called.
>>
>> There is thus no reason for not granting full ownership of the data to
>> `T::unbind`, so do it.
>
> There are multiple reasons I did avoid this for:
>
> (1) Race conditions
>
> A driver can call Device::drvdata() and obtain a reference to the driver's
> device private data as long as it has a &Device<Bound> and asserts the correct
> type of the driver's device private data [1].
>
> Assume you have an IRQ registration, for instance, that lives within this device
> private data.  Within the IRQ handler, nothing prevents us from calling
> Device::drvdata() given that the IRQ handler has a Device<Bound> reference.
>
> Consequently, with passing the device private data by value to unbind() it can
> happen that we have both a mutable and immutable reference at of the device
> private data at the same time.
>
> The same is true for a lot of other cases, such as work items or workqueues that
> are scoped to the Device being bound living within the device private data.
>
> More generally, you can't take full ownership of the device private data as long
> as the device is not yet fully unbound (which is not yet the case in unbind()).

Ah, I completely ignored the fact that we can indeed have other
references to the private data! The fact that `unbind` works with
`Device<Bounded>` should have given me a hint, but somehow I blissfully
ignored that. ^_^;

I will implement some basic locking on the command queue so we can work
with a non-mutable reference.