Re: [PATCH v2 09/11] rust: driver: remove open-coded matching logic

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Data race and NULL pointer dereference due to lockless access of `dev->driver` in `Adapter::id_info`.
--

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

rust: driver: remove open-coded matching logic

This commit replaces the open-coded ACPI and OF matching logic in the Rust
driver abstractions with a single call to device_get_match_data(). It removes
the acpi_of_match_device() helper and updates the Adapter::id_info() function
to use the C API directly.

> diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
> index 824899d76fed2..c9c74c4dde8fa 100644
> --- a/rust/kernel/driver.rs
> +++ b/rust/kernel/driver.rs

[ ... ]

> @@ -325,117 +304,23 @@ pub trait Adapter {
[ ... ]
> -    fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
[ ... ]
> +    /// Returns the driver's private data from the matching entry of any of the ID tables, if any.
> +    ///
> +    /// If this returns `None`, it means that there is no match in any of the ID tables directly
> +    /// associated with a [`device::Device`].
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must ensure that the `dev` matched data is of type `Self::IdInfo`.
> +    #[inline]
> +    unsafe fn id_info(dev: &device::Device) -> Option<&'static Self::IdInfo> {
> +        // SAFETY: `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
> +        let data = unsafe { bindings::device_get_match_data(dev.as_raw()) };

[Severity: High]
Does this introduce a potential NULL pointer dereference if dev->driver is
not set? 

Previously, id_info() performed a table lookup that did not depend on the
device being bound to a driver. By switching to device_get_match_data(), the
underlying C implementations (such as of_device_get_match_data() or
acpi_device_get_match_data()) will unconditionally dereference dev->driver.

If a Rust driver calls Adapter::id_info() on a device::Device that is not
currently bound (for example, from a concurrent sysfs attribute or a
workqueue), could this lead to a panic? 

Should this lifecycle precondition (that the device must be bound to a driver)
be explicitly documented in the # Safety contract to prevent unsoundness, or
should we ensure dev->driver is valid before calling this function?

> +
> +        // SAFETY: Per safety requirement, `data` is of type `Self::IdInfo`.
> +        unsafe { data.cast::<Self::IdInfo>().as_ref() }
>      }
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-id_info-v2-0-56fccbe9c5ef@garyguo.net?part=9