[PATCH v2 0/2] rust: pwm: Fix init error handling and tidy style

[PATCH v2 0/2] rust: pwm: Fix init error handling and tidy style

[Kari Argillander]

This series contains two small updates to the Rust PWM bindings.

The first patch fixes a potential memory leak on an error path during PWM
chip initialization. Someone needs to decide if this goes to stable.

The second patch is just style-only cleanup.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
Changes in v2:
- Use inspect_err().
- Reword SAFETY comment.
- Reword git messages.
- Added Fixes tag
- Link to v1: https://lore.kernel.org/r/20251219-pwm-rust-v1-0-46873e19679d@gmail.com

---
Kari Argillander (2):
      rust: pwm: Fix potential memory leak on init error
      rust: pwm: Simplify to_result call sites and unsafe blocks

 rust/kernel/pwm.rs | 53 +++++++++++++++++++----------------------------------
 1 file changed, 19 insertions(+), 34 deletions(-)
---
base-commit: cc3aa43b44bdb43dfbac0fcb51c56594a11338a8
change-id: 20251219-pwm-rust-7df846d0f183

Best regards,
-- 
Kari Argillander <kari.argillander@gmail.com>

[PATCH v2 1/2] rust: pwm: Fix potential memory leak on init error

[Kari Argillander]

When initializing a PWM chip using pwmchip_alloc(), the allocated device
owns an initial reference that must be released on all error paths.

If __pinned_init() were to fail, the allocated pwm_chip would currently
leak because the error path returns without calling pwmchip_put().

Fixes: 7b3dce814a15 ("rust: pwm: Add Kconfig and basic data structures")
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 rust/kernel/pwm.rs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
index 2dd72a39acb5..4f683158fc08 100644
--- a/rust/kernel/pwm.rs
+++ b/rust/kernel/pwm.rs
@@ -607,7 +607,11 @@ pub fn new<'a>(
         let drvdata_ptr = unsafe { bindings::pwmchip_get_drvdata(c_chip_ptr) };
 
         // SAFETY: We construct the `T` object in-place in the allocated private memory.
-        unsafe { data.__pinned_init(drvdata_ptr.cast())? };
+        unsafe { data.__pinned_init(drvdata_ptr.cast()) }.inspect_err(|_| {
+            // SAFETY: It is safe to call `pwmchip_put()` with a valid pointer obtained
+            // from `pwmchip_alloc()`. We will not use pointer after this.
+            unsafe { bindings::pwmchip_put(c_chip_ptr) }
+        })?;
 
         // SAFETY: `c_chip_ptr` points to a valid chip.
         unsafe {

-- 
2.43.0

Re: [PATCH v2 1/2] rust: pwm: Fix potential memory leak on init error

[Michal Wilczynski]

Hi,
On 1/2/26 08:51, Kari Argillander wrote:
> When initializing a PWM chip using pwmchip_alloc(), the allocated device
> owns an initial reference that must be released on all error paths.
> 
> If __pinned_init() were to fail, the allocated pwm_chip would currently
> leak because the error path returns without calling pwmchip_put().
> 
> Fixes: 7b3dce814a15 ("rust: pwm: Add Kconfig and basic data structures")
> Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> ---
>  rust/kernel/pwm.rs | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
> index 2dd72a39acb5..4f683158fc08 100644
> --- a/rust/kernel/pwm.rs
> +++ b/rust/kernel/pwm.rs
> @@ -607,7 +607,11 @@ pub fn new<'a>(
>          let drvdata_ptr = unsafe { bindings::pwmchip_get_drvdata(c_chip_ptr) };
>  
>          // SAFETY: We construct the `T` object in-place in the allocated private memory.
> -        unsafe { data.__pinned_init(drvdata_ptr.cast())? };
> +        unsafe { data.__pinned_init(drvdata_ptr.cast()) }.inspect_err(|_| {
> +            // SAFETY: It is safe to call `pwmchip_put()` with a valid pointer obtained
> +            // from `pwmchip_alloc()`. We will not use pointer after this.
> +            unsafe { bindings::pwmchip_put(c_chip_ptr) }
> +        })?;
>  
>          // SAFETY: `c_chip_ptr` points to a valid chip.
>          unsafe {
> 

Thank you for your patch, sorry for late reply due to holiday season. I
think the patch is good and the Fixes tag is appropriate, as this logic
should have been there in the first place regardless of whether the
pinned init can fail currently or not.

The v2 is fairly similar to what happens with drm device in device.rs
and I think it's more appropriate than v1.

Acked-by: Michal Wilczynski <m.wilczynski@samsung.com>

[PATCH v2 2/2] rust: pwm: Simplify to_result call sites and unsafe blocks

[Kari Argillander]

Remove unnecessary temporary variables around to_result() calls and move
trailing semicolons outside unsafe blocks to improve readability and
produce cleaner rustfmt output.

No functional change intended.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 rust/kernel/pwm.rs | 47 ++++++++++++++---------------------------------
 1 file changed, 14 insertions(+), 33 deletions(-)

diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
index 4f683158fc08..6c9d667009ef 100644
--- a/rust/kernel/pwm.rs
+++ b/rust/kernel/pwm.rs
@@ -129,8 +129,7 @@ pub fn set_waveform(&self, wf: &Waveform, exact: bool) -> Result {
         // SAFETY: `self.as_raw()` provides a valid `*mut pwm_device` pointer.
         // `&c_wf` is a valid pointer to a `pwm_waveform` struct. The C function
         // handles all necessary internal locking.
-        let ret = unsafe { bindings::pwm_set_waveform_might_sleep(self.as_raw(), &c_wf, exact) };
-        to_result(ret)
+        to_result(unsafe { bindings::pwm_set_waveform_might_sleep(self.as_raw(), &c_wf, exact) })
     }
 
     /// Queries the hardware for the configuration it would apply for a given
@@ -160,9 +159,7 @@ pub fn get_waveform(&self) -> Result<Waveform> {
 
         // SAFETY: `self.as_raw()` is a valid pointer. We provide a valid pointer
         // to a stack-allocated `pwm_waveform` struct for the kernel to fill.
-        let ret = unsafe { bindings::pwm_get_waveform_might_sleep(self.as_raw(), &mut c_wf) };
-
-        to_result(ret)?;
+        to_result(unsafe { bindings::pwm_get_waveform_might_sleep(self.as_raw(), &mut c_wf) })?;
 
         Ok(Waveform::from(c_wf))
     }
@@ -263,8 +260,8 @@ unsafe fn serialize_wfhw(wfhw: &T::WfHw, wfhw_ptr: *mut c_void) -> Result {
                 core::ptr::from_ref::<T::WfHw>(wfhw).cast::<u8>(),
                 wfhw_ptr.cast::<u8>(),
                 size,
-            );
-        }
+            )
+        };
 
         Ok(())
     }
@@ -284,8 +281,8 @@ unsafe fn deserialize_wfhw(wfhw_ptr: *const c_void) -> Result<T::WfHw> {
                 wfhw_ptr.cast::<u8>(),
                 core::ptr::from_mut::<T::WfHw>(&mut wfhw).cast::<u8>(),
                 size,
-            );
-        }
+            )
+        };
 
         Ok(wfhw)
     }
@@ -311,9 +308,7 @@ unsafe fn deserialize_wfhw(wfhw_ptr: *const c_void) -> Result<T::WfHw> {
         // Now, call the original release function to free the `pwm_chip` itself.
         // SAFETY: `dev` is the valid pointer passed into this callback, which is
         // the expected argument for `pwmchip_release`.
-        unsafe {
-            bindings::pwmchip_release(dev);
-        }
+        unsafe { bindings::pwmchip_release(dev) };
     }
 
     /// # Safety
@@ -413,9 +408,7 @@ unsafe fn deserialize_wfhw(wfhw_ptr: *const c_void) -> Result<T::WfHw> {
         match T::round_waveform_fromhw(chip, pwm, &wfhw, &mut rust_wf) {
             Ok(()) => {
                 // SAFETY: `wf_ptr` is guaranteed valid by the C caller.
-                unsafe {
-                    *wf_ptr = rust_wf.into();
-                };
+                unsafe { *wf_ptr = rust_wf.into() };
                 0
             }
             Err(e) => e.to_errno(),
@@ -614,16 +607,12 @@ pub fn new<'a>(
         })?;
 
         // SAFETY: `c_chip_ptr` points to a valid chip.
-        unsafe {
-            (*c_chip_ptr).dev.release = Some(Adapter::<T>::release_callback);
-        }
+        unsafe { (*c_chip_ptr).dev.release = Some(Adapter::<T>::release_callback) };
 
         // SAFETY: `c_chip_ptr` points to a valid chip.
         // The `Adapter`'s `VTABLE` has a 'static lifetime, so the pointer
         // returned by `as_raw()` is always valid.
-        unsafe {
-            (*c_chip_ptr).ops = Adapter::<T>::VTABLE.as_raw();
-        }
+        unsafe { (*c_chip_ptr).ops = Adapter::<T>::VTABLE.as_raw() };
 
         // Cast the `*mut bindings::pwm_chip` to `*mut Chip`. This is valid because
         // `Chip` is `repr(transparent)` over `Opaque<bindings::pwm_chip>`, and
@@ -645,9 +634,7 @@ unsafe impl<T: PwmOps> AlwaysRefCounted for Chip<T> {
     fn inc_ref(&self) {
         // SAFETY: `self.0.get()` points to a valid `pwm_chip` because `self` exists.
         // The embedded `dev` is valid. `get_device` increments its refcount.
-        unsafe {
-            bindings::get_device(&raw mut (*self.0.get()).dev);
-        }
+        unsafe { bindings::get_device(&raw mut (*self.0.get()).dev) };
     }
 
     #[inline]
@@ -656,9 +643,7 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
 
         // SAFETY: `obj` is a valid pointer to a `Chip` (and thus `bindings::pwm_chip`)
         // with a non-zero refcount. `put_device` handles decrement and final release.
-        unsafe {
-            bindings::put_device(&raw mut (*c_chip_ptr).dev);
-        }
+        unsafe { bindings::put_device(&raw mut (*c_chip_ptr).dev) };
     }
 }
 
@@ -692,9 +677,7 @@ pub fn register(self) -> Result<ARef<Chip<T>>> {
 
         // SAFETY: `c_chip_ptr` points to a valid chip with its ops initialized.
         // `__pwmchip_add` is the C function to register the chip with the PWM core.
-        unsafe {
-            to_result(bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()))?;
-        }
+        to_result(unsafe { bindings::__pwmchip_add(c_chip_ptr, core::ptr::null_mut()) })?;
 
         let registration = Registration {
             chip: ARef::clone(&self.chip),
@@ -730,9 +713,7 @@ fn drop(&mut self) {
         // SAFETY: `chip_raw` points to a chip that was successfully registered.
         // `bindings::pwmchip_remove` is the correct C function to unregister it.
         // This `drop` implementation is called automatically by `devres` on driver unbind.
-        unsafe {
-            bindings::pwmchip_remove(chip_raw);
-        }
+        unsafe { bindings::pwmchip_remove(chip_raw) };
     }
 }
 

-- 
2.43.0

Re: [PATCH v2 2/2] rust: pwm: Simplify to_result call sites and unsafe blocks

[Michal Wilczynski]

On 1/2/26 08:51, Kari Argillander wrote:
> Remove unnecessary temporary variables around to_result() calls and move
> trailing semicolons outside unsafe blocks to improve readability and
> produce cleaner rustfmt output.
> 
> No functional change intended.
> 
> Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> ---
>  rust/kernel/pwm.rs | 47 ++++++++++++++---------------------------------
>  1 file changed, 14 insertions(+), 33 deletions(-)
> 

Hi, the changes look fine to me, they are more idiomatic to Rust.

Acked-by: Michal Wilczynski <m.wilczynski@samsung.com>

Re: [PATCH v2 0/2] rust: pwm: Fix init error handling and tidy style

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

On Fri, Jan 02, 2026 at 09:51:40AM +0200, Kari Argillander wrote:
> This series contains two small updates to the Rust PWM bindings.
> 
> The first patch fixes a potential memory leak on an error path during PWM
> chip initialization. Someone needs to decide if this goes to stable.
> 
> The second patch is just style-only cleanup.
> 
> Signed-off-by: Kari Argillander <kari.argillander@gmail.com>

I applied these with Michal's Ack to
	https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next
.

For now I don't intend to send the first patch to Linus for 6.19 and
queued both for the next merge window. Please convince me otherwise if
you think this to be wrong.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]