On Fri Jun 19, 2026 at 6:05 PM BST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> - [Critical] Arbitrary Memory Dereference / Privilege Escalation via PCI
> dynamic IDs (`new_id`)
> - [High] Undefined Behavior via unconditional null reference creation when
> `driver_data` is 0
> --
>
> commit 01871465d12a97f7be006ca329ebda24f458cf1e
> Author: Gary Guo <[email protected]>
>
> rust: driver: store pointers in `DeviceId`
>
> This commit changes Rust device IDs to store pointers directly in the
> driver_data field, matching the behavior of C drivers. It works around
> the limitation of missing provenance during const evaluation by wrapping
> raw IDs in MaybeUninit.
>
>> diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
>> index 84852a2d9ad78..59453588df0e7 100644
>> --- a/rust/kernel/device_id.rs
>> +++ b/rust/kernel/device_id.rs
> [ ... ]
>> @@ -47,15 +50,48 @@ pub unsafe trait RawDeviceIdIndex: RawDeviceId {
>> /// The offset (in bytes) to the context/data field in the raw device
>> ID.
>> const DRIVER_DATA_OFFSET: usize;
>>
>> - /// The index stored at `DRIVER_DATA_OFFSET` of the implementor of the
>> [`RawDeviceIdIndex`]
>> - /// trait.
>> - fn index(&self) -> usize;
>> + /// Obtain the data pointer stored inside the device ID.
>> + ///
>> + /// # Safety
>> + ///
>> + /// `&Self` must be stored inside a `IdArray<Self, U>`.
>> + unsafe fn info_unchecked<U>(&self) -> &'static U {
>> + // SAFETY: By safety requirement of the trait, this is
>> `self.driver_data as *const U` and by
>> + // the safety requirement of the function, this is stored in
>> `IdArray<Self, U>` so is
>> + // convertible to `&'static U`.
>> + unsafe {
>> + core::ptr::from_ref(self)
>> + .byte_add(Self::DRIVER_DATA_OFFSET)
>> + .cast::<&U>()
>> + .read()
>> + }
>> + }
>
> [Severity: High]
> Does this code create an unconditional null reference in Rust if the
> driver_data or driver_info field is 0?
>
> Creating a null reference is immediate undefined behavior in Rust, even for
> zero-sized types. This 0 value can occur in paths like:
>
> 1. IdArray::new_without_index() leaving driver_data fields as 0.
>
> 2. When userspace adds a dynamic USB ID via the sysfs new_id interface using
> fewer than 5 fields (omitting refVendor), USB core initializes driver_info
> to 0.
>
> [ ... ]
>> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
>> index a3dd48f763532..a630c7fc6a853 100644
>> --- a/rust/kernel/pci.rs
>> +++ b/rust/kernel/pci.rs
> [ ... ]
>> @@ -110,10 +110,13 @@ extern "C" fn probe_callback(
>> // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of
>> `struct pci_device_id` and
>> // does not add additional invariants, so it's safe to transmute.
>> let id = unsafe { &*id.cast::<DeviceId>() };
>> - let info = T::ID_TABLE.info(id.index());
>> +
>> + // SAFETY: `id` comes from `T::ID_TABLE` which is of type
>> `IdArray<_, T::IdInfo>` or
>> + // `pci_device_id_any` which has 0 as driver_data.
>> + let info = unsafe { id.info_unchecked_opt::<T::IdInfo>() };
>
> [Severity: Critical]
> Can this allow an arbitrary memory dereference or bypass kernel lockdown?
>
> The PCI subsystem supports dynamic IDs via the sysfs new_id interface
> (drivers/pci/pci-driver.c), which parses up to 7 arguments, the last being
> driver_data (%lx).
>
> This allows a privileged user (root) to supply an arbitrary pointer value.
> The code here will blindly cast this integer to an Option<&'static U> via
> info_unchecked_opt() and dereference it if non-zero, allowing arbitrary
> kernel memory access.
False positive because dyn ID requires the data to be matching one of the
existing data.
>
> [ ... ]
>> diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
>> index 500b5e0ba4ea9..8aeff50117552 100644
>> --- a/rust/kernel/usb.rs
>> +++ b/rust/kernel/usb.rs
> [ ... ]
>> @@ -89,7 +89,8 @@ extern "C" fn probe_callback(
>> // does not add additional invariants, so it's safe to
>> transmute.
>> let id = unsafe { &*id.cast::<DeviceId>() };
>>
>> - let info = T::ID_TABLE.info(id.index());
>> + // SAFETY: `id` comes from `T::ID_TABLE` which is of type
>> `IdArray<_, T::IdInfo>`.
>> + let info = unsafe { id.info_unchecked::<T::IdInfo>() };
>> let data = T::probe(intf, id, info);
>
> [Severity: High]
> Does this code trigger undefined behavior when the underlying USB device ID
> driver_info field is 0?
>
> When the id is constructed using IdArray::new_without_index() or when
> dynamically added via sysfs new_id with fewer fields, the driver_info is 0.
> /sys/bus/
> This probe unconditionally expects a valid non-null reference and creates
> one even for 0-initialized IDs via info_unchecked().
Looks like I need to convert USB code to use `Option<&Self::IdInfo>` too, like
what I did to PCI.
Best,
Gary
