linux-kernel - Re: [PATCH v2 4/5] PCI: generic: Correct, and avoid overflow, in bus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 23 Sep 2015 20:47:53 +0100
From:	Will Deacon <will.deacon@....com>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	David Daney <ddaney@...iumnetworks.com>,
	Mark Rutland <Mark.Rutland@....com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Pawel Moll <Pawel.Moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Marc Zyngier <Marc.Zyngier@....com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	David Daney <david.daney@...ium.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	David Daney <ddaney.cavm@...il.com>,
	Kumar Gala <galak@...eaurora.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>
Subject: Re: [PATCH v2 4/5] PCI: generic: Correct, and avoid overflow, in
 bus_max calculation.

On Wed, Sep 23, 2015 at 08:39:27PM +0100, Arnd Bergmann wrote:
> On Wednesday 23 September 2015 20:35:45 Will Deacon wrote:
> > On Wed, Sep 23, 2015 at 08:27:41PM +0100, Arnd Bergmann wrote:
> > > On Wednesday 23 September 2015 11:21:56 David Daney wrote:
> > > > >>
> > > > >>      /* Limit the bus-range to fit within reg */
> > > > >> -    bus_max = pci->cfg.bus_range->start +
> > > > >> -              (resource_size(&pci->cfg.res) >> pci->cfg.ops.bus_shift) - 1;
> > > > >> +    bus_max = (resource_size(&pci->cfg.res) >> pci->cfg.ops.bus_shift) - 1;
> > > > >> +    if (bus_max > 255)
> > > > >> +            bus_max = 255;
> > > > >
> > > > > I still don't understand the need for this part. If the cfg space is bigger
> > > > > than bus_max, isn't that simply an invalid resource? Given that the resource
> > > > > could be broken in other ways too, this check feels more like a specific
> > > > > workaround rather than generally useful code.
> > > > 
> > > > Imagine...
> > > > 
> > > >    bus-range [0x80 .. 0xff], this requires a cfg.res that will cover the 
> > > > entire range of 0..0xff.
> > > > 
> > > >    according to the calculations above, (resource_size(&pci->cfg.res) >> 
> > > > pci->cfg.ops.bus_shift) - 1 will have a value of 0xff, so...
> > > 
> > > Extending the computation to 32 bit seems fine, but I'd rather warn loudly
> > > if the bus range does not fit within the registers.
> > > 
> > > Also note that the computation is already correct with my interpretation
> > > of the reg property.
> > 
> > From what Lorenzo was saying, ACPI shares the interpretation that David is
> > implementing here and, given that the DT version seems to be subjective,
> > aligning this reg property with MMCFG seems to make sense.
> 
> We should then make it very clear in the binding that the driver
> is not allowed to actually map the registers for the buses outside
> of the bus-range, as that is highly unusual.
> 
> We would also need a special exception for this if we ever get to
> implement the DT source checker that we have been talking about for
> years, as the reg property might then overlap with a property from
> another device.

Completely agreed. Having a base that isn't actually safe to map is horrible
and should be called out.

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/