Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934501AbbLPSzp (ORCPT ); Wed, 16 Dec 2015 13:55:45 -0500 Received: from mx2.suse.de ([195.135.220.15]:46300 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750727AbbLPSzo convert rfc822-to-8bit (ORCPT ); Wed, 16 Dec 2015 13:55:44 -0500 From: Hannes Reinecke To: Alexander Duyck Cc: Bjorn Helgaas , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Michal Kubecek Subject: Re: [PATCHv2] pci: Update VPD size with correct length Date: Wed, 16 Dec 2015 19:55:41 +0100 Message-ID: <1571929.py3SMrb2RG@lennox.hannes-reinecke.de> Organization: SUSE Linux GmbH User-Agent: KMail/4.14.10 (Linux/4.1.13-5-default; KDE/4.14.10; x86_64; ; ) In-Reply-To: References: <1450262992-77276-1-git-send-email-hare@suse.de> <4468155.Bdxh8senif@lennox.hannes-reinecke.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="iso-8859-1" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3313 Lines: 72 On Wednesday, December 16, 2015 09:13:35 AM Alexander Duyck wrote: > On Wed, Dec 16, 2015 at 9:01 AM, Hannes Reinecke wrote: > > On Wednesday, December 16, 2015 08:52:10 AM Alexander Duyck wrote: > >> On Wed, Dec 16, 2015 at 2:49 AM, Hannes Reinecke 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/