Re: [PATCH v4 5/9] agp/via: Don't rely on address of pci_device_id

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

Pre-existing issues:
- [High] The exact same out-of-bounds array access vulnerability triggered by dynamic IDs is fixed in via-agp but left unpatched in the adjacent amd-k7-agp driver.
--

--- Patch [5]: [PATCH v4 5/9] agp/via: Don't rely on address of pci_device_id ---
commit 2a62b6ae1ca2d9be48b681d606a5d3d5afef208c
Author: Gary Guo <[email protected]>

agp/via: Don't rely on address of pci_device_id

Address of pci_device_id cannot be relied on due to presence of dynamic ID
and driver_override. Use driver_data instead.

> diff --git a/drivers/char/agp/via-agp.c b/drivers/char/agp/via-agp.c
> --- a/drivers/char/agp/via-agp.c
> +++ b/drivers/char/agp/via-agp.c
[ ... ]
> @@ -437,17 +239,14 @@ static void check_via_agp3 (struct agp_bridge_data *bridge)
>  
>  static int agp_via_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
> -	struct agp_device_ids *devs = via_agp_device_ids;
>  	struct agp_bridge_data *bridge;
> -	int j = 0;
>  	u8 cap_ptr;
>  
>  	cap_ptr = pci_find_capability(pdev, PCI_CAP_ID_AGP);
>  	if (!cap_ptr)
>  		return -ENODEV;
>  
> -	j = ent - agp_via_pci_table;
> -	printk (KERN_INFO PFX "Detected VIA %s chipset\n", devs[j].chipset_name);
> +	dev_info(&pdev->dev, "Detected VIA %s chipset\n", (const char *)ent->driver_data);

[Severity: High]
This is a pre-existing issue, but the exact same out-of-bounds array access 
vulnerability triggered by dynamic IDs is left unpatched in the adjacent 
drivers/char/agp/amd-k7-agp.c driver.

Looking at agp_amdk7_probe() in amd-k7-agp.c:

    j = ent - agp_amdk7_pci_table;
    dev_info(&pdev->dev, "AMD %s chipset\n",
             amd_agp_device_ids[j].chipset_name);

Should a similar fix be applied there as well, since userspace could trigger 
a kernel oops via sysfs driver_override or new_id entries?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5