Re: [PATCH v3 2/6] LoongArch/PCI: Refresh screen_info after resource assignment

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] LoongArch/PCI: repeated fixup and UAF via
  screen_info_apply_fixups()
--

commit 1413644b1b0b80d135ecb2502366b5c2eb670d52
Author: wuqianhai <wuqianhai@loongson.cn>

LoongArch/PCI: Refresh screen_info after resource assignment

This commit calls screen_info_apply_fixups() in pci_acpi_scan_root() after
assigning root bus resources so that screen_info reflects any relocated
firmware framebuffer BAR before vga_arb_device_init() and sysfb_init() run.

> diff --git a/arch/loongarch/pci/acpi.c b/arch/loongarch/pci/acpi.c
> index ccbcea61fcd9c..cb313ff67f7e7 100644
> --- a/arch/loongarch/pci/acpi.c
> +++ b/arch/loongarch/pci/acpi.c
[ ... ]
> @@ -256,6 +257,9 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  		 */
>  		pci_assign_unassigned_root_bus_resources(bus);
>  
> +		if (IS_ENABLED(CONFIG_SCREEN_INFO))
> +			screen_info_apply_fixups();

[Severity: Medium]
Is screen_info_apply_fixups() safe to call from pci_acpi_scan_root()?

screen_info_apply_fixups() was previously called only once from
sysfb_init().  When it updates the framebuffer address, it neither updates
screen_info_lfb_res_start nor clears screen_info_lfb_pdev:

drivers/video/screen_info_pci.c:screen_info_apply_fixups() {
	...
	if (screen_info_lfb_pdev) {
		struct resource *pr = &screen_info_lfb_pdev->resource[screen_info_lfb_bar];

		if (pr->start != screen_info_lfb_res_start) {
			if (__screen_info_relocation_is_valid(si, pr)) {
				__screen_info_set_lfb_base(si, pr->start + screen_info_lfb_offset);
				pr_info("Relocating firmware framebuffer to offset %pa[d] within %pr\n",
					&screen_info_lfb_offset, pr);
	...
}

Because pr->start != screen_info_lfb_res_start remains true after a
relocation, every subsequent root bridge scan on multi-root systems and the
later call in sysfb_init() will re-run the relocation branch and emit
duplicate messages.  If a built-in DRM driver probes before sysfb_init(),
calls sysfb_disable(), and resizes the BAR, sysfb_init() will also
re-evaluate the resized BAR before checking whether sysfb was disabled.

In addition, screen_info_lfb_pdev is an unreferenced pointer that is never
cleared on PCI device removal, while pci_acpi_scan_root() can be called
after boot via acpi_pci_root_add() hotplug.  Removing the boot display PCI
device and hot-adding an ACPI PCI root bridge would dereference the freed
screen_info_lfb_pdev in screen_info_apply_fixups().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110090203.2631481-1-wuqianhai@loongson.cn?part=2