Re: [PATCH v3 10/27] rust: pci: make Driver trait lifetime-parameterized

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The introduction of the `'bound` lifetime parameter allows drivers to capture short-lived references into driver private data, leading to a formal use-after-free (provenance violation) and Undefined Behavior when accessed later.
- [Low] The signature of `unbind` in `samples/rust/rust_driver_pci.rs` does not match the newly parameterized `pci::Driver` trait, causing a compilation error.
--

commit e20151fd03221a2c3958612a0dabaa50cdd67768
Author: Danilo Krummrich <dakr@kernel.org>

rust: pci: make Driver trait lifetime-parameterized

This changes the pci::Driver trait to add a 'bound lifetime to the
associated Data type, allowing bus device private data to capture the
device and driver bound lifetime.

> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index c06ac3fb2a64f..8060861199d41 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs

[ ... ]

> @@ -130,7 +130,7 @@ extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {
>          // SAFETY: `remove_callback` is only ever called after a successful call to
>          // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
>          // and stored a `Pin<KBox<T::Data>>`.
> -        let data = unsafe { pdev.as_ref().drvdata_borrow::<T::Data>() };
> +        let data = unsafe { pdev.as_ref().drvdata_borrow::<T::Data<'_>>() };

Does this lead to a formal use-after-free and provenance violation under
Rust's aliasing rules?

In probe_callback, a reference to pdev is created with a lifetime restricted
to the callback's scope, which is stored in Data<'probe>.

Since set_drvdata stores this data by converting it to a raw pointer, the
'probe lifetime formally ends when probe_callback returns.

Between probe and remove, the C subsystem can mutate the pci_dev struct using
its parent raw pointer, which invalidates the child reference created during
probe.

When remove_callback calls drvdata_borrow, does this execute an unchecked
transmute from the expired 'probe lifetime to a fresh 'remove lifetime?

When the driver accesses these references, will it use a reference with an
invalidated provenance tag and trigger undefined behavior?

Would it be safer to use ARef<Device>, which stores a raw pointer and creates
fresh references on demand, rather than circumventing the borrow checker with
synthetic lifetimes?

[ ... ]

> diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
> index f43c6a660b395..30b9311812f3e 100644
> --- a/samples/rust/rust_driver_pci.rs
> +++ b/samples/rust/rust_driver_pci.rs
> @@ -140,11 +140,14 @@ fn config_space(pdev: &pci::Device<Bound>) {
>  
>  impl pci::Driver for SampleDriver {
>      type IdInfo = TestIndex;
> -    type Data = Self;
> +    type Data<'bound> = Self;
>  
>      const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
>  
> -    fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> {
> +    fn probe<'bound>(
> +        pdev: &'bound pci::Device<Core>,
> +        info: &'bound Self::IdInfo,
> +    ) -> impl PinInit<Self, Error> + 'bound {
>          pin_init::pin_init_scope(move || {
>              let vendor = pdev.vendor_id();
>              dev_dbg!(

Does the unbind method in this file also need to be updated to match the new
pci::Driver trait signature?

Since the trait adds a 'bound lifetime parameter to unbind, failing to update
the implementation here leaves elided lifetimes that resolve to two separate
anonymous lifetimes.

Will the compiler reject this with a lifetime parameter mismatch error,
breaking the build?

I note this regression is fixed later in the patch series by the commit
"samples: rust: rust_driver_pci: use HRT lifetime for Bar", but does it break
git bisectability here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517000149.3226762-1-dakr@kernel.org?part=10