linux-kernel - Re: [PATCH 7/8] rust: pci: add physfn(), to return PF device for VF device

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: <20251121232642.GG233636@ziepe.ca>
Date: Fri, 21 Nov 2025 19:26:42 -0400
From: Jason Gunthorpe <jgg@...pe.ca>
To: Peter Colberg <pcolberg@...hat.com>
Cc: Danilo Krummrich <dakr@...nel.org>, 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>,
	Abdiel Janulgue <abdiel.janulgue@...il.com>,
	Daniel Almeida <daniel.almeida@...labora.com>,
	Robin Murphy <robin.murphy@....com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Dave Ertman <david.m.ertman@...el.com>,
	Ira Weiny <ira.weiny@...el.com>, Leon Romanovsky <leon@...nel.org>,
	linux-pci@...r.kernel.org, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Alexandre Courbot <acourbot@...dia.com>,
	Alistair Popple <apopple@...dia.com>,
	Joel Fernandes <joelagnelf@...dia.com>,
	John Hubbard <jhubbard@...dia.com>, Zhi Wang <zhiw@...dia.com>
Subject: Re: [PATCH 7/8] rust: pci: add physfn(), to return PF device for VF
 device

On Wed, Nov 19, 2025 at 05:19:11PM -0500, Peter Colberg wrote:
> Add a method to return the Physical Function (PF) device for a Virtual
> Function (VF) device in the bound device context.
> 
> Unlike for a PCI driver written in C, guarantee that when a VF device is
> bound to a driver, the underlying PF device is bound to a driver, too.

You can't do this as an absolutely statement from rust code alone,
this statement is confused.

> When a device with enabled VFs is unbound from a driver, invoke the
> sriov_configure() callback to disable SR-IOV before the unbind()
> callback. To ensure the guarantee is upheld, call disable_sriov()
> to remove all VF devices if the driver has not done so already.

This doesn't seem like it should be in this patch.

Good drivers using the PCI APIs should be calling pci_disable_sriov()
during their unbind. 

The prior patch #3 should not have added "safe" bindings for
enable_sriov that allow this lifetime rule to be violated.

> +    #[cfg(CONFIG_PCI_IOV)]
> +    pub fn physfn(&self) -> Result<&Device<device::Bound>> {
> +        if !self.is_virtfn() {
> +            return Err(EINVAL);
> +        }
> +        // SAFETY:
> +        // `self.as_raw` returns a valid pointer to a `struct pci_dev`.
> +        //
> +        // `physfn` is a valid pointer to a `struct pci_dev` since self.is_virtfn() is `true`.
> +        //
> +        // `physfn` may be cast to a `Device<device::Bound>` since `pci::Driver::remove()` calls
> +        // `disable_sriov()` to remove all VF devices, which guarantees that the underlying
> +        // PF device is always bound to a driver when the VF device is bound to a driver.

Wrong safety statement. There are drivers that don't call
disable_sriov(). You need to also check that the driver attached to
the PF is actually working properly.

I do not like to see this attempt to open code the tricky login of
pci_iov_get_pf_drvdata() in rust without understanding the issues :(

Jason