On Fri, Jul 16, 2021 at 08:03:45AM +0200, Hannes Reinecke wrote:
> On 7/15/21 11:59 PM, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> >
> > VPD consists of a series of Small and Large Resources. Computing the size
> > of VPD requires only the length of each, which is specified in the generic
> > tag of each resource. We only expect to see ID_STRING, RO_DATA, and
> > RW_DATA in VPD, but it's not a problem if it contains other resource types.
> >
> > Drop the validity checking of Large Resource items.
> >
> > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > ---
> > drivers/pci/vpd.c | 37 ++++++++++++++-----------------------
> > 1 file changed, 14 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c
> > index 9c2744d79b53..d7a4a9f05bd6 100644
> > --- a/drivers/pci/vpd.c
> > +++ b/drivers/pci/vpd.c
> > @@ -82,31 +82,22 @@ static size_t pci_vpd_size(struct pci_dev *dev, size_t old_size)
> > if (header[0] & PCI_VPD_LRDT) {
> > /* Large Resource Data Type Tag */
> > tag = pci_vpd_lrdt_tag(header);
> > - /* Only read length from known tag items */
> > - if ((tag == PCI_VPD_LTIN_ID_STRING) ||
> > - (tag == PCI_VPD_LTIN_RO_DATA) ||
> > - (tag == PCI_VPD_LTIN_RW_DATA)) {
> > - if (pci_read_vpd(dev, off+1, 2,
> > - &header[1]) != 2) {
> > - pci_warn(dev, "failed VPD read at offset %zu",
> > - off + 1);
> > - return 0;
> > - }
> > - size = pci_vpd_lrdt_size(header);
> > -
> > - /*
> > - * Missing EEPROM may read as 0xff.
> > - * Length of 0xffff (65535) cannot be valid
> > - * because VPD can't be that large.
> > - */
> > - if (size > PCI_VPD_MAX_SIZE)
> > - goto error;
> > - off += PCI_VPD_LRDT_TAG_SIZE + size;
> > - } else {
> > - pci_warn(dev, "invalid large VPD tag %02x at offset %zu",
> > - tag, off);
> > + if (pci_read_vpd(dev, off + 1, 2, &header[1]) != 2) {
> > + pci_warn(dev, "failed VPD read at offset %zu",
> > + off + 1);
> > return 0;
> > }
> > + size = pci_vpd_lrdt_size(header);
> > +
> > + /*
> > + * Missing EEPROM may read as 0xff. Length of
> > + * 0xffff (65535) cannot be valid because VPD can't
> > + * be that large.
> > + */
> > + if (size > PCI_VPD_MAX_SIZE)
> > + goto error;
> > +
> > + off += PCI_VPD_LRDT_TAG_SIZE + size;
> > } else {
> > /* Short Resource Data Type Tag */
> > tag = pci_vpd_srdt_tag(header);
> >
> I'm not entirely happy with this; we really have to rely on well-formed VPD
> tags for the protocol to work correctly, and that's why I took the cautious
> approach. But with the check for missing EEPROM I hope we've covered the
> most common causes for invalid tags. Let's see how it goes.
True. The tags need to be well-formed in the sense of having
intelligible lengths so we can identify the beginning and end of each
resource. But we don't actually depend on the resource name
(ID_STRING, etc) or the content.
> Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes Reinecke Kernel Storage Architect
> hare@xxxxxxx +49 911 74053 688
> SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
> HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
