Re: [PATCH v2 8/8] samples: rust: add Rust SR-IOV VF driver sample

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pci: resource leak of PCI enable count and bus-master state
--

commit e1308014aae1a5196bca00ac49a31f8c5fc01f1e
Author: Peter Colberg <pcolberg@redhat.com>

samples: rust: add Rust SR-IOV VF driver sample

Add a new SR-IOV driver sample that demonstrates how to enable and disable
the Single Root I/O Virtualization capability for a PCI device. The sample
implements the PF and VF as ordinary PCI drivers.

Link: https://www.qemu.org/docs/master/system/devices/igb.html

> diff --git a/samples/rust/rust_driver_sriov.rs b/samples/rust/rust_driver_sriov.rs
> new file mode 100644
> index 0000000000000..c5fcf4a0c62e9
> --- /dev/null
> +++ b/samples/rust/rust_driver_sriov.rs
[ ... ]
> +#[vtable]
> +impl pci::Driver for SamplePfDriver {
[ ... ]
> +    fn probe<'bound>(
> +        pdev: &'bound pci::Device<Core<'_>>,
> +        _info: Option<&'bound Self::IdInfo>,
> +    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
> +        pin_init::pin_init_scope(move || {
> +            dev_info!(
> +                pdev,
> +                "Probe Rust SR-IOV PF sample (PCI ID: {}, 0x{:x}).\n",
> +                pdev.vendor_id(),
> +                pdev.device_id()
> +            );
> +
> +            pdev.enable_device_mem()?;
> +            pdev.set_master();
> +
> +            Ok(try_pin_init!(PfDriverData {

[Severity: Medium]
If the allocation for PfDriverData or VfRegistration::new() fails within the
try_pin_init! macro, the PF probe returns an error. Does this leave the PCI
device enabled and bus-mastering active? 

The Rust PCI API pdev.enable_device_mem() binds directly to
pci_enable_device_mem(), which increments the device's enable_cnt without
using devres management, so this might leak the enable_cnt on the error path.

> +                // SAFETY:
> +                // - probe has exclusive access to this PF before SR-IOV is enabled;
[ ... ]
> +#[vtable]
> +impl pci::Driver for SampleVfDriver {
[ ... ]
> +    fn probe<'bound>(
> +        pdev: &'bound pci::Device<Core<'_>>,
> +        _info: Option<&'bound Self::IdInfo>,
> +    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
> +        pin_init::pin_init_scope(move || {
> +            dev_info!(
[ ... ]
> +            let pdev_bound: &'bound pci::Device<Bound> = pdev;
> +            let pf_api = pdev_bound.vf_registration_data::<PfApiForLt>()?;
> +
> +            pdev.enable_device_mem()?;
> +            pdev.set_master();
> +
> +            let request = pf_api.submit(pdev)?;

[Severity: Medium]
Similarly here in SampleVfDriver::probe, if pf_api.submit(pdev) fails, the VF
probe exits via the ? operator without undoing the manual enable_device_mem()
and set_master() calls. Can this permanently leak the enable_cnt and prevent
proper PCI resource management and power state transitions?

> +            dev_info!(pdev, "Submitted request {} through PF data.\n", request);
> +
> +            Ok(try_pin_init!(VfDriverData { pdev: pdev.into() }))
> +        })
> +    }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for PfDriverData<'_> {
> +    fn drop(self: Pin<&mut Self>) {
> +        dev_info!(self.pdev, "Remove Rust SR-IOV PF sample.\n");
> +    }
> +}

[Severity: Medium]
During normal driver removal, the drop logic for PfDriverData (and
VfDriverData below) fails to disable the PCI device or clear the bus master
flag. Does this mean the enable_cnt is leaked even on the successful
teardown path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924190556.1620886-1-zhiw@nvidia.com?part=8