Re: [PATCH v2] leds: flash: sgm3140: fix child node reference leak

[Sakari Ailus]

Hi Guangshuo,

On Mon, Sep 21, 2026 at 05:21:28PM +0800, Guangshuo Li wrote:
> sgm3140_probe() obtains a reference to the LED child node with
> device_get_next_child_node(), but the reference is not released after a
> successful probe.
> 
> The child node is also passed to devm_led_classdev_flash_register_ext().
> The LED class device stores the fwnode without taking a reference of its
> own, so the reference obtained by sgm3140_probe() must remain valid until
> the LED class device is unregistered.
> 
> Manage the child node reference with a devm action registered before the
> LED class device. This ensures that the LED class device is unregistered
> before the child node reference is dropped, while also handling probe
> failure and device removal.

This doesn't look like a problem related to just this driver.

led_classdev_register_ext() creates a device and sets its fwnode without
taking a reference. Should we instead take a refenrece there and release it
at unregister time?

> 
> Fixes: cef8ec8cbd21 ("leds: add sgm3140 driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> ---
> v2:
>   - Keep the child node reference alive until the LED class device is
>     unregistered, as pointed out by Laurent Pinchart.
>   - Manage the reference with a devm action registered before the LED
>     class device registration.
> 
>  drivers/leds/flash/leds-sgm3140.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/leds/flash/leds-sgm3140.c b/drivers/leds/flash/leds-sgm3140.c
> index dc6840357370..973676b1dbb1 100644
> --- a/drivers/leds/flash/leds-sgm3140.c
> +++ b/drivers/leds/flash/leds-sgm3140.c
> @@ -182,6 +182,11 @@ static void sgm3140_init_v4l2_flash_config(struct sgm3140 *priv,
>  }
>  #endif
>  
> +static void sgm3140_fwnode_put(void *data)
> +{
> +	fwnode_handle_put(data);
> +}
> +
>  static int sgm3140_probe(struct platform_device *pdev)
>  {
>  	struct sgm3140 *priv;
> @@ -220,6 +225,10 @@ static int sgm3140_probe(struct platform_device *pdev)
>  			"No fwnode child node found for connected LED.\n");
>  		return -EINVAL;
>  	}
> +	ret = devm_add_action_or_reset(&pdev->dev, sgm3140_fwnode_put,
> +				       child_node);
> +	if (ret)
> +		return ret;
>  
>  	ret = fwnode_property_read_u32(child_node, "flash-max-timeout-us",
>  				       &priv->max_timeout);
> @@ -276,7 +285,6 @@ static int sgm3140_probe(struct platform_device *pdev)
>  	return ret;
>  
>  err:
> -	fwnode_handle_put(child_node);
>  	return ret;
>  }
>  

-- 
Regards,

Sakari Ailus