public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v1] fbdev/offb: fix PCI device reference leak on probe failure
@ 2026-04-20  1:01 Yuho Choi
  2026-04-23  0:29 ` Claude review: " Claude Code Review Bot
  2026-04-23  0:29 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Yuho Choi @ 2026-04-20  1:01 UTC (permalink / raw)
  To: Helge Deller, Jason Donenfeld, linux-fbdev
  Cc: dri-devel, linux-kernel, Myeonghun Pak, Ijae Kim, Taegyu Kim,
	Yuho Choi

offb_init_nodriver() gets a referenced PCI device with
pci_get_device(). If pci_enable_device() fails, the function returns
without dropping that reference.

Release the PCI device reference before returning from the
pci_enable_device() failure path.

Fixes: 5bda8f7b5468 ("video: fbdev: offb: Call pci_enable_device() before using the PCI VGA device")
Co-developed-by: Myeonghun Pak <mhun512@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Co-developed-by: Taegyu Kim <tmk5904@psu.edu>
Signed-off-by: Taegyu Kim <tmk5904@psu.edu>
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
---
 drivers/video/fbdev/offb.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
index f85428e13996b..166b2dff36f59 100644
--- a/drivers/video/fbdev/offb.c
+++ b/drivers/video/fbdev/offb.c
@@ -640,8 +640,13 @@ static void offb_init_nodriver(struct platform_device *parent, struct device_nod
 			vid = be32_to_cpup(vidp);
 			did = be32_to_cpup(didp);
 			pdev = pci_get_device(vid, did, NULL);
-			if (!pdev || pci_enable_device(pdev))
+			if (!pdev)
 				return;
+
+			if (pci_enable_device(pdev)) {
+				pci_dev_put(pdev);
+				return;
+			}
 		}
 #endif
 		/* kludge for valkyrie */
-- 
2.50.1 (Apple Git-155)


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

* Claude review: fbdev/offb: fix PCI device reference leak on probe failure
  2026-04-20  1:01 [PATCH v1] fbdev/offb: fix PCI device reference leak on probe failure Yuho Choi
@ 2026-04-23  0:29 ` Claude Code Review Bot
  2026-04-23  0:29 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-23  0:29 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: fbdev/offb: fix PCI device reference leak on probe failure
Author: Yuho Choi <dbgh9129@gmail.com>
Patches: 1
Reviewed: 2026-04-23T10:29:37.182445

---

This is a single-patch series that fixes a PCI device reference leak in the `offb` framebuffer driver. The fix is correct and straightforward: `pci_get_device()` returns a device with an incremented reference count, and the original code failed to call `pci_dev_put()` when `pci_enable_device()` failed. The patch properly splits the combined condition to handle each failure case appropriately.

The Fixes tag correctly identifies commit 5bda8f7b5468 which introduced the `pci_enable_device()` call and thus the leaking error path.

**Verdict: The patch is correct and should be accepted**, with one observation about a pre-existing issue noted below.

---
Generated by Claude Code Patch Reviewer

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

* Claude review: fbdev/offb: fix PCI device reference leak on probe failure
  2026-04-20  1:01 [PATCH v1] fbdev/offb: fix PCI device reference leak on probe failure Yuho Choi
  2026-04-23  0:29 ` Claude review: " Claude Code Review Bot
@ 2026-04-23  0:29 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-04-23  0:29 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**The bug:** In `offb_init_nodriver()`, the original code at line 642 was:

```c
pdev = pci_get_device(vid, did, NULL);
if (!pdev || pci_enable_device(pdev))
    return;
```

`pci_get_device()` increments the device's reference count. When `pdev` is non-NULL but `pci_enable_device()` fails, the function returned without calling `pci_dev_put(pdev)`, leaking the reference.

**The fix:** The patch correctly splits the condition:

```c
pdev = pci_get_device(vid, did, NULL);
if (!pdev)
    return;

if (pci_enable_device(pdev)) {
    pci_dev_put(pdev);
    return;
}
```

This ensures the reference is released on the `pci_enable_device()` failure path while keeping the `!pdev` case unchanged (no reference to release).

**Observation (pre-existing issue, not a problem with this patch):** On the *success* path, `pdev` is also never released. After `pci_enable_device()` succeeds, execution falls through to `offb_init_fb()` at line 655, which does not receive or store `pdev`. The reference from `pci_get_device()` is leaked on the success path too. This is outside the scope of this patch, but worth noting as a potential follow-up fix — the function would need a `pci_dev_put(pdev)` after the `pci_enable_device()` block completes (the device remains enabled even after the reference is dropped).

---
Generated by Claude Code Patch Reviewer

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

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-20  1:01 [PATCH v1] fbdev/offb: fix PCI device reference leak on probe failure Yuho Choi
2026-04-23  0:29 ` Claude review: " Claude Code Review Bot
2026-04-23  0:29 ` 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