public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/nouveau: fix nvkm_device leak on aperture removal failure
@ 2026-04-11  6:29 David Carlier
  2026-04-11 23:12 ` Claude review: " Claude Code Review Bot
  2026-04-11 23:12 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: David Carlier @ 2026-04-11  6:29 UTC (permalink / raw)
  To: dakr, lyude
  Cc: mripard, maarten.lankhorst, tzimmermann, airlied, simona, bskeggs,
	dri-devel, nouveau, stable, linux-kernel, David Carlier

When aperture_remove_conflicting_pci_devices() fails during probe, the
error path returns directly without unwinding the nvkm_device that was
just allocated by nvkm_device_pci_new(). This leaks both the device
wrapper and the pci_enable_device() reference taken inside it.

Jump to the existing fail_nvkm label so nvkm_device_del() runs and
balances both. The leak was introduced when the intermediate
nvkm_device_del() between detection and aperture removal was dropped
in favor of creating the pci device once.

Fixes: c0bfe34330b5 ("drm/nouveau: create pci device once")
Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 5d8475e4895e..517ff2c31dce 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -875,7 +875,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
 	/* Remove conflicting drivers (vesafb, efifb etc). */
 	ret = aperture_remove_conflicting_pci_devices(pdev, driver_pci.name);
 	if (ret)
-		return ret;
+		goto fail_nvkm;
 
 	pci_set_master(pdev);
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Claude review: drm/nouveau: fix nvkm_device leak on aperture removal failure
  2026-04-11  6:29 [PATCH] drm/nouveau: fix nvkm_device leak on aperture removal failure David Carlier
  2026-04-11 23:12 ` Claude review: " Claude Code Review Bot
@ 2026-04-11 23:12 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-11 23:12 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/nouveau: fix nvkm_device leak on aperture removal failure
Author: David Carlier <devnexen@gmail.com>
Patches: 1
Reviewed: 2026-04-12T09:12:26.635838

---

This is a single-patch series fixing a resource leak in `nouveau_drm_probe()`. The analysis and fix are correct. When `aperture_remove_conflicting_pci_devices()` fails, the code previously returned directly, leaking the `nvkm_device` allocated by `nvkm_device_pci_new()` and the `pci_enable_device()` reference it takes internally. The fix properly routes to the existing `fail_nvkm` error label.

The patch is minimal, targeted, and appropriate for stable backport.

**Verdict: Looks good.**

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Claude review: drm/nouveau: fix nvkm_device leak on aperture removal failure
  2026-04-11  6:29 [PATCH] drm/nouveau: fix nvkm_device leak on aperture removal failure David Carlier
@ 2026-04-11 23:12 ` Claude Code Review Bot
  2026-04-11 23:12 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-11 23:12 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Confirmed.**

The probe function allocates the nvkm device early:

```c
ret = nvkm_device_pci_new(pdev, nouveau_config, nouveau_debug, &device);
if (ret)
    return ret;
```

`nvkm_device_pci_new()` internally calls `pci_enable_device()` (at `nvkm/engine/device/pci.c:1662`), so on success we hold both the device allocation and a PCI enable reference.

The very next fallible call is:

```c
ret = aperture_remove_conflicting_pci_devices(pdev, driver_pci.name);
if (ret)
-       return ret;
+       goto fail_nvkm;
```

The old `return ret` skips cleanup entirely. The `fail_nvkm` label correctly runs:

```c
fail_nvkm:
    nvkm_device_del(&device);
    return ret;
```

And `nvkm_device_del` invokes `nvkm_device_pci_dtor()` which calls `pci_disable_device()` (`pci.c:1635`), properly balancing the enable taken during `nvkm_device_pci_new()`.

There is no double-free risk here: at this point in the probe sequence, no other resources (the second `pci_enable_device` at line 891, `nouveau_drm_device_new`, etc.) have been allocated yet, so jumping to `fail_nvkm` — which only tears down the nvkm device — is the correct label.

**Commit message:** Accurate. The description correctly identifies what leaks, why, and which commit introduced the problem.

**Tags:** `Fixes:` and `Cc: stable` are appropriate.

**No issues found.** This could take a:

Reviewed-by if you agree with the analysis.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-11 23:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-11  6:29 [PATCH] drm/nouveau: fix nvkm_device leak on aperture removal failure David Carlier
2026-04-11 23:12 ` Claude review: " Claude Code Review Bot
2026-04-11 23:12 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox