mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Francesco Magazzu <postadelmaga@gmail.com>
To: Lyude Paul <lyude@redhat.com>, Danilo Krummrich <dakr@kernel.org>
Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, Dan Carpenter <error27@gmail.com>,
	Karol Herbst <kherbst@redhat.com>
Subject: [PATCH v3 0/4] drm/nouveau: fix list cursor use after loop in the clk pstate paths
Date: Fri, 18 Sep 2026 15:16:16 +0200	[thread overview]
Message-ID: <20260918131620.405133-1-postadelmaga@gmail.com> (raw)

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
(dan.carpenter@oracle.com); .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 error27@gmail.com 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/20260727152821.128432-1-postadelmaga@gmail.com/

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/20260712123616.1180830-1-postadelmaga@gmail.com/
Link to Lyude's review of patch 1:
https://lore.kernel.org/dri-devel/ad023b5df1495ced25aa681d10f57628df41ac13.camel@redhat.com/
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


             reply	other threads:[~2026-09-18 13:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18 13:16 Francesco Magazzu [this message]
2026-09-18 13:16 ` [PATCH v3 1/4] drm/nouveau/clk: fix list cursor use after loop in nvkm_clk_ustate_update Francesco Magazzu
2026-09-18 18:05   ` lyude
2026-09-18 13:16 ` [PATCH v3 2/4] drm/nouveau/clk: don't use the pstate cursor after the loop Francesco Magazzu
2026-09-18 13:16 ` [PATCH v3 3/4] drm/nouveau/device: " Francesco Magazzu
2026-09-18 13:16 ` [PATCH v3 4/4] drm/nouveau/clk: don't clobber reclock status when restoring volt/fan Francesco Magazzu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260918131620.405133-1-postadelmaga@gmail.com \
    --to=postadelmaga@gmail.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=error27@gmail.com \
    --cc=kherbst@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lyude@redhat.com \
    --cc=nouveau@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®