linux-kernel - Re: [PATCH v2 1/2] rust: pci: skip probing VFs if driver doesn't support VFs

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: <DD85P4NV5B5Y.367RGWFHBR0RF@kernel.org>
Date: Thu, 02 Oct 2025 23:32:44 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Jason Gunthorpe" <jgg@...dia.com>
Cc: "John Hubbard" <jhubbard@...dia.com>, "Alexandre Courbot"
 <acourbot@...dia.com>, "Joel Fernandes" <joelagnelf@...dia.com>, "Timur
 Tabi" <ttabi@...dia.com>, "Alistair Popple" <apopple@...dia.com>, "Zhi
 Wang" <zhiw@...dia.com>, "Surath Mitra" <smitra@...dia.com>, "David Airlie"
 <airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>, "Alex Williamson"
 <alex.williamson@...hat.com>, "Bjorn Helgaas" <bhelgaas@...gle.com>,
 Krzysztof Wilczyński <kwilczynski@...nel.org>, "Miguel
 Ojeda" <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "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>, "Alice
 Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>,
 <nouveau@...ts.freedesktop.org>, <linux-pci@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>, "LKML" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] rust: pci: skip probing VFs if driver doesn't
 support VFs

On Thu Oct 2, 2025 at 11:14 PM CEST, Danilo Krummrich wrote:
> On 10/2/25 11:04 PM, Jason Gunthorpe wrote:
>> On Thu, Oct 02, 2025 at 09:36:17PM +0200, Danilo Krummrich wrote:
>>> If we want to obtain the driver's private data from a device outside the scope
>>> of bus callbacks, we always need to ensure that the device is guaranteed to be
>>> bound and we also need to prove the type of the private data, since a device
>>> structure can't be generic over its bound driver.
>> 
>> pci_iov_get_pf_drvdata() does both of these things - this is what it
>> is for. Please don't open code it :(
>
> It makes no sense to use it, both of those things will be ensured in a more
> generic way in the base device implementation already (which is what I meant
> with layering).
>
> Both requirements are not specific to PCI, or the specific VF -> PF use-case.
>
> In order to guarantee soundness both of those things have to be guaranteed for
> any access to the driver's private data.

Actually, let me elaborate a bit more on this:

In C when a driver calls dev_get_drvdata() it asserts two things:

  (1) The device is bound to the driver calling this function.
  (2) It casts the returned void * to the correct type.

Obviously, relying on this in Rust would be fundamentally unsafe.

Hence, the idea is to implement Device::drvdata_borrow::<T>(), which takes a
&Device<Bound> as argument, which proves that the device must be bound.

The T must be the driver specific driver type, i.e. the type that implements
e.g. the pci::Driver trait.

With that, Device::drvdata_borrow() can internally check whether the asserted
type T matches the unique type ID that we store in the device in probe().

This way we can prove that the device is actually bound and that we cast to the
correct type.

Furthermore, the returned reference to the driver's private data can't out-live
the lifetime of the given &Device<Bound>, so we're also guaranteed that the
device won't be unbound while we still have a reference to the driver's private
data.

So, when we call pdev.physfn().drvdata_borrow::<NovaCore>() the checks are
included already.

> I will send some patches soon, I think this will make it obvious. :)
>>>> Certain conditions may be workable, some drivers seem to have
>>>> preferences not to call disable, though I think that is wrong :\
>>>
>>> I fully agree! I was told that this is because apparently some PF drivers are
>>> only loaded to enable SR-IOV and then removed to shrink the potential attack
>>> surface. Personally, I think that's slightly paranoid, if the driver would not
>>> do anything else than enable / disable SR-IOV, but I think we can work around
>>> this use-case if people really want it.
>> 
>> I've heard worse reasons than that. If that is the interest I'd
>> suggest they should just use VFIO and leave a userspace stub
>> process..
>
> I'm not sure I follow your proposal, can you elaborate?