[PATCH] drm/nouveau/acr: fix missing nvkm_done() in error path of nvkm_acr_oneinit()

[PATCH] drm/nouveau/acr: fix missing nvkm_done() in error path of nvkm_acr_oneinit()

[Wentao Liang]

In nvkm_acr_oneinit(), nvkm_kmap(acr->wpr) is invoked unconditionally
at line 309 to obtain a mapping reference. Additionally, when both
acr->wpr_fw and acr->wpr_comp are present, a second nvkm_kmap() is
called inside the conditional block. Both mappings are expected to be
released by nvkm_done(acr->wpr) at line 320 before the function returns
successfully.

However, when a mismatch is detected during the loop within the
conditional block, the function returns -EINVAL at line 318 without
calling nvkm_done(). This results in a leak of the kmap reference(s)
acquired earlier.

Fix the issue by invoking nvkm_done(acr->wpr) prior to the early return
to ensure proper release of the mapping references.

Fixes: 22dcda45a3d1 ("drm/nouveau/acr: implement new subdev to replace "secure boot"")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/acr/base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/base.c
index 4c7745cd6ae5..7fd967a2554f 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/base.c
@@ -315,6 +315,7 @@ nvkm_acr_oneinit(struct nvkm_subdev *subdev)
 					  i, us, fw);
 			}
 		}
+		nvkm_done(acr->wpr);
 		return -EINVAL;
 	}
 	nvkm_done(acr->wpr);
-- 
2.34.1

Re: [PATCH] drm/nouveau/acr: fix missing nvkm_done() in error path of nvkm_acr_oneinit()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [Medium] Incomplete fix of memory mapping reference leak. The patch adds one `nvkm_done(acr->wpr)` before returning `-EINVAL`, but `nvkm_kmap(acr->wpr)` was called twice on this code path.
- [Medium] The commit message demonstrates a misunderstanding of the control flow, stating that the function returns `-EINVAL` only when a mismatch is detected.
- [Medium] The return values of `acr->func->wpr_build()` and `acr->func->wpr_patch()` are ignored, allowing initialization to proceed with corrupted firmware data.
- [Medium] The WPR comparison loop can read out of bounds of the `acr->wpr` memory object and the firmware data blob.
--

commit 018071a64ea846382bbfa2aefceef09269c67376
Author: Wentao Liang <vulab@iscas.ac.cn>

drm/nouveau/acr: fix missing nvkm_done() in error path of nvkm_acr_oneinit()

This commit attempts to fix a memory mapping reference leak in
nvkm_acr_oneinit() by adding a missing nvkm_done() call in the error
path during WPR comparison.

>     However, when a mismatch is detected during the loop within the
>     conditional block, the function returns -EINVAL at line 318 without
>     calling nvkm_done(). This results in a leak of the kmap reference(s)
>     acquired earlier.

[Severity: Medium]
This isn't a bug introduced by this patch, but the commit message states the
function returns -EINVAL when a mismatch is detected.

Looking at nvkm_acr_oneinit(), the return statement is unconditional for the
entire block and happens regardless of whether a mismatch is detected:

	if (acr->wpr_fw && acr->wpr_comp) {
		nvkm_kmap(acr->wpr);
		for (i = 0; i < acr->wpr_fw->size; i += 4) {
			...
			if (fw != us) {
				nvkm_warn(...);
			}
		}
		nvkm_done(acr->wpr);
		return -EINVAL;
	}

Is this unconditional return intended?

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/base.c
> index 4c7745cd6ae52..7fd967a2554f9 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/acr/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/acr/base.c
> @@ -315,6 +315,7 @@ nvkm_acr_oneinit(struct nvkm_subdev *subdev)
>  					  i, us, fw);
>  			}
>  		}

[Severity: Medium]
This is a pre-existing issue, but could this loop read out-of-bounds of the
acr->wpr memory object and the firmware data blob?

If the user provides a firmware file larger than the dynamically calculated
wpr_size, there doesn't appear to be a check to ensure
acr->wpr_fw->size <= wpr_size before reading nvkm_ro32(acr->wpr, i).

Also, if acr->wpr_fw->size is not a multiple of 4, will the loop increment
cause the final iteration to read out-of-bounds?

[Severity: Medium]
This is a pre-existing issue, but does ignoring the return values from
wpr_build() and wpr_patch() earlier in this function allow initialization
to proceed with corrupted firmware data?

	if (!acr->wpr_fw || acr->wpr_comp)
		acr->func->wpr_build(acr, rtos);
	acr->func->wpr_patch(acr, (s64)acr->wpr_start - acr->wpr_prev);

If either function fails, could the system continue initializing with an
incomplete or corrupted WPR image?

> +		nvkm_done(acr->wpr);
>  		return -EINVAL;

[Severity: Medium]
This is a pre-existing issue, but does this error path still leak a mapping
reference?

In nvkm_acr_oneinit(), nvkm_kmap(acr->wpr) is called unconditionally before
the wpr_build() call:

	nvkm_kmap(acr->wpr);
	if (acr->wpr_fw && !acr->wpr_comp)
		nvkm_wobj(acr->wpr, 0, acr->wpr_fw->data, acr->wpr_fw->size);

It is then called a second time within this wpr_comp block.

Since the reference count is incremented twice, does it need to be
decremented twice by nvkm_done() before returning -EINVAL here?

>  	}
>  	nvkm_done(acr->wpr);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260606155606.77593-1-vulab@iscas.ac.cn?part=1

Re: [PATCH] drm/nouveau/acr: fix missing nvkm_done() in error path of nvkm_acr_oneinit()

[Danilo Krummrich]

On Sat,  6 Jun 2026 15:56:06 +0000, Wentao Liang wrote:
> [PATCH] drm/nouveau/acr: fix missing nvkm_done() in error path of nvkm_acr_oneinit()

Applied, thanks!

  Branch: drm-misc-fixes
  Tree:   https://gitlab.freedesktop.org/drm/misc/kernel.git

[1/1] drm/nouveau/acr: fix missing nvkm_done() in error path of nvkm_acr_oneinit()
      commit: c3027973f692

The patch will appear in the next linux-next integration (typically within 24
hours on weekdays).

The patch is queued up for Linus's tree and should land in the next -rc release.

Re: [PATCH] drm/nouveau/acr: fix missing nvkm_done() in error path of nvkm_acr_oneinit()

[Danilo Krummrich]

On Sat,  6 Jun 2026 15:56:06 +0000, Wentao Liang wrote:
> [PATCH] drm/nouveau/acr: fix missing nvkm_done() in error path of nvkm_acr_oneinit()

Applied, thanks!

  Branch: drm-misc-fixes
  Tree:   https://gitlab.freedesktop.org/drm/misc/kernel.git

[1/1] drm/nouveau/acr: fix missing nvkm_done() in error path of nvkm_acr_oneinit()
      commit: c3027973f692

The patch will appear in the next linux-next integration (typically within 24
hours on weekdays).

The patch is queued up for Linus's tree and should land in the next -rc release.