Re: [PATCH v4 1/4] leds: trigger: netdev: Put netdev on activate error path

[Lee Jones]

On Thu, 01 Oct 2026, A. Sverdlin wrote:

> From: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxxxx>
>
> When hw control is active, set_device_name() takes a reference on the
> netdev via dev_get_by_name(). The register_netdevice_notifier() failure
> path frees trigger_data without dropping it, leaking the netdev.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/all/20260914140045.B8F4C1F000FF@xxxxxxxxxxxxxxx/
> Fixes: 0316cc5629d1 ("leds: trigger: netdev: init mode if hw control already active")
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxxxx>
> ---
> Changelog:
> v3:
> - introduced unset_device_name() as a counterpart of set_device_name()
>
>  drivers/leds/trigger/ledtrig-netdev.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
> index 5b0132484594c..e2a6c95a0dc97 100644
> --- a/drivers/leds/trigger/ledtrig-netdev.c
> +++ b/drivers/leds/trigger/ledtrig-netdev.c
> @@ -324,6 +324,13 @@ static int set_device_name(struct led_netdev_data *trigger_data,
>  	return 0;
>  }
>
> +static void unset_device_name(struct led_netdev_data *trigger_data)
> +{
> +	dev_put(trigger_data->net_dev);
> +	trigger_data->net_dev = NULL;
> +	trigger_data->device_name[0] = 0;
> +}

Given that both callers immediately 'kfree(trigger_data)' after calling
this, why bother clearing these at all?  Wouldn't calling 'dev_put()'
directly in 'netdev_trig_activate()' be simpler than introducing a
helper here?

> +
>  static ssize_t device_name_store(struct device *dev,
>  				 struct device_attribute *attr, const char *buf,
>  				 size_t size)
> @@ -776,8 +783,10 @@ static int netdev_trig_activate(struct led_classdev *led_cdev)
>  	led_set_trigger_data(led_cdev, trigger_data);
>
>  	rc = register_netdevice_notifier(&trigger_data->notifier);
> -	if (rc)
> +	if (rc) {
> +		unset_device_name(trigger_data);
>  		kfree(trigger_data);
> +	}
>
>  	return rc;
>  }
> @@ -790,7 +799,7 @@ static void netdev_trig_deactivate(struct led_classdev *led_cdev)
>
>  	cancel_delayed_work_sync(&trigger_data->work);
>
> -	dev_put(trigger_data->net_dev);
> +	unset_device_name(trigger_data);
>
>  	kfree(trigger_data);
>  }
> -- 
> 2.55.0
>

-- 
Lee Jones