Hi Matt and Ben,
What happens when the VPD information is corrupted? I have an NIC which
functions but has a badly formed VPD (When iterating through the VPD
tags, you will never reach an end tag) And because this patch will scan
through the entire VPD address space, it will generate an out-of-range
access. Would you have any suggestions on how we could correct this?
Thanks again.
-Ben
On Mon, 2008-06-30 at 17:52 +0100, Ben Hutchings wrote:
> The PCI 2.2 VPD address range is 32 KB but few PCI devices actually
> map this much storage. It is not clear whether hardware or software
> is responsible for avoiding out-of-range access, and some devices are
> known to lock up if we read beyond the valid size. Therefore we
> follow the VPD resource chain to find the valid size.
>
> Signed-off-by: Ben Hutchings <bhutchings@xxxxxxxxxxxxxx>
> ---
> drivers/pci/access.c | 55 ++++++++++++++++++++++++++++++++++++++--------
> drivers/pci/pci-sysfs.c | 2 +-
> drivers/pci/pci.h | 4 ++-
> 3 files changed, 49 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> index ec8f700..8df2db2 100644
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -178,8 +178,7 @@ static int pci_vpd_pci22_read(struct pci_dev *dev, int pos, int size,
> int ret;
> int begin, end, i;
>
> - if (pos < 0 || pos > PCI_VPD_PCI22_SIZE ||
> - size > PCI_VPD_PCI22_SIZE - pos)
> + if (pos < 0 || pos > vpd->base.size || size > vpd->base.size - pos)
> return -EINVAL;
> if (size == 0)
> return 0;
> @@ -223,8 +222,8 @@ static int pci_vpd_pci22_write(struct pci_dev *dev, int pos, int size,
> u32 val;
> int ret;
>
> - if (pos < 0 || pos > PCI_VPD_PCI22_SIZE || pos & 3 ||
> - size > PCI_VPD_PCI22_SIZE - pos || size < 4)
> + if (pos < 0 || pos > vpd->base.size || pos & 3 ||
> + size > vpd->base.size - pos || size < 4)
> return -EINVAL;
>
> val = (u8) *buf++;
> @@ -255,11 +254,6 @@ out:
> return 4;
> }
>
> -static int pci_vpd_pci22_get_size(struct pci_dev *dev)
> -{
> - return PCI_VPD_PCI22_SIZE;
> -}
> -
> static void pci_vpd_pci22_release(struct pci_dev *dev)
> {
> kfree(container_of(dev->vpd, struct pci_vpd_pci22, base));
> @@ -268,7 +262,6 @@ static void pci_vpd_pci22_release(struct pci_dev *dev)
> static struct pci_vpd_ops pci_vpd_pci22_ops = {
> .read = pci_vpd_pci22_read,
> .write = pci_vpd_pci22_write,
> - .get_size = pci_vpd_pci22_get_size,
> .release = pci_vpd_pci22_release,
> };
>
> @@ -285,13 +278,55 @@ int pci_vpd_pci22_init(struct pci_dev *dev)
> return -ENOMEM;
>
> vpd->base.ops = &pci_vpd_pci22_ops;
> + vpd->base.size = PCI_VPD_PCI22_SIZE;
> spin_lock_init(&vpd->lock);
> vpd->cap = cap;
> vpd->busy = false;
> dev->vpd = &vpd->base;
> + pci_vpd_set_real_size(dev);
> return 0;
> }
>
> +void pci_vpd_set_real_size(struct pci_dev *dev)
> +{
> + unsigned limit = dev->vpd->size;
> + unsigned addr = 0;
> +
> + /* Follow chain of resources until we find the end tag, a
> + * resource size that would overflow the limit, or an I/O
> + * error.
> + */
> + for (;;) {
> + u8 tag;
> + __le16 res_size_le;
> + unsigned res_size;
> +
> + if (dev->vpd->ops->read(dev, addr, 1, (char *)&tag) != 1)
> + break;
> + if (tag & 0x80) {
> + if (addr > limit - 3)
> + break;
> + if (dev->vpd->ops->read(dev, addr + 1, 1,
> + (char *)&res_size_le) +
> + dev->vpd->ops->read(dev, addr + 2, 1,
> + (char *)&res_size_le + 1) != 2)
> + break;
> + res_size = le16_to_cpu(res_size_le);
> + addr += 3;
> + } else {
> + res_size = tag & 7;
> + addr += 1;
> + if (res_size == 0)
> + break; /* valid ending tag */
> + }
> + if (addr >= limit - res_size)
> + break;
> + addr += res_size;
> + }
> +
> + dev->vpd->size = addr;
> +}
> +
> /**
> * pci_block_user_cfg_access - Block userspace PCI config reads/writes
> * @dev: pci device struct
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 6f3c744..3759fd3 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -736,7 +736,7 @@ int __must_check pci_create_sysfs_dev_files (struct pci_dev *pdev)
> attr = kzalloc(sizeof(*attr), GFP_ATOMIC);
> if (attr) {
> pdev->vpd->attr = attr;
> - attr->size = pdev->vpd->ops->get_size(pdev);
> + attr->size = pdev->vpd->size;
> attr->attr.name = "vpd";
> attr->attr.mode = S_IRUGO | S_IWUSR;
> attr->read = pci_read_vpd;
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 0a497c1..836a26b 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -21,12 +21,12 @@ extern int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
> struct pci_vpd_ops {
> int (*read)(struct pci_dev *dev, int pos, int size, char *buf);
> int (*write)(struct pci_dev *dev, int pos, int size, const char *buf);
> - int (*get_size)(struct pci_dev *dev);
> void (*release)(struct pci_dev *dev);
> };
>
> struct pci_vpd {
> struct pci_vpd_ops *ops;
> + unsigned size;
> struct bin_attribute *attr; /* descriptor for sysfs VPD entry */
> };
>
> @@ -36,6 +36,8 @@ static inline void pci_vpd_release(struct pci_dev *dev)
> if (dev->vpd)
> dev->vpd->ops->release(dev);
> }
> +/* Set VPD size by looking at its contents, limited to previous size */
> +extern void pci_vpd_set_real_size(struct pci_dev *dev);
>
> /* PCI /proc functions */
> #ifdef CONFIG_PROC_FS
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
