Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Safe Rust code can cause a Use-After-Free (UAF) and lock a freed Mutex by calling `registration_data_with()` on a surviving `ARef<Device>` after the parent device is unbound. -- commit 9c83313149d56b7513a5eff4702ae4ca9adae277 Author: Danilo Krummrich <[email protected]> rust: auxiliary: sample: demonstrate ForLt with invariant Mutex type Extend the auxiliary driver sample to demonstrate both access patterns: - registration_data() with CovariantForLt!(Data<'_>) for the covariant data type that holds a plain &'bound reference. - registration_data_with() with ForLt!(MutexData<'_>) for an invariant data type that wraps a Mutex<&'bound Device>. Since Mutex<T> is invariant over T, MutexData cannot implement CovariantForLt and must use the closure-based accessor. > diff --git a/samples/rust/rust_driver_auxiliary.rs > b/samples/rust/rust_driver_auxiliary.rs > index 92ee6a6d348e7..e441ae81fa2cf 100644 > --- a/samples/rust/rust_driver_auxiliary.rs > +++ b/samples/rust/rust_driver_auxiliary.rs [ ... ] > @@ -115,22 +140,39 @@ fn probe<'bound>( > > impl ParentDriver { > fn connect(adev: &auxiliary::Device<Bound>) -> Result { [ ... ] > + // Invariant ForLt types (e.g. containing a Mutex) require the > closure-based accessor. > + INVARIANT_DEV_ID => { > + adev.registration_data_with::<ForLt!(MutexData<'_>), > _>(|data| { > + let pdev = *data.parent.lock(); [Severity: Critical] This is a pre-existing issue, but can this safe Rust code cause a use-after-free if called on a surviving ARef<Device> after the parent device is unbound? In the auxiliary core, Registration::drop() frees the RegistrationData: rust/kernel/auxiliary.rs:Registration::drop() { ... drop(unsafe { Pin::<KBox<RegistrationData<F::Of<'static>>>>::from_foreign( (*self.adev.as_ptr()).registration_data_rust, ) }); ... } However, it leaves the registration_data_rust pointer dangling. Since the auxiliary_device can safely outlive the Registration (e.g. via a safe ARef<Device> inside an async task, cdev, or sysfs file), a caller holding that reference could safely call registration_data_with() and trigger a use-after-free when the dangling pointer is read: rust/kernel/auxiliary.rs:Device::registration_data_pinned() { ... let type_id = unsafe { ptr.cast::<TypeId>().read() }; ... } In this sample, wouldn't that result in locking a freed Mutex when data.parent.lock() is called? > + dev_info!( > + pdev, > + "Connected to auxiliary device with index {} (via > Mutex).\n", > + data.index > + ); > + })?; > + } > + _ => return Err(EINVAL), > + } > > Ok(()) > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
