netdev - Re: [PATCH] pciaer: report config read/write errors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <493586AF.3030402@myri.com>
Date:	Tue, 02 Dec 2008 14:04:15 -0500
From:	Loic Prylli <loic@...i.com>
To:	Stephen Hemminger <shemminger@...tta.com>
CC:	David Miller <davem@...emloft.net>, jeffrey.t.kirsher@...el.com,
	netdev@...r.kernel.org, jeff@...zik.org,
	peter.p.waskiewicz.jr@...el.com, linux-pci@...r.kernel.org
Subject: Re: [PATCH] pciaer: report config read/write errors

On 12/02/2008 12:23 PM, Stephen Hemminger wrote:
> This patch does more error checking in the Advanced Error Reporting code.
> Since AER needs to access PCI registers > 255, it won't work without MMCONFIG
> and other quirks may stop it as well. The code must check this by looking
> at return values from pci_read/write_config_XXX calls.  
>
> I don't have any hardware that uses AER routines but discovered this 
> in earlier versions of the sky2 driver that tried to use
> pci AER routines. Ended up just giving up and using other ways to access PCI
> config space on sky2 since there were too many platform glitches.
>   




When experimenting with sky2 driver, was pci_find_ext_capability() 
returning non-zero although further ext-space accesses were failing?



>
> Signed-off-by: Stephen Hemminger <shemminger@...tta.com>
>
>
> --- a/drivers/pci/pcie/aer/aerdrv_core.c	2008-12-02 07:56:08.000000000 -0800
> +++ b/drivers/pci/pcie/aer/aerdrv_core.c	2008-12-02 09:07:32.000000000 -0800
> @@ -31,80 +31,92 @@ module_param(forceload, bool, 0);
>  int pci_enable_pcie_error_reporting(struct pci_dev *dev)
>  {
>  	u16 reg16 = 0;
> -	int pos;
> +	int pos, err;
> +	u32 status;
>  
>  	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
>  	if (!pos)
>  		return -EIO;
>  
> +	err = pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
> +	if (err)
> +		return err;
> +
>
>   



For legacy-conf-space, most kernel code assumes success without 
checking. For ext-conf-space, wouldn't it be convenient to be able to 
make the same assumption when pci_find_ext_capability() returns a valid 
offset?

The patch looks good to me, but I am just asking whether there is a 
known case where pcie_find_ext_capability() returns a valid offset, 
although that offset might turn out unusable (it might be worth 
investigating pci_find_ext_capability() then).


Loic


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html