Re: [PATCH net 02/10] net: pse-pd: fix module refcount leak in pse_control_get_internal()

[Kory Maincent]

On 10/18/26 10:23, Carlo Szelinsky wrote:
> pse_control_get_internal() takes a module reference and then has two
> error paths that skip module_put(): a controller without
> pi_get_admin_state(), and a failing pse_pi_is_hw_enabled(). The second
> one happens on any I2C or UART error. Each time, one more reference
> leaks, and the PSE driver can no longer be unloaded.
>
> Send both to put_module, like the regulator_get_exclusive() error path.
>
> Fixes: ffef61d6d273 ("net: pse-pd: Add support for budget evaluation strategies")
> Signed-off-by: Carlo Szelinsky <github@xxxxxxxxxxxx>

As spotted by sashiko you can remove the two checks on 
pi_get_admin_state callback as it is already done in the 
pse_controller_register() function.
Maybe you can do it in a patch before this one even if it is not a fix.
With this change:

Reviewed-by: Kory Maincent <kory.maincent@xxxxxxxxxxx>

Thank you!

> ---
>   drivers/net/pse-pd/pse_core.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
> index c0573cbbba5b..1710f9bb7d27 100644
> --- a/drivers/net/pse-pd/pse_core.c
> +++ b/drivers/net/pse-pd/pse_core.c
> @@ -1427,7 +1427,7 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
>   
>   	if (!pcdev->ops->pi_get_admin_state) {
>   		ret = -EOPNOTSUPP;
> -		goto free_psec;
> +		goto put_module;
>   	}
>   
>   	/* Initialize admin_state_enabled before the regulator_get. This
> @@ -1436,7 +1436,7 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
>   	 */
>   	ret = pse_pi_is_hw_enabled(pcdev, index);
>   	if (ret < 0)
> -		goto free_psec;
> +		goto put_module;
>   
>   	pcdev->pi[index].admin_state_enabled = ret;
>   	psec->ps = regulator_get_exclusive(pcdev->dev,

--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com