Re: [PATCH 03/10] rust: pci: use `Option<&IdInfo>` for device ID info

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The dynamic ID matching in the PCI core returns a pointer to a dynamically allocated ID without holding a lock or reference, leading to a Use-After-Free if the ID is concurrently removed via sysfs.
--

commit d1889a1b7c5b08bca9b93c95bb95cf7a983ddfea
Author: Gary Guo <gary@garyguo.net>

rust: pci: use Option<&IdInfo> for device ID info

This commit updates the Rust PCI probe functions to accept an Option<&IdInfo>
instead of a direct reference. This allows the driver to handle situations
where the driver_data is 0, such as when driver_override is used.

[ ... ]

> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 5071cae6543fd..0e055e4df99ee 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -113,7 +113,7 @@ extern "C" fn probe_callback(
>          let info = T::ID_TABLE.info(id.index());
>  
>          from_result(|| {
> -            let data = T::probe(pdev, info);
> +            let data = T::probe(pdev, Some(info));

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing
Use-After-Free in the C PCI core that impacts this callback.

In the C subsystem, pci_match_device() searches for dynamic IDs under
drv->dynids.lock and returns a pointer to the interior of dynid after
dropping the lock:

drivers/pci/pci-driver.c:pci_match_device() {
    ...
    found_id = &dynid->id;
    break;
    }
    }
    spin_unlock(&drv->dynids.lock);

    if (found_id)
    return found_id;
}

Concurrently, if a user removes the ID via sysfs, remove_id_store() frees
it unconditionally:

drivers/pci/pci-driver.c:remove_id_store() {
    ...
    list_del(&dynid->node);
    kfree(dynid);
}

When local_pci_probe() calls the driver's probe callback, it passes this
potentially dangling id pointer. Since the Rust probe_callback() receives
this pointer from C, will dereferencing it (such as when calling id.index()
just above this change) trigger a Use-After-Free crash?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618-id_info-v1-0-96af1e559ef9@garyguo.net?part=3