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

[sashiko-bot@kernel.org]

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