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

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

[Guangshuo Li]

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.

Fixes: cef8ec8cbd21 ("leds: add sgm3140 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
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;
 }
 
-- 
2.43.0

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@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> 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

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

[Laurent Pinchart]

Why did you submit a v2 ignoring my latest comments on v1 ?

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.
> 
> Fixes: cef8ec8cbd21 ("leds: add sgm3140 driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> 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,

Laurent Pinchart