Re:[PATCH v2] char: agp: amd64 - fix broken error propagation in agp_amd64_probe()

[w15303746062 <w15303746062@163.com>]

Hi Dave and everyone,

Just a gentle ping on this patch. It has been a month since v2 was submitted.

Please let me know if there are any questions, or if any further modifications are needed.

Thanks,
Mingyu

At 2026-05-04 15:48:23, w15303746062@163.com wrote:
>From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>
>A NULL pointer dereference was observed in the AMD64 AGP driver when
>running in a virtualized environment (e.g., QEMU/KVM) without a physical
>AMD Northbridge. The crash occurs in amd64_fetch_size() when attempting
>to dereference the pointer returned by node_to_amd_nb(0).
>
>The root cause of this crash is broken error propagation in
>agp_amd64_probe(). When no AMD Northbridges are found, cache_nbs()
>correctly returns -ENODEV. However, the probe function erroneously
>checked the return value against exactly -1, rather than < 0.
>
>As a result, the hardware absence error was masked, allowing the driver
>to improperly proceed with initialization. It eventually called
>agp_add_bridge(), which invokes amd64_fetch_size(). Since the hardware
>does not exist, node_to_amd_nb(0) returns NULL, leading to a General
>Protection Fault (GPF) when accessing its ->misc member.
>
>Fix the issue by correcting the error check in agp_amd64_probe() to
>abort properly when cache_nbs() returns any negative error code. This
>prevents the driver from erroneously proceeding without hardware, thereby
>resolving the subsequent NULL pointer dereference at its source.
>
>Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>---
>+ Changes in v2:
>+ - Dropped redundant NULL pointer checks in various initialization functions.
>+ - Fixed the actual root cause: broken error propagation in agp_amd64_probe() 
>+   where it erroneously checked cache_nbs() against exactly -1 instead of < 0.
>+   (Thanks to Sashiko AI for the review feedback).
>+
> drivers/char/agp/amd64-agp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
>index 2505df1f4e69..6741270e0a98 100644
>--- a/drivers/char/agp/amd64-agp.c
>+++ b/drivers/char/agp/amd64-agp.c
>@@ -546,7 +546,7 @@ static int agp_amd64_probe(struct pci_dev *pdev,
> 	/* Fill in the mode register */
> 	pci_read_config_dword(pdev, bridge->capndx+PCI_AGP_STATUS, &bridge->mode);
> 
>-	if (cache_nbs(pdev, cap_ptr) == -1) {
>+	if (cache_nbs(pdev, cap_ptr) < 0) {
> 		agp_put_bridge(bridge);
> 		return -ENODEV;
> 	}
>-- 
>2.34.1