This series deals with the three places in the nouveau clk pstate code where the list_for_each_entry() cursor is used after the loop, plus one unrelated fix. They are exactly the three sites Dan Carpenter listed in 2022.
The only change in v3 is the one Lyude asked for: patch 1 is Dan's original patch again, with his authorship and his Signed-off-by restored. The code is unchanged from what he posted; I only expanded the commit message and noted that below the ---. His Signed-off-by is kept with the address he signed it with in 2022 ([email protected]); .mailmap already maps it to the address he uses today, and I did not want to rewrite someone else's tag. Say the word if you would rather have [email protected] in there. Dan is Cc'd at that address this time - the v2 went to his old linaro.org one, which is dead, so he most likely never saw it. Patches 2, 3 and 4 are unchanged from v2 and carry Lyude's Reviewed-by. Only patch 1 fixes a bug that can actually be triggered: nvkm_clk_ustate_update() takes an arbitrary pstate id from the user and never checks that a matching entry exists, so if it does not the cursor ends up pointing at the list head and the code reads past it. Patches 2 and 3 fix the same pattern in nvkm_pstate_prog() and nvkm_control_mthd_pstate_attr(), but neither is triggerable as the code stands: the callers of nvkm_pstate_prog() clamp the index against clk->state_nr first, and nvkm_control_mthd_pstate_attr() already rejects args->v0.state >= clk->state_nr before the loop. Both are hardening, not bug fixes, and they carry no Fixes: tag on purpose. The point is to stop the two functions from being correct only by virtue of what their callers do. Patch 4 is unrelated and is a real bug, though a modest one: nvkm_cstate_prog() overwrites the reclock status in 'ret' with the status of the voltage/fan restore calls it makes afterwards. The only consumer of the return value is an error message in nvkm_pstate_work(), so the observable effect is that a failing reclock is never reported in dmesg. Compile-tested only. All the affected paths are reachable only by root, through the 'pstate' debugfs file, so I could not exercise them in any other way. The two follow-up patches for nvkm_pstate_prog() that Lyude also reviewed apply on top of this series and are not resent here: https://lore.kernel.org/dri-devel/[email protected]/ v3: restore Dan Carpenter's authorship on patch 1 (Lyude), collect the Reviewed-by tags, rebase on drm-misc-next. Link to v2: https://lore.kernel.org/dri-devel/[email protected]/ Link to Lyude's review of patch 1: https://lore.kernel.org/dri-devel/[email protected]/ Link to Dan's original patch: https://lore.kernel.org/dri-devel/YvSkKAdk8Pe0g2K9@kili/ Dan Carpenter (1): drm/nouveau/clk: fix list cursor use after loop in nvkm_clk_ustate_update Francesco Magazzu (3): drm/nouveau/clk: don't use the pstate cursor after the loop drm/nouveau/device: don't use the pstate cursor after the loop drm/nouveau/clk: don't clobber reclock status when restoring volt/fan .../gpu/drm/nouveau/nvkm/engine/device/ctrl.c | 8 ++++- .../gpu/drm/nouveau/nvkm/subdev/clk/base.c | 31 +++++++++++++------ 2 files changed, 28 insertions(+), 11 deletions(-) base-commit: bc47d5937f21c5fc94504f03e18f1adb56d97634 -- 2.55.0
