| 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
| ||
|
Message-ID: <4FC93B1A.6030800@redhat.com>
Date: Fri, 01 Jun 2012 17:58:50 -0400
From: Don Dutile <ddutile@...hat.com>
To: Myron Stowe <myron.stowe@...hat.com>
CC: bhelgaas@...gle.com, linux-pci@...r.kernel.org,
linux-kernel@...r.kernel.org, xudong.hao@...ux.intel.com,
yu.zhao@...el.com
Subject: Re: [PATCH 3/4] PCI: Add pci_pcie_cap2() check for PCIe feature capabilities
>= v2
On 06/01/2012 05:16 PM, Myron Stowe wrote:
> This patch resolves potential issues when accessing PCI Express
> capability structures. The makeup of Express' capability structure
> varies substantially between v1 and v2:
>
> PCI Express 1.0 and 1.1 base neither requires the endpoint to
> implement the entire PCIe capability structure nor specifies
> default values of registers that are not implemented by the
> device. Its capability version is 1.
>
> PCIe 1.1 Capability Structure Expansion ECN, PCIe 2.0, 2.1, and
> 3.0 added additional registers to the structure and require all
> registers to be either implemented or hardwired to 0. Their PCIe
> capability version is 2.
>
> Due to the differences in the capability structures, code dealing with
> capability features must be careful not to access the additional
> registers introduced with v2 unless the device is specifically known to
> be a v2 capable device. Otherwise, attempts to access non-existant
> registers will occur. This is a subtle issue that is hard to track down
> when it occurs (and it has - see commit 864d296cf94).
>
> To try and help mitigate such occurrances, this patch introduces
> pci_pcie_cap2() which is similar to pci_pcie_cap() but also checks
> that the PCIe capability version is>= 2. pci_pcie_cap2() should be
> used for qualifying PCIe capability features introduced after v1.
>
> Suggested by Don Dutile.
>
Acked by him too!
... except one nit...
I would change the comment "is a PCIe v2 feature" to "is a PCIe cap v2 list feature"
so no one confuses spec version with a specific cap-struct version.
but I wouldn't hold up this patch for this nit.
> Signed-off-by: Myron Stowe<myron.stowe@...hat.com>
> ---
>
> drivers/pci/pci.c | 65 +++++++++++++++++++++++++++++++++++-----------
> include/linux/pci_regs.h | 7 +++++
> 2 files changed, 57 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index ff0beb0..26933ff 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -279,6 +279,38 @@ int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap)
> }
>
> /**
> + * pci_pcie_cap2 - query for devices' PCI_CAP_ID_EXP v2 capability structure
> + * @dev: PCI device to check
> + *
> + * Like pci_pcie_cap() but also checks that the PCIe capability version is
> + *>= 2. Note that v1 capability structures could be sparse in that not
> + * all register fields were required. v2 requires the entire structure to
> + * be present size wise, while still allowing for non-implemented registers
> + * to exist but they must be hardwired to 0.
> + *
> + * Due to the differences in the versions of capability structures, one
> + * must be careful not to try and access non-existant registers that may
> + * exist in early versions - v1 - of Express devices.
> + *
> + * Returns the offset of the PCIe capability structure as long as the
> + * capability version is>= 2; otherwise 0 is returned.
> + */
> +static int pci_pcie_cap2(struct pci_dev *dev)
> +{
> + u16 flags;
> + int pos;
> +
> + pos = pci_pcie_cap(dev);
> + if (pos) {
> + pci_read_config_word(dev, pos + PCI_EXP_FLAGS,&flags);
> + if ((flags& PCI_EXP_FLAGS_VERS)< 2)
> + pos = 0;
> + }
> +
> + return pos;
> +}
> +
> +/**
> * pci_find_ext_capability - Find an extended capability
> * @dev: PCI device to query
> * @cap: capability code
> @@ -1984,7 +2016,7 @@ void pci_enable_ari(struct pci_dev *dev)
> {
> int pos;
> u32 cap;
> - u16 flags, ctrl;
> + u16 ctrl;
> struct pci_dev *bridge;
>
> if (pcie_ari_disabled || !pci_is_pcie(dev) || dev->devfn)
> @@ -1998,13 +2030,9 @@ void pci_enable_ari(struct pci_dev *dev)
> if (!bridge)
> return;
>
> - pos = pci_pcie_cap(bridge);
> - if (!pos)
> - return;
> -
> /* ARI is a PCIe v2 feature */
> - pci_read_config_word(bridge, pos + PCI_EXP_FLAGS,&flags);
> - if ((flags& PCI_EXP_FLAGS_VERS)< 2)
> + pos = pci_pcie_cap2(bridge);
> + if (!pos)
> return;
>
> pci_read_config_dword(bridge, pos + PCI_EXP_DEVCAP2,&cap);
> @@ -2019,7 +2047,7 @@ void pci_enable_ari(struct pci_dev *dev)
> }
>
> /**
> - * pci_enable_ido - enable ID-based ordering on a device
> + * pci_enable_ido - enable ID-based Ordering on a device
> * @dev: the PCI device
> * @type: which types of IDO to enable
> *
> @@ -2032,7 +2060,8 @@ void pci_enable_ido(struct pci_dev *dev, unsigned long type)
> int pos;
> u16 ctrl;
>
> - pos = pci_pcie_cap(dev);
> + /* ID-based Ordering is a PCIe v2 feature */
> + pos = pci_pcie_cap2(dev);
> if (!pos)
> return;
>
> @@ -2055,7 +2084,8 @@ void pci_disable_ido(struct pci_dev *dev, unsigned long type)
> int pos;
> u16 ctrl;
>
> - pos = pci_pcie_cap(dev);
> + /* ID-based Ordering is a PCIe v2 feature */
> + pos = pci_pcie_cap2(dev);
> if (!pos)
> return;
>
> @@ -2094,7 +2124,8 @@ int pci_enable_obff(struct pci_dev *dev, enum pci_obff_signal_type type)
> u16 ctrl;
> int ret;
>
> - pos = pci_pcie_cap(dev);
> + /* OBFF is a PCIe v2 feature */
> + pos = pci_pcie_cap2(dev);
> if (!pos)
> return -ENOTSUPP;
>
> @@ -2144,7 +2175,8 @@ void pci_disable_obff(struct pci_dev *dev)
> int pos;
> u16 ctrl;
>
> - pos = pci_pcie_cap(dev);
> + /* OBFF is a PCIe v2 feature */
> + pos = pci_pcie_cap2(dev);
> if (!pos)
> return;
>
> @@ -2166,7 +2198,8 @@ static bool pci_ltr_supported(struct pci_dev *dev)
> int pos;
> u32 cap;
>
> - pos = pci_pcie_cap(dev);
> + /* LTR is a PCIe v2 feature */
> + pos = pci_pcie_cap2(dev);
> if (!pos)
> return false;
>
> @@ -2194,7 +2227,8 @@ int pci_enable_ltr(struct pci_dev *dev)
> if (!pci_ltr_supported(dev))
> return -ENOTSUPP;
>
> - pos = pci_pcie_cap(dev);
> + /* LTR is a PCIe v2 feature */
> + pos = pci_pcie_cap2(dev);
> if (!pos)
> return -ENOTSUPP;
>
> @@ -2229,7 +2263,8 @@ void pci_disable_ltr(struct pci_dev *dev)
> if (!pci_ltr_supported(dev))
> return;
>
> - pos = pci_pcie_cap(dev);
> + /* LTR is a PCIe v2 feature */
> + pos = pci_pcie_cap2(dev);
> if (!pos)
> return;
>
> diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
> index 4b608f5..bc1b54d 100644
> --- a/include/linux/pci_regs.h
> +++ b/include/linux/pci_regs.h
> @@ -507,6 +507,13 @@
> #define PCI_EXP_RTSTA 32 /* Root Status */
> #define PCI_EXP_RTSTA_PME 0x10000 /* PME status */
> #define PCI_EXP_RTSTA_PENDING 0x20000 /* PME pending */
> +/*
> + * Note that the following PCI Express 'Capability Structure' registers
> + * were introduced with 'Capability Version' 0x2 (v2). These registers
> + * do not exist, and thus should not be attempted to be accesses, by
> + * PCI Express v1 devices (see pci_pcie_cap2() for use in qualifying
> + * PCI Express v2 capability related features).
> + */
> #define PCI_EXP_DEVCAP2 36 /* Device Capabilities 2 */
> #define PCI_EXP_DEVCAP2_ARI 0x20 /* Alternative Routing-ID */
> #define PCI_EXP_DEVCAP2_LTR 0x800 /* Latency tolerance reporting */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists