Re: [PATCH net-next v8 3/7] net: pse-pd: unwind allocations when controller registration fails

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: pse-pd: TOCTOU race and NULL pointer dereference via unguarded pcdev->pi[i].pw_d assignment
--

commit 8c5741051355bc530c2efc50d6e4e9ebe8e283f3
Author: Carlo Szelinsky <github@szelinsky.de>

net: pse-pd: unwind allocations when controller registration fails

This commit unwinds allocations when pse_controller_register() fails. It
updates the error paths to properly free the notification kfifo, flush
the power domains, and clear the power domain pointers to NULL to avoid
dangling references.

> diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
> --- a/drivers/net/pse-pd/pse_core.c
> +++ b/drivers/net/pse-pd/pse_core.c
> @@ -937,11 +937,21 @@ static void pse_flush_pw_ds(struct pse_controller_dev *pcdev)
>  			continue;
>  
>  		pw_d = xa_load(&pse_pw_d_map, pcdev->pi[i].pw_d->id);
> -		if (!pw_d)
> +		if (!pw_d) {
> +			pcdev->pi[i].pw_d = NULL;
>  			continue;
> +		}
>  
>  		kref_put_mutex(&pw_d->refcnt, __pse_pw_d_release,
>  			       &pse_pw_d_mutex);
> +		/* The pw_d is devm memory of whichever controller created
> +		 * it, so it can go away as soon as that probe unwinds.
> +		 * Nothing may be left pointing at it: pse_pi_is_enabled()
> +		 * reaches pi[].pw_d from the regulator "state" attribute,
> +		 * which stays readable until the PI regulators are
> +		 * unregistered after us.
> +		 */
> +		pcdev->pi[i].pw_d = NULL;

[Severity: High]
Does setting pcdev->pi[i].pw_d to NULL here without holding pcdev->lock
introduce a race condition with concurrent sysfs accesses?

Since the PI regulators are registered via devm_pse_pi_regulator_register()
and accessible via sysfs before pse_flush_pw_ds() is called in the error
path, a concurrent read of the regulator attributes could invoke regulator
operations like pse_pi_is_enabled(). These operations take pcdev->lock and
can call into pse_pi_allocate_pw_budget().

In pse_pi_allocate_pw_budget(), pi->pw_d is evaluated twice without READ_ONCE:

    if (!pi->pw_d)
        return 0;

    /* PSE_BUDGET_EVAL_STRAT_STATIC */
    if (pi->pw_d->budget_eval_strategy == PSE_BUDGET_EVAL_STRAT_STATIC)

If pcdev->pi[i].pw_d is set to NULL concurrently between the null check and
the dereference, could this cause a TOCTOU NULL pointer dereference?

>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004164219.1161294-1-github@szelinsky.de?part=3