* [PATCH 1/2] rust: pwm: Fix potential memory leak on init error
2025-12-19 17:02 [PATCH 0/2] rust: pwm: Fix init error handling and tidy style Kari Argillander
@ 2025-12-19 17:02 ` Kari Argillander
2025-12-19 17:02 ` [PATCH 2/2] rust: pwm: Simplify to_result call sites and unsafe blocks Kari Argillander
1 sibling, 0 replies; 3+ messages in thread
From: Kari Argillander @ 2025-12-19 17:02 UTC (permalink / raw)
To: Michal Wilczynski, Uwe Kleine-König, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich
Cc: linux-pwm, rust-for-linux, linux-kernel, 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 returned without calling pwmchip_put().
While __pinned_init() does not currently have any failure paths in code
base, this change makes the ownership transfer explicit and keeps the
code correct if failure paths are added in the future.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
rust/kernel/pwm.rs | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs
index 2dd72a39acb5..4b6a3b5ef929 100644
--- a/rust/kernel/pwm.rs
+++ b/rust/kernel/pwm.rs
@@ -607,7 +607,12 @@ 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())? };
+ if let Err(e) = unsafe { data.__pinned_init(drvdata_ptr.cast()) } {
+ // SAFETY: It is safe to call `pwmchip_put()` with a valid pointer
+ // obtained from `pwmchip_alloc()`.
+ unsafe { bindings::pwmchip_put(c_chip_ptr) };
+ return Err(e);
+ }
// SAFETY: `c_chip_ptr` points to a valid chip.
unsafe {
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH 2/2] rust: pwm: Simplify to_result call sites and unsafe blocks
2025-12-19 17:02 [PATCH 0/2] rust: pwm: Fix init error handling and tidy style Kari Argillander
2025-12-19 17:02 ` [PATCH 1/2] rust: pwm: Fix potential memory leak on init error Kari Argillander
@ 2025-12-19 17:02 ` Kari Argillander
1 sibling, 0 replies; 3+ messages in thread
From: Kari Argillander @ 2025-12-19 17:02 UTC (permalink / raw)
To: Michal Wilczynski, Uwe Kleine-König, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich
Cc: linux-pwm, rust-for-linux, linux-kernel, 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 4b6a3b5ef929..3b6c72dcbd39 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(),
@@ -615,16 +608,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
@@ -646,9 +635,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]
@@ -657,9 +644,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) };
}
}
@@ -693,9 +678,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),
@@ -731,9 +714,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
^ permalink raw reply [flat|nested] 3+ messages in thread