linux-kernel - Re: [RFC/PATCH 4/4] [POWERPC] pci: Disable IO/Mem on a device when resources can't be allocated

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]

Message-Id: <1198617653.7209.4.camel@pasglop>
Date:	Wed, 26 Dec 2007 08:20:53 +1100
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	Grant Grundler <grundler@...isc-linux.org>
Cc:	linux-pci@...ey.karlin.mff.cuni.cz, Alan Cox <alan@...hat.com>,
	Greg Kroah-Hartman <greg@...ah.com>, jgarzik@...ox.com,
	Ivan Kokshaysky <ink@...assic.park.msu.ru>,
	wingel@...o-system.com,
	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>,
	james.smart@...lex.com, linux-driver@...gic.com,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC/PATCH 4/4] [POWERPC] pci: Disable IO/Mem on a device when
	resources can't be allocated


On Mon, 2007-12-24 at 00:23 -0700, Grant Grundler wrote:
> On Tue, Dec 18, 2007 at 10:01:15AM +1100, Benjamin Herrenschmidt wrote:
> > This patch changes the PowerPC PCI code to disable IO and/or Memory
> > decoding on a PCI device when a resource of that type failed to be
> > allocated. This is done to avoid having unallocated dangling BARs enabled
> > that might try to decode on top of other devices.
> > 
> > If a proper resource is assigned later on, then pci_enable_device{,_io,_mem}
> > will take care of re-enabling decoding.
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@...nel.crashing.org>
> ....
> > @@ -1062,8 +1065,12 @@ static void __init pcibios_allocate_reso
> >  				disabled = !(command & PCI_COMMAND_IO);
> >  			else
> >  				disabled = !(command & PCI_COMMAND_MEMORY);
> > -			if (pass == disabled)
> > -				alloc_resource(dev, idx);
> > +			if (pass == disabled && alloc_resource(dev, idx)) {
> > +				command &= ~(r->flags & (IORESOURCE_IO |
> > +							 IORESOURCE_MEM));
> 
> While this may be ok for PPC, in general, wouldn't we want to only disable
> which ever type of resource that couldn't be allocated?

This is exactly what's supposed to be happening, but the code is buggy
and nobody noticed :-) (I'm mixing up IORESOURCE_* flags and
PCI_COMMAND_* flags). Thanks for reviewing !

> ie make two calls: alloc_resource_io() and alloc_resource_mem() and disable
> the respective flag if the alloc call fails?

No need for 2 calls, just disable whatever type the resource is, but
using the right bits instead of what my code incorrectly does.

> Thus a device which was enable and programmed by BIOS could remain functional
> despite one resource not being allocated.

Yes, that's what intended by the above code, if I didn't have mixed up
the flags.

Ben.


--
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/