Re: [PATCH] rust: pci: rework device enabling API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The new `enable_device_managed` API uses `pcim_enable_device()`, which activates legacy automatic IRQ management that conflicts with Rust's `alloc_irq_vectors()` and violates a documented kernel API contract, leading to double-free issues.
- [High] `disable_device` and `enable_device` are exposed as safe functions, allowing safe Rust code to unbalance the PCI device's `enable_cnt` reference count, causing soundness holes that lead to DMA-after-free.
--

commit 47e71534c620f329a7271d6914a49b70de710719
Author: Maurice Hieronymus <mhi@mailbox.org>
Subject: rust: pci: rework device enabling API

This commit replaces enable_device_mem() with enable_device_managed() to
provide automatic cleanup on unbind via pcim_enable_device(). It also
introduces an unmanaged enable_device()/disable_device() pair for paths
like runtime PM, enabling both I/O and memory resources.

Link: https://lore.kernel.org/r/20260620-b4-rust-pci-edu-driver-v2-0-6fd6684f2c14@mailbox.org [1]
Link: https://lore.kernel.org/rust-for-linux/DJEPMYC7J6ZG.D0ODUD4YGFCT@kernel.org/ [2]

[ ... ]
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 5071cae6543fd..a408b34120777 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -453,10 +453,30 @@ pub fn pci_class(&self) -> Class {
>  }
>  
>  impl<'a> Device<device::Core<'a>> {
> -    /// Enable memory resources for this device.
> -    pub fn enable_device_mem(&self) -> Result {
> +    /// Enable I/O and memory resources for this device.
> +    ///
> +    /// This function is unmanaged and does not perform any cleanup when the device is unbound.
> +    /// For a managed function take a look at [`Device::enable_device_managed`].
> +    #[inline]
> +    pub fn enable_device(&self) -> Result {
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> +        to_result(unsafe { bindings::pci_enable_device(self.as_raw()) })
> +    }
> +
> +    /// Disable I/O and memory resources for this device.
> +    ///
> +    /// This function is the counterpart to [`Device::enable_device`].
> +    #[inline]
> +    pub fn disable_device(&self) {

[Severity: High]
Is it safe to expose these as safe Rust functions? 

Since they directly modify the underlying PCI device's enable_cnt reference
count, safe Rust code could call them in an unbalanced way. For instance,
calling enable_device() multiple times would inflate the count. 

During driver unload, the cleanup path would only decrement it once, leaving
bus-mastering enabled and potentially leading to DMA-after-free when memory
buffers are freed. Should these be marked unsafe, or require an RAII guard
to track the enable count?

> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> +        unsafe { bindings::pci_disable_device(self.as_raw()) }
> +    }
> +
> +    /// Enable I/O and memory resources for this device, with automatic cleanup.
> +    #[inline]
> +    pub fn enable_device_managed(&self) -> Result {
>          // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
> -        to_result(unsafe { bindings::pci_enable_device_mem(self.as_raw()) })
> +        to_result(unsafe { bindings::pcim_enable_device(self.as_raw()) })
>      }

[Severity: High]
Will wrapping pcim_enable_device() cause double-free issues with IRQ vectors?

This function activates legacy C devres management for the device by setting
pdev->is_managed = true. If a driver later calls alloc_irq_vectors(), devres
will automatically register pcim_msi_release() as a cleanup action.

However, Rust's IrqVectorRegistration object also explicitly calls
pci_free_irq_vectors() on Drop. When the driver is unbound, Rust will drop
the registration (freeing the vectors), and then the devres subsystem will
execute pcim_msi_release(), attempting to free them again.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-rust-pci-enable-device-managed-v1-1-75bc4ff2935c@mailbox.org?part=1