Re: [PATCH v2 2/4] drm/nouveau/clk: don't use the pstate cursor after the loop

[lyude]

Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>

On Sun, 2026-07-12 at 14:36 +0200, Francesco Magazzu wrote:
> nvkm_pstate_prog() walks clk->states looking for the entry at index
> 'pstatei' and then keeps using the list_for_each_entry cursor after
> the
> loop.  This is not triggerable today: every caller clamps the index
> against clk->state_nr before calling, so the loop always breaks on a
> real
> entry.  It is safe by virtue of what the callers happen to do, not by
> anything the function itself checks.
> 
> Should a caller ever pass an index that is not on the list, the
> cursor
> would point at the list head rather than at a pstate, and the
> pstate->base.domain[] and pstate->fanspeed accesses that follow would
> read
> past it.  Rather than leave that trap in place for the next caller,
> track
> whether the entry was found and return -EINVAL if it was not.
> 
> No functional change.
> 
> Signed-off-by: Francesco Magazzu <postadelmaga@xxxxxxxxx>
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> index 42f3709e0..4d546b07f 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> @@ -270,13 +270,19 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int
> pstatei)
>  	struct nvkm_fb *fb = subdev->device->fb;
>  	struct nvkm_pci *pci = subdev->device->pci;
>  	struct nvkm_pstate *pstate;
> +	bool found = false;
>  	int ret, idx = 0;
>  
>  	list_for_each_entry(pstate, &clk->states, head) {
> -		if (idx++ == pstatei)
> +		if (idx++ == pstatei) {
> +			found = true;
>  			break;
> +		}
>  	}
>  
> +	if (!found)
> +		return -EINVAL;
> +
>  	nvkm_debug(subdev, "setting performance state %d\n",
> pstatei);
>  	clk->pstate = pstatei;
>