[PATCH] char: agp: amd64 - fix null-ptr-deref in amd64_fetch_size and related functions

[PATCH] char: agp: amd64 - fix null-ptr-deref in amd64_fetch_size and related functions

[w15303746062]

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

A NULL pointer dereference vulnerability was identified in the AMD64 AGP
driver (amd64-agp) during driver initialization and aperture size fetching.

When the `amd64_agp` module is loaded on a system without a physical AMD
Northbridge (e.g., in a QEMU/KVM virtualized environment with a simulated
AMD 8151 AGP bridge), the underlying hardware query `node_to_amd_nb(0)`
returns NULL.

In `amd64_fetch_size()`, the code previously attempted to unconditionally
dereference the `misc` member of the returned pointer:
    `dev = node_to_amd_nb(0)->misc;`

Since `node_to_amd_nb(0)` can return NULL (either due to missing hardware
or when CONFIG_AMD_NB is disabled), this direct dereference results in a
General Protection Fault (GPF) and a subsequent kernel panic, as caught
by KASAN.

Fix this by introducing proper sanity checks. Before accessing the `misc`
pointer, explicitly verify that the pointer returned by `node_to_amd_nb()`
is not NULL.

Furthermore, to prevent similar crashes, this patch sweeps the entire
driver and applies the same safeguard to all other functions that iterate
over or directly access the AMD Northbridge descriptors, including
`amd_8151_configure()`, `amd64_cleanup()`, `cache_nbs()`, `uli_agp_init()`,
and `nforce3_agp_init()`.

Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
 drivers/char/agp/amd64-agp.c | 50 ++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
index 2505df1f4e69..7bbadfc74ffe 100644
--- a/drivers/char/agp/amd64-agp.c
+++ b/drivers/char/agp/amd64-agp.c
@@ -121,14 +121,16 @@ static struct aper_size_info_32 amd64_aperture_sizes[7] =
 static int amd64_fetch_size(void)
 {
 	struct pci_dev *dev;
+	struct amd_northbridge *nb;
 	int i;
 	u32 temp;
 	struct aper_size_info_32 *values;
 
-	dev = node_to_amd_nb(0)->misc;
-	if (dev==NULL)
+	nb = node_to_amd_nb(0);
+	if (!nb || !nb->misc)
 		return 0;
 
+	dev = nb->misc;
 	pci_read_config_dword(dev, AMD64_GARTAPERTURECTL, &temp);
 	temp = (temp & 0xe);
 	values = A_SIZE_32(amd64_aperture_sizes);
@@ -187,8 +189,12 @@ static int amd_8151_configure(void)
 
 	/* Configure AGP regs in each x86-64 host bridge. */
 	for (i = 0; i < amd_nb_num(); i++) {
-		agp_bridge->gart_bus_addr =
-			amd64_configure(node_to_amd_nb(i)->misc, gatt_bus);
+		struct amd_northbridge *nb = node_to_amd_nb(i);
+
+		if (!nb || !nb->misc)
+			continue;
+
+		agp_bridge->gart_bus_addr = amd64_configure(nb->misc, gatt_bus);
 	}
 	amd_flush_garts();
 	return 0;
@@ -204,7 +210,13 @@ static void amd64_cleanup(void)
 		return;
 
 	for (i = 0; i < amd_nb_num(); i++) {
-		struct pci_dev *dev = node_to_amd_nb(i)->misc;
+		struct amd_northbridge *nb = node_to_amd_nb(i);
+		struct pci_dev *dev;
+
+		if (!nb || !nb->misc)
+			continue;
+
+		dev = nb->misc;
 		/* disable gart translation */
 		pci_read_config_dword(dev, AMD64_GARTAPERTURECTL, &tmp);
 		tmp &= ~GARTEN;
@@ -335,7 +347,13 @@ static int cache_nbs(struct pci_dev *pdev, u32 cap_ptr)
 
 	i = 0;
 	for (i = 0; i < amd_nb_num(); i++) {
-		struct pci_dev *dev = node_to_amd_nb(i)->misc;
+		struct amd_northbridge *nb = node_to_amd_nb(i);
+		struct pci_dev *dev;
+
+		if (!nb || !nb->misc)
+			continue;
+
+		dev = nb->misc;
 		if (fix_northbridge(dev, pdev, cap_ptr) < 0) {
 			dev_err(&dev->dev, "no usable aperture found\n");
 #ifdef __x86_64__
@@ -391,6 +409,7 @@ static int uli_agp_init(struct pci_dev *pdev)
 {
 	u32 httfea,baseaddr,enuscr;
 	struct pci_dev *dev1;
+	struct amd_northbridge *nb;
 	int i, ret;
 	unsigned size = amd64_fetch_size();
 
@@ -411,9 +430,14 @@ static int uli_agp_init(struct pci_dev *pdev)
 		goto put;
 	}
 
+	nb = node_to_amd_nb(0);
+	if (!nb || !nb->misc) {
+		ret = -ENODEV;
+		goto put;
+	}
+
 	/* shadow x86-64 registers into ULi registers */
-	pci_read_config_dword (node_to_amd_nb(0)->misc, AMD64_GARTAPERTUREBASE,
-			       &httfea);
+	pci_read_config_dword(nb->misc, AMD64_GARTAPERTUREBASE, &httfea);
 
 	/* if x86-64 aperture base is beyond 4G, exit here */
 	if ((httfea & 0x7fff) >> (32 - 25)) {
@@ -453,6 +477,7 @@ static int nforce3_agp_init(struct pci_dev *pdev)
 {
 	u32 tmp, apbase, apbar, aplimit;
 	struct pci_dev *dev1;
+	struct amd_northbridge *nb;
 	int i, ret;
 	unsigned size = amd64_fetch_size();
 
@@ -479,9 +504,14 @@ static int nforce3_agp_init(struct pci_dev *pdev)
 	tmp |= nforce3_sizes[i].size_value;
 	pci_write_config_dword(dev1, NVIDIA_X86_64_1_APSIZE, tmp);
 
+	nb = node_to_amd_nb(0);
+	if (!nb || !nb->misc) {
+		ret = -ENODEV;
+		goto put;
+	}
+
 	/* shadow x86-64 registers into NVIDIA registers */
-	pci_read_config_dword (node_to_amd_nb(0)->misc, AMD64_GARTAPERTUREBASE,
-			       &apbase);
+	pci_read_config_dword(nb->misc, AMD64_GARTAPERTUREBASE, &apbase);
 
 	/* if x86-64 aperture base is beyond 4G, exit here */
 	if ( (apbase & 0x7fff) >> (32 - 25) ) {
-- 
2.34.1

Re: [PATCH] char: agp: amd64 - fix null-ptr-deref in amd64_fetch_size and related functions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch masks a kernel crash symptom without fixing the underlying broken error propagation in `agp_amd64_probe()`, allowing the driver to erroneously proceed without hardware.
- [Low] The newly added NULL pointer safeguards across multiple initialization and cleanup functions are mathematically redundant or structurally unreachable.
--

commit 97080eec153aeef89c2ae80f04042c9b13cb0fb4
Author: Mingyu Wang <25181214217@stu.xidian.edu.cn>

char: agp: amd64 - fix null-ptr-deref in amd64_fetch_size and related functions

This patch adds safeguards across the AMD64 AGP driver to verify that
node_to_amd_nb() and its misc pointer are not null. It attempts to fix a
kernel panic in environments without a physical AMD Northbridge.

> diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
> --- a/drivers/char/agp/amd64-agp.c
> +++ b/drivers/char/agp/amd64-agp.c
> @@ -121,14 +121,16 @@ static struct aper_size_info_32 amd64_aperture_sizes[7] =
>  static int amd64_fetch_size(void)
>  {
>  	struct pci_dev *dev;
> +	struct amd_northbridge *nb;
>  	int i;
>  	u32 temp;
>  	struct aper_size_info_32 *values;
>  
> -	dev = node_to_amd_nb(0)->misc;
> -	if (dev==NULL)
> +	nb = node_to_amd_nb(0);
> +	if (!nb || !nb->misc)
>  		return 0;

Does returning 0 here merely mask the underlying issue?

The patch prevents a crash by handling the missing hardware here, but this
still allows the driver to erroneously proceed without hardware. Should
agp_amd64_probe() be fixed to abort on cache_nbs() < 0 instead of just
checking for == -1, so the error propagates properly when hardware is
missing?

[ ... ]

> @@ -187,8 +189,12 @@ static int amd_8151_configure(void)
>  
>  	/* Configure AGP regs in each x86-64 host bridge. */
>  	for (i = 0; i < amd_nb_num(); i++) {
> -		agp_bridge->gart_bus_addr =
> -			amd64_configure(node_to_amd_nb(i)->misc, gatt_bus);
> +		struct amd_northbridge *nb = node_to_amd_nb(i);
> +
> +		if (!nb || !nb->misc)
> +			continue;

Are these safeguards necessary inside the loop? 

Since the loop is bounded by amd_nb_num(), isn't the Northbridge descriptor
and its misc pointer guaranteed to be non-NULL if amd_nb_num() is greater
than zero? This applies to amd_8151_configure(), amd64_cleanup(), and
cache_nbs().

[ ... ]

> @@ -391,6 +409,7 @@ static int uli_agp_init(struct pci_dev *pdev)
>  {
>  	u32 httfea,baseaddr,enuscr;
>  	struct pci_dev *dev1;
> +	struct amd_northbridge *nb;
>  	int i, ret;
>  	unsigned size = amd64_fetch_size();
>  
> @@ -411,9 +430,14 @@ static int uli_agp_init(struct pci_dev *pdev)
>  		goto put;
>  	}
>  
> +	nb = node_to_amd_nb(0);
> +	if (!nb || !nb->misc) {
> +		ret = -ENODEV;
> +		goto put;
> +	}

Is this check structurally reachable?

If amd64_fetch_size() returned 0 earlier in this function due to missing
hardware, the array lookup for uli_sizes will fail. This causes an early
-ENODEV exit before ever reaching this new check. The same unreachable
logic appears to apply to the changes in nforce3_agp_init().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504065441.99033-1-w15303746062@163.com?part=1