Re: [PATCH v16 2/3] samples: rust: add Rust serial device bus sample device driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Race condition causing Use-After-Free and NULL pointer dereference in `receive_buf_callback` during device unbind or probe failure.
- [Medium] Missing `write_wakeup` hook in serdev abstraction causes driver stalls on transmit buffer full.
--

--- Patch [2]: [PATCH v16 2/3] samples: rust: add Rust serial device bus sample device driver ---
commit 55560927fe62016a13f3a51d6f3e3f813e10db57
Author: Markus Probst <markus.probst@posteo.de>

samples: rust: add Rust serial device bus sample device driver
    
Add a sample Rust serial device bus device driver illustrating the usage
of the serial device bus abstractions.

> diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
> new file mode 100644
> index 0000000000000..a12b1412db30c
> --- /dev/null
> +++ b/samples/rust/rust_driver_serdev.rs
[ ... ]
> +    fn receive<'bound>(
> +        sdev: &'bound serdev::Device<Bound>,
> +        _this: Pin<&Self>,
> +        data: &[u8],
> +    ) -> usize {
> +        sdev.write(data).unwrap_or_default() as usize

[Severity: Medium]
Will this driver stall on a full transmit buffer?

If the transmit buffer fills up, sdev.write() returns a short count, which the
driver propagates back to the TTY core.

The TTY core responds by pausing further receive operations until space becomes
available. 

Since the Rust serdev abstraction hardcodes write_wakeup to
bindings::serdev_device_write_wakeup (which only wakes up synchronous writes via
write_all):

rust/kernel/serdev.rs:Adapter::OPS {
    ...
    write_wakeup: Some(bindings::serdev_device_write_wakeup),
    ...
}

the driver cannot hook write_wakeup to be notified when transmit space becomes
available.

As a result, the driver never resumes the TTY receive path, causing a permanent
stall until a new byte physically arrives to re-trigger the receive workqueue.

> +    }
> +}
> +
> +impl Drop for SampleDriver {
> +    fn drop(&mut self) {
> +        dev_dbg!(
> +            self.sdev.as_ref(),
> +            "Remove Rust Serial device bus device driver sample.\n"
> +        );
> +    }
> +}

[Severity: High]
Does the serdev abstraction have a use-after-free or NULL pointer dereference
race condition during device unbind or probe failure?

During device unbind, the driver core invokes post_unbind_callback, which calls
drvdata_obtain to take ownership of the driver data, setting drvdata to NULL:

rust/kernel/device.rs:drvdata_obtain() {
    ...
    unsafe { bindings::dev_set_drvdata(self.as_raw(), core::ptr::null_mut()) };
    ...
}

It then drops PrivateData, which closes the device:

rust/kernel/serdev.rs:PinnedDrop for PrivateData {
    ...
    unsafe { bindings::serdev_device_close(self.sdev.as_raw()) };
    ...
}

Because the device is closed only at the end of the drop routine, the TTY port
remains active while drvdata is NULL and while PrivateData is being freed.

If data arrives on the UART during this window, the TTY layer concurrently
invokes receive_buf_callback, which borrows drvdata:

rust/kernel/serdev.rs:receive_buf_callback() {
    ...
    let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
    ...
}

rust/kernel/device.rs:drvdata_borrow() {
    ...
    unsafe { Pin::<KBox<T>>::borrow(ptr.cast()) }
    ...
}

This will either dereference the NULL drvdata pointer, or if it already read
the pointer before drvdata_obtain, it will access PrivateData after it has been
freed when attempting to lock active.

The same race exists in the probe error path where ScopeGuard drops PrivateData
before the device is closed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718-rust_serdev-v16-0-5809384d2e1b@posteo.de?part=2