Reviewed-by: Lyude Paul <[email protected]> On Sun, 2026-07-12 at 14:36 +0200, Francesco Magazzu wrote: > nvkm_control_mthd_pstate_attr() looks up the pstate at the index > supplied > by userspace by walking clk->states, and then keeps using the > list_for_each_entry cursor after the loop. This is not triggerable > today: > the function already rejects args->v0.state >= clk->state_nr before > the > loop, and clk->state_nr is kept in sync with the number of entries on > clk->states, so the lookup always breaks on a real entry. > > Should the loop ever run to completion, the cursor would point at the > list > head rather than at a pstate, and the pstate->base.domain[] read and > the > walk of pstate->list that follow would read past it. Rather than > leave > that trap in place, track whether the entry was found and return - > EINVAL if > it was not, like the other lookup failures in this function. > > No functional change. > > Signed-off-by: Francesco Magazzu <[email protected]> > --- > drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c > b/drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c > index f2e9a0626..28702741a 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/ctrl.c > @@ -74,6 +74,7 @@ nvkm_control_mthd_pstate_attr(struct nvkm_control > *ctrl, void *data, u32 size) > const struct nvkm_domain *domain; > struct nvkm_pstate *pstate; > struct nvkm_cstate *cstate; > + bool found = false; > int i = 0, j = -1; > u32 lo, hi; > int ret = -ENOSYS; > @@ -104,10 +105,15 @@ nvkm_control_mthd_pstate_attr(struct > nvkm_control *ctrl, void *data, u32 size) > > if (args->v0.state != > NVIF_CONTROL_PSTATE_ATTR_V0_STATE_CURRENT) { > list_for_each_entry(pstate, &clk->states, head) { > - if (i++ == args->v0.state) > + if (i++ == args->v0.state) { > + found = true; > break; > + } > } > > + if (!found) > + return -EINVAL; > + > lo = pstate->base.domain[domain->name]; > hi = lo; > list_for_each_entry(cstate, &pstate->list, head) {
