Re: [PATCH v18 1/3] rust: leds: add basic led classdev abstractions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `Device` abstraction drops the `'a` lifetime of the `name` reference, leading to a potential use-after-free of the LED name.
--

commit bc77118736b867c88c1854d7a5c4eb57818535fd
Author: Markus Probst <markus.probst@posteo.de>

rust: leds: add basic led classdev abstractions

Implement the core abstractions needed for led class devices, including
LedOps, DeviceBuilder, and Device.

> diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
> new file mode 100644
> index 0000000000000..09244133f14c8
> --- /dev/null
> +++ b/rust/kernel/led/normal.rs

[ ... ]

> +impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
> +    /// Registers a new [`Device`].
> +    pub fn build<'bound: 'a, T: LedOps + 'bound>(
> +        self,
> +        parent: &'bound T::Bus,
> +        ops: impl PinInit<T, Error> + 'a,
> +    ) -> impl PinInit<Device<'bound, T>, Error> + 'a {
> +        const_assert!(T::MAX_BRIGHTNESS <= i32::MAX.unsigned_abs() || !T::HAS_BRIGHTNESS_GET);
> +
> +        try_pin_init!(Device {
> +            ops <- ops,
> +            classdev <- Opaque::try_ffi_init(|ptr: *mut bindings::led_classdev| {
> +                // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write.
> +                // `led_classdev` gets fully initialized in-place by
> +                // `led_classdev_register_ext` including `mutex` and `list_head`.
> +                unsafe {
> +                    ptr.write(bindings::led_classdev {
> +                        brightness_set: (!T::BLOCKING)
> +                            .then_some(Adapter::<T>::brightness_set_callback),
> +                        brightness_set_blocking: T::BLOCKING
> +                            .then_some(Adapter::<T>::brightness_set_blocking_callback),
> +                        brightness_get: T::HAS_BRIGHTNESS_GET
> +                            .then_some(Adapter::<T>::brightness_get_callback),
> +                        blink_set: T::HAS_BLINK_SET.then_some(Adapter::<T>::blink_set_callback),
> +                        max_brightness: T::MAX_BRIGHTNESS,
> +                        brightness: self.initial_brightness,
> +                        color: self.color as u32,
> +                        name: self.name.map_or(core::ptr::null(), CStrExt::as_char_ptr),

[Severity: High]
Does this erase the lifetime of the name string and risk a use-after-free?

In DeviceBuilder::build, the builder takes a string reference tied to the 'a
lifetime, but the returned Device struct does not capture this lifetime. The
raw pointer from the name is then written directly into the long-lived
led_classdev struct.

If the name string is dropped immediately after the Device is initialized
(for example, if a dynamically allocated CString was used in the driver probe
function), the led_classdev struct registered with the C subsystem will
contain a dangling pointer.

Could this lead to memory corruption if C code inspects the global leds_list
and dereferences the dangling name pointer?

> +                        ..bindings::led_classdev::default()
> +                    })
> +                };

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260531-rust_leds-v18-0-d07102ba5170@posteo.de?part=1