On Wed, Nov 17, 2021 at 10:31:51PM +0100, Heiner Kallweit wrote: > On 13.10.2021 20:37, Heiner Kallweit wrote: > > We have a problem with a device where each VPD read returns 0x33 [0]. > > This results in a valid VPD structure (except the tag id) and > > therefore pci_vpd_size() scans the full VPD address range. > > On an affected system this took ca. 80s. > > > > That's not acceptable, on the other hand we may not want to re-add > > the old tag checks. In addition these tag check still wouldn't be able > > to avoid the described scenario 100%. > > Instead let's add a simple sanity check on the number of found tags. > > A VPD image conforming to the PCI spec [1] can have max. 4 tags: > > id string, ro section, rw section, end tag. > > > > [0] https://lore.kernel.org/lkml/20210915223218.GA1542966@bjorn-Precision-5520/ > > [1] PCI 3.0 I.3.1. VPD Large and Small Resource Data Tags > > > > Reviewed-by: Krzysztof Wilczyński <kw@xxxxxxxxx> > > Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx> > > --- > > drivers/pci/vpd.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c > > index a4fc4d069..921470611 100644 > > --- a/drivers/pci/vpd.c > > +++ b/drivers/pci/vpd.c > > @@ -56,6 +56,7 @@ static size_t pci_vpd_size(struct pci_dev *dev) > > { > > size_t off = 0, size; > > unsigned char tag, header[1+2]; /* 1 byte tag, 2 bytes length */ > > + int num_tags = 0; > > > > while (pci_read_vpd_any(dev, off, 1, header) == 1) { > > size = 0; > > @@ -63,6 +64,10 @@ static size_t pci_vpd_size(struct pci_dev *dev) > > if (off == 0 && (header[0] == 0x00 || header[0] == 0xff)) > > goto error; > > > > + /* We can have max 4 tags: STRING_ID, RO, RW, END */ > > + if (++num_tags > 4) > > + goto error; > > + > > if (header[0] & PCI_VPD_LRDT) { > > /* Large Resource Data Type Tag */ > > if (pci_read_vpd_any(dev, off + 1, 2, &header[1]) != 2) { > > > > Can this one be picked up for next? I'm hesitating because we (or maybe just "I" :)) worked so hard to avoid interpreting the VPD data, and now we're back to that. There's nothing of value in this particular device's VPD. Is there any reason we shouldn't just use quirk_blacklist_vpd() for it? Bjorn
