linux-kernel - Re: [PATCH 0/2] rust: pci: expose is_virtfn() and reject VFs in nova-core

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: <20251002123244.GF3195801@nvidia.com>
Date: Thu, 2 Oct 2025 09:32:44 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: John Hubbard <jhubbard@...dia.com>, Zhi Wang <zhiw@...dia.com>,
	Alistair Popple <apopple@...dia.com>,
	Alexandre Courbot <acourbot@...dia.com>,
	Joel Fernandes <joelagnelf@...dia.com>,
	Timur Tabi <ttabi@...dia.com>, Surath Mitra <smitra@...dia.com>,
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
	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" <nouveau@...ts.freedesktop.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"rust-for-linux@...r.kernel.org" <rust-for-linux@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Alex Williamson <alex.williamson@...hat.com>
Subject: Re: [PATCH 0/2] rust: pci: expose is_virtfn() and reject VFs in
 nova-core

On Thu, Oct 02, 2025 at 02:08:27PM +0200, Danilo Krummrich wrote:

> Why? What about other upstream drivers that clearly assert that they don't
> support VFs?

They shouldn't be doing that either. There is lots of junk in Linux,
that doesn't mean it should be made first-class to encourage more
people to do the wrong thing.

> Why would we want to force them to try to boot to a point where
> they "naturally" fail?

We want them to work.
 
> https://elixir.bootlin.com/linux/v6.17/source/drivers/net/ethernet/realtek/rtase/rtase_main.c#L2195
> https://elixir.bootlin.com/linux/v6.17/source/drivers/net/ethernet/intel/ice/ice_main.c#L5266
> https://elixir.bootlin.com/linux/v6.17/source/drivers/net/ethernet/intel/igb/igb_main.c#L3221

This usage seems wrong to me:

commit 50ac7479846053ca8054be833c1594e64de496bb
Author: Anirudh Venkataramanan <anirudh.venkataramanan@...el.com>
Date:   Wed Jul 28 12:39:10 2021 -0700

    ice: Prevent probing virtual functions
    
    The userspace utility "driverctl" can be used to change/override the
    system's default driver choices. This is useful in some situations
    (buggy driver, old driver missing a device ID, trying a workaround,
    etc.) where the user needs to load a different driver.
    
    However, this is also prone to user error, where a driver is mapped
    to a device it's not designed to drive. For example, if the ice driver
    is mapped to driver iavf devices, the ice driver crashes.
    
    Add a check to return an error if the ice driver is being used to
    probe a virtual function.

Decoding this.. There is actually an "iavf" driver, and it does have
special PCI IDs for VFs:

static const struct pci_device_id iavf_pci_tbl[] = {
        {PCI_VDEVICE(INTEL, IAVF_DEV_ID_VF), 0},
        {PCI_VDEVICE(INTEL, IAVF_DEV_ID_VF_HV), 0},
        {PCI_VDEVICE(INTEL, IAVF_DEV_ID_X722_VF), 0},
        {PCI_VDEVICE(INTEL, IAVF_DEV_ID_ADAPTIVE_VF), 0},

In normal cases iavf will probe to the SRIOV VFS just fine.

The above is saying if the user mis-uses driverctl to bind the ice
driver to a function that doesn't have matching PCI IDs then the
kernel crashes. Yeah. I'm pretty sure that is true for a lot of
drivers. Bind them to HW not in their ID tables and their are not
going to work right.

I would have rejected a patch like this. The ID table is already
correct and properly excludes VFs.

Jason