[PATCH] x86/pci/mrst: add extra check in fixed bar detection

[PATCH] x86/pci/mrst: add extra check in fixed bar detection

[Jacob Pan]

Fixed bar capability structure is searched in PCI extended configuration
space. We need to make sure there is a valid capability id to begin with
otherwise, the search code may stuck in a infinite loop which results in
boot hang.
This patch adds additional check for cap ID 0, which is also invalid.

Suggested-by: "H. Peter Anvin" <hpa@zytor.com>

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 arch/x86/pci/mrst.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/pci/mrst.c b/arch/x86/pci/mrst.c
index c9649d0..8636567 100644
--- a/arch/x86/pci/mrst.c
+++ b/arch/x86/pci/mrst.c
@@ -66,8 +66,9 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
 					  devfn, pos, 4, &pcie_cap))
 			return 0;
 
-		if (pcie_cap == 0xffffffff)
-			return 0;
+		if (PCI_EXT_CAP_ID(pcie_cap) == 0x0000 ||
+			PCI_EXT_CAP_ID(pcie_cap) == 0xffff)
+			break;
 
 		if (PCI_EXT_CAP_ID(pcie_cap) == PCI_EXT_CAP_ID_VNDR) {
 			raw_pci_ext_ops->read(pci_domain_nr(bus), bus->number,
@@ -76,7 +77,7 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
 				return pos;
 		}
 
-		pos = pcie_cap >> 20;
+		pos = PCI_EXT_CAP_NEXT(pcie_cap);
 	}
 
 	return 0;
-- 
1.7.0.4

Re: [PATCH] x86/pci/mrst: add extra check in fixed bar detection

[Jesse Barnes]

On Fri, 16 Jul 2010 11:58:26 -0700
Jacob Pan <jacob.jun.pan@linux.intel.com> wrote:

> Fixed bar capability structure is searched in PCI extended configuration
> space. We need to make sure there is a valid capability id to begin with
> otherwise, the search code may stuck in a infinite loop which results in
> boot hang.
> This patch adds additional check for cap ID 0, which is also invalid.
> 
> Suggested-by: "H. Peter Anvin" <hpa@zytor.com>
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
>  arch/x86/pci/mrst.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/pci/mrst.c b/arch/x86/pci/mrst.c
> index c9649d0..8636567 100644
> --- a/arch/x86/pci/mrst.c
> +++ b/arch/x86/pci/mrst.c
> @@ -66,8 +66,9 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
>  					  devfn, pos, 4, &pcie_cap))
>  			return 0;
>  
> -		if (pcie_cap == 0xffffffff)
> -			return 0;
> +		if (PCI_EXT_CAP_ID(pcie_cap) == 0x0000 ||
> +			PCI_EXT_CAP_ID(pcie_cap) == 0xffff)
> +			break;
>  
>  		if (PCI_EXT_CAP_ID(pcie_cap) == PCI_EXT_CAP_ID_VNDR) {
>  			raw_pci_ext_ops->read(pci_domain_nr(bus), bus->number,
> @@ -76,7 +77,7 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
>  				return pos;
>  		}
>  
> -		pos = pcie_cap >> 20;
> +		pos = PCI_EXT_CAP_NEXT(pcie_cap);
>  	}
>  
>  	return 0;

Looks good.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

[tip:x86/urgent] x86, pci, mrst: Add extra sanity check in walking the PCI extended cap chain

[tip-bot for Jacob Pan]

Commit-ID:  f82c3d71d6fd2e6a3e3416f09099e29087e39abf
Gitweb:     http://git.kernel.org/tip/f82c3d71d6fd2e6a3e3416f09099e29087e39abf
Author:     Jacob Pan <jacob.jun.pan@linux.intel.com>
AuthorDate: Fri, 16 Jul 2010 11:58:26 -0700
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Fri, 16 Jul 2010 16:52:15 -0700

x86, pci, mrst: Add extra sanity check in walking the PCI extended cap chain

The fixed bar capability structure is searched in PCI extended
configuration space.  We need to make sure there is a valid capability
ID to begin with otherwise, the search code may stuck in a infinite
loop which results in boot hang.  This patch adds additional check for
cap ID 0, which is also invalid, and indicates end of chain.

End of chain is supposed to have all fields zero, but that doesn't
seem to always be the case in the field.

Suggested-by: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
LKML-Reference: <1279306706-27087-1-git-send-email-jacob.jun.pan@linux.intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/pci/mrst.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/pci/mrst.c b/arch/x86/pci/mrst.c
index 7ef3a27..cb29191 100644
--- a/arch/x86/pci/mrst.c
+++ b/arch/x86/pci/mrst.c
@@ -66,8 +66,9 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
 					  devfn, pos, 4, &pcie_cap))
 			return 0;
 
-		if (pcie_cap == 0xffffffff)
-			return 0;
+		if (PCI_EXT_CAP_ID(pcie_cap) == 0x0000 ||
+			PCI_EXT_CAP_ID(pcie_cap) == 0xffff)
+			break;
 
 		if (PCI_EXT_CAP_ID(pcie_cap) == PCI_EXT_CAP_ID_VNDR) {
 			raw_pci_ext_ops->read(pci_domain_nr(bus), bus->number,
@@ -76,7 +77,7 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
 				return pos;
 		}
 
-		pos = pcie_cap >> 20;
+		pos = PCI_EXT_CAP_NEXT(pcie_cap);
 	}
 
 	return 0;

Re: [PATCH] x86/pci/mrst: add extra check in fixed bar detection

[Ben Greear]

On 07/16/2010 11:58 AM, Jacob Pan wrote:
> Fixed bar capability structure is searched in PCI extended configuration
> space. We need to make sure there is a valid capability id to begin with
> otherwise, the search code may stuck in a infinite loop which results in
> boot hang.
> This patch adds additional check for cap ID 0, which is also invalid.

I think this should be added to the stable tree for 2.6.34
as well...

Thanks,
Ben

>
> Suggested-by: "H. Peter Anvin"<hpa@zytor.com>
>
> Signed-off-by: Jacob Pan<jacob.jun.pan@linux.intel.com>
> ---
>   arch/x86/pci/mrst.c |    7 ++++---
>   1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/pci/mrst.c b/arch/x86/pci/mrst.c
> index c9649d0..8636567 100644
> --- a/arch/x86/pci/mrst.c
> +++ b/arch/x86/pci/mrst.c
> @@ -66,8 +66,9 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
>   					  devfn, pos, 4,&pcie_cap))
>   			return 0;
>
> -		if (pcie_cap == 0xffffffff)
> -			return 0;
> +		if (PCI_EXT_CAP_ID(pcie_cap) == 0x0000 ||
> +			PCI_EXT_CAP_ID(pcie_cap) == 0xffff)
> +			break;
>
>   		if (PCI_EXT_CAP_ID(pcie_cap) == PCI_EXT_CAP_ID_VNDR) {
>   			raw_pci_ext_ops->read(pci_domain_nr(bus), bus->number,
> @@ -76,7 +77,7 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
>   				return pos;
>   		}
>
> -		pos = pcie_cap>>  20;
> +		pos = PCI_EXT_CAP_NEXT(pcie_cap);
>   	}
>
>   	return 0;


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH] x86/pci/mrst: add extra check in fixed bar detection

[H. Peter Anvin]

On 07/20/2010 10:18 AM, Ben Greear wrote:
> On 07/16/2010 11:58 AM, Jacob Pan wrote:
>> Fixed bar capability structure is searched in PCI extended configuration
>> space. We need to make sure there is a valid capability id to begin with
>> otherwise, the search code may stuck in a infinite loop which results in
>> boot hang.
>> This patch adds additional check for cap ID 0, which is also invalid.
> 
> I think this should be added to the stable tree for 2.6.34
> as well...
> 

Agreed. Greg, could you consider upstream
f82c3d71d6fd2e6a3e3416f09099e29087e39abf for -stable 2.6.34?

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] x86/pci/mrst: add extra check in fixed bar detection

[Greg KH]

On Tue, Jul 20, 2010 at 10:26:08AM -0700, H. Peter Anvin wrote:
> On 07/20/2010 10:18 AM, Ben Greear wrote:
> > On 07/16/2010 11:58 AM, Jacob Pan wrote:
> >> Fixed bar capability structure is searched in PCI extended configuration
> >> space. We need to make sure there is a valid capability id to begin with
> >> otherwise, the search code may stuck in a infinite loop which results in
> >> boot hang.
> >> This patch adds additional check for cap ID 0, which is also invalid.
> > 
> > I think this should be added to the stable tree for 2.6.34
> > as well...
> > 
> 
> Agreed. Greg, could you consider upstream
> f82c3d71d6fd2e6a3e3416f09099e29087e39abf for -stable 2.6.34?

Will do.

thanks,

greg k-h