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

[Michal Wilczynski <m.wilczynski@samsung.com>]

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>