Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1293308

Re: [PATCHv2] pci: Update VPD size with correct length

Path csiph.com!eternal-september.org!feeder.eternal-september.org!aioe.org!bofh.it!news.nic.it!robomod
From Hannes Reinecke <hare@suse.de>
Newsgroups linux.kernel
Subject Re: [PATCHv2] pci: Update VPD size with correct length
Date Wed, 16 Dec 2015 20:00:02 +0100
Message-ID <qGpsS-5tw-3@gated-at.bofh.it> (permalink)
References <qGhYm-Gy-21@gated-at.bofh.it> <qGnKq-4Bl-31@gated-at.bofh.it> <qGnU5-4Ey-7@gated-at.bofh.it>
X-Original-To Alexander Duyck <alexander.duyck@gmail.com>
Organization SUSE Linux GmbH
User-Agent KMail/4.14.10 (Linux/4.1.13-5-default; KDE/4.14.10; x86_64; ; )
MIME-Version 1.0
Content-Transfer-Encoding 8BIT
Content-Type text/plain; charset="iso-8859-1"
Sender robomod@news.nic.it
List-ID <linux-kernel.vger.kernel.org>
X-Mailing-List linux-kernel@vger.kernel.org
Approved robomod@news.nic.it
Lines 72
X-Original-Cc Bjorn Helgaas <bhelgaas@google.com>, "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Michal Kubecek <mkubecek@suse.com>
X-Original-Date Wed, 16 Dec 2015 19:55:41 +0100
X-Original-Message-ID <1571929.py3SMrb2RG@lennox.hannes-reinecke.de>
X-Original-References <1450262992-77276-1-git-send-email-hare@suse.de> <4468155.Bdxh8senif@lennox.hannes-reinecke.de> <CAKgT0UcT_FxpjFUFF-HELHU7v_g_AcnPTg4tHWt3FsWO4G3fZg@mail.gmail.com>
X-Original-Sender linux-kernel-owner@vger.kernel.org
Xref csiph.com linux.kernel:1293308

Show key headers only | View raw


On Wednesday, December 16, 2015 09:13:35 AM Alexander Duyck wrote:
> On Wed, Dec 16, 2015 at 9:01 AM, Hannes Reinecke <hare@suse.de> wrote:
> > On Wednesday, December 16, 2015 08:52:10 AM Alexander Duyck wrote:
> >> On Wed, Dec 16, 2015 at 2:49 AM, Hannes Reinecke <hare@suse.de> wrote:
> >> > +               if (header[0] == 0xff) {
> >> > +                       /* Invalid data from VPD read */
> >> > +                       tag = header[0];
> >> > +               } else if (header[0] & 0x80) {
> >> > +                       /* Large Resource Data Type Tag */
> >> > +                       if (pci_read_vpd(dev, off+1, 2, &header[1]) !=
> >> > 2)
> >> > +                               return off + 1;
> >> > +                       off += 3 + ((header[2] << 8) | header[1]);
> >> > +                       tag = (header[0] & 0x7f);
> >> > +               } else {
> >> > +                       /* Short Resource Data Type Tag */
> >> > +                       off += 1 + (header[0] & 0x07);
> >> > +                       tag = (header[0] & 0x78) >> 3;
> >> > +               }
> >> > +               if (tag == 0x0f)        /* End tag descriptor */
> >> > +                       break;
> >> 
> >> It might make sense to just use the "return off" here since this is
> >> the only spot that should be returning the offset.
> > 
> > Which I'm not sure about.
> > We have three cases to worry about:
> > a) return after the 'end' tag
> > b) return after failing to read the 'end' tag
> > c) return after reading an invalid tag
> > 
> > For a) we obviously have to return the size.
> > But for b) and c)?
> > Just returning the maximal size (= old_size) would be exposing
> > invalid data to userland, with the possibility of hanging the system
> > by just reading from the attribute.
> > So to avoid that I've been returning the size of valid data.
> > 
> > But I'm open to suggestions if you think that's wrong.
> 
> If you didn't encounter an end tag how can you be sure you have valid
> data?  Maybe the random data managed to work out for the first couple
> of reads and then suddenly failed.  You might have a block of data
> that is valid for half of something like the read-only area and is
> going to return garbage data starting part way through.  I'd say you
> should handle this with an all-or-nothing type approach in order to
> err on the side of caution.  We could then see about white listing in
> those rare cases where a tag is missing using something like PCI quirk
> since we likely cannot use a parsing based approach if we cannot find
> the end tag.

Fair enough.

The only 'error' cases I've encountered so far is a read of all zeroes (and a 
halting the machine once you've read beyond a certain point) or a read of 0xff 
throughout the entire area. So that approach would work for both of them.

I'll be updating the patch.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		 zSeries & Storage
hare@suse.de		+49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCHv2] pci: Update VPD size with correct length Hannes Reinecke <hare@suse.de> - 2015-12-16 12:00 +0100
  Re: [PATCHv2] pci: Update VPD size with correct length Alexander Duyck <alexander.duyck@gmail.com> - 2015-12-16 18:00 +0100
    Re: [PATCHv2] pci: Update VPD size with correct length Hannes Reinecke <hare@suse.de> - 2015-12-16 18:10 +0100
      Re: [PATCHv2] pci: Update VPD size with correct length Alexander Duyck <alexander.duyck@gmail.com> - 2015-12-16 18:20 +0100
        Re: [PATCHv2] pci: Update VPD size with correct length Hannes Reinecke <hare@suse.de> - 2015-12-16 20:00 +0100
          RE: [PATCHv2] pci: Update VPD size with correct length "Seymour, Shane M" <shane.seymour@hpe.com> - 2015-12-17 06:30 +0100

csiph-web