[PATCH] fix: drivers/video: __screen_info_pci_dev: leaked pci_dev references in pci_get_base_class loop

[PATCH] fix: drivers/video: __screen_info_pci_dev: leaked pci_dev references in pci_get_base_class loop

[WenTao Liang]

In __screen_info_pci_dev(), the loop uses pci_get_base_class() with a
non-NULL starting device pdev. Each iteration returns a new device
reference but does not release the previous one. When a non-matching
device is found, pdev is overwritten and the previous reference leaks.
When no match is found, all acquired references are leaked.

Add pci_dev_put(pdev) for non-matching devices before continuing the loop.

Cc: stable@vger.kernel.org
Fixes: 036105e3a776 ("video: Provide screen_info_get_pci_dev() to find screen_info's PCI device")
Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
---
 drivers/video/screen_info_pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/video/screen_info_pci.c b/drivers/video/screen_info_pci.c
index 8f34d8a74f09..c821101e9304 100644
--- a/drivers/video/screen_info_pci.c
+++ b/drivers/video/screen_info_pci.c
@@ -123,6 +123,10 @@ static struct pci_dev *__screen_info_pci_dev(struct resource *res)
 
 	while (!r && (pdev = pci_get_base_class(PCI_BASE_CLASS_DISPLAY, pdev))) {
 		r = pci_find_resource(pdev, res);
+		if (!r) {
+			pci_dev_put(pdev);
+			pdev = NULL;
+		}
 	}
 
 	return pdev;
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] fix: drivers/video: __screen_info_pci_dev: leaked pci_dev references in pci_get_base_class loop

[sashiko-bot]

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

New issues:
- [Critical] The patch introduces a kernel-hanging infinite loop and is based on a fundamentally incorrect assumption about standard PCI iterators leaking memory.

Pre-existing issues:
- [High] A pre-existing memory leak of the `pdev` reference occurs when `vga_is_firmware_default` checks for the firmware default device.
--

commit 5ad7d85025adc767e102b08f0b9fd9b693e4565f
Author: WenTao Liang <vulab@iscas.ac.cn>

fix: drivers/video: __screen_info_pci_dev: leaked pci_dev references in pci_get_base_class loop

This commit adds pci_dev_put(pdev) for non-matching devices before continuing
the loop in __screen_info_pci_dev(). It aims to fix a perceived pci_dev
reference leak when iterating with pci_get_base_class().

> diff --git a/drivers/video/screen_info_pci.c b/drivers/video/screen_info_pci.c
> index 8f34d8a74f098..c821101e93045 100644
> --- a/drivers/video/screen_info_pci.c
> +++ b/drivers/video/screen_info_pci.c
> @@ -123,6 +123,10 @@ static struct pci_dev *__screen_info_pci_dev(struct resource *res)
>  
>  	while (!r && (pdev = pci_get_base_class(PCI_BASE_CLASS_DISPLAY, pdev))) {
>  		r = pci_find_resource(pdev, res);
> +		if (!r) {
> +			pci_dev_put(pdev);
> +			pdev = NULL;
> +		}

[Severity: Critical]
Does this introduce an infinite loop? 

By setting pdev = NULL, the iterator state for pci_get_base_class() is
destroyed. The next iteration will restart the search from NULL, retrieving
the identical non-matching device endlessly and leading to a kernel hang on
systems where the first PCI display device found does not contain the
targeted screen_info resource.

Is the original assumption of a memory leak correct?

Standard PCI iterators like pci_get_base_class() unconditionally drop the
reference to the previously visited device via pci_dev_put() on the 'from'
parameter.

>  	}
>  
>  	return pdev;

[Severity: High]
This is a pre-existing issue, but does vga_is_firmware_default() leak a
pci_dev reference?

drivers/pci/vgaarb.c:vga_is_firmware_default() {
    ...
    return pdev == screen_info_pci_dev(&sysfb_primary_display.screen);
}

The screen_info_pci_dev() function returns a pci_dev with an incremented
reference count, but the caller simply compares the returned pointer and
discards it without calling pci_dev_put() to release it.

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

Re: [PATCH] fix: drivers/video: __screen_info_pci_dev: leaked pci_dev references in pci_get_base_class loop

[Helge Deller]

On 6/27/26 05:44, WenTao Liang wrote:
> In __screen_info_pci_dev(), the loop uses pci_get_base_class() with a
> non-NULL starting device pdev. Each iteration returns a new device
> reference but does not release the previous one. When a non-matching
> device is found, pdev is overwritten and the previous reference leaks.
> When no match is found, all acquired references are leaked.
> 
> Add pci_dev_put(pdev) for non-matching devices before continuing the loop.
> 
> Cc: stable@vger.kernel.org
> Fixes: 036105e3a776 ("video: Provide screen_info_get_pci_dev() to find screen_info's PCI device")
> Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
> ---
>   drivers/video/screen_info_pci.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/video/screen_info_pci.c b/drivers/video/screen_info_pci.c
> index 8f34d8a74f09..c821101e9304 100644
> --- a/drivers/video/screen_info_pci.c
> +++ b/drivers/video/screen_info_pci.c
> @@ -123,6 +123,10 @@ static struct pci_dev *__screen_info_pci_dev(struct resource *res)
>   
>   	while (!r && (pdev = pci_get_base_class(PCI_BASE_CLASS_DISPLAY, pdev))) {
>   		r = pci_find_resource(pdev, res);
> +		if (!r) {
> +			pci_dev_put(pdev);
> +			pdev = NULL;
> +		}

Have you tested the code?
If pdev gets assigned NULL, doesn't that introduce an endless loop?
And, similar code is in amdgpu*  and google's framebuffer-coreboot.c files, so if
this is correct, don't they need fixing as well?

Helge