linux-kernel - Re: [patch 2/2] PNP: don't check disabled PCI BARs for conflicts in quirk_system_pci

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: <48E11EFA.8010402@keyaccess.nl>
Date:	Mon, 29 Sep 2008 20:31:22 +0200
From:	Rene Herman <rene.herman@...access.nl>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	Bjorn Helgaas <bjorn.helgaas@...com>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Len Brown <lenb@...nel.org>, Frans Pop <elendil@...net.nl>,
	"Rafael J. Wysocki" <rjw@...k.pl>, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, linux-acpi@...r.kernel.org,
	Adam Belay <abelay@....edu>, Avuton Olrich <avuton@...il.com>,
	Karl Bellve <karl.bellve@...ssmed.edu>,
	Willem Riede <wriede@...de.org>,
	Matthew Hall <mhall@...omputing.net>
Subject: Re: [patch 2/2] PNP: don't check disabled PCI BARs for conflicts
 in quirk_system_pci_resources()

On 29-09-08 18:34, Linus Torvalds wrote:

> On Mon, 29 Sep 2008, Bjorn Helgaas wrote:
>>  
>> +			if (!pci_resource_enabled(pdev, i))
>> +				continue;
> 
> I really don't think this is the right approach.
> 
> Maybe the PCI device has been turned off, but the *resource* may still be 
> valid.
> 
> Wouldn't it be much better to just check whether the resource is inserted 
> in the resource tree or not? 
> 
> Quite frankly, it looks like your change will basically cause us to look 
> over *every* system PnP resource, and for each of them, it will look at 
> *every* PCI device, and for each PCI device it will look at *every* BAR, 
> and for each BAR it finds it will read the PCI status register, over and 
> over and over again.
> 
> Now, I doubt you'll be able to wear out the PCI bus, but doesn't this just 
> make you go "umm, that's not pretty, and it doesn't make much sense".
> 
> If we've detected the PCI resource as being valid by the PCI layer, why 
> not just use that information? And afaik, the easy way to check that is 
> just whether it's inserted into the resource tree, which in turn is most 
> trivially done by just checking whether the resource has a parent.
> 
> IOW, why isn't it just doing
> 
> 	struct resource *res = dev->resource[bar];
> 
> 	if (!res->parent)
> 		continue;
> 
> or something? Or what was wrong with just checking the res->start for 
> being zero? Wherever PnP is relevant, resources that start at zero are 
> disabled, no? 

I believe the possible issue is that resources that do _not_ (seem to) 
start at zero might also be disabled.

Bjorn commented that pci_resource_start() returns a CPU address for I/O 
which might not be the actual I/O address on some platforms. I haven't a 
clue if that's actually possible "wherever PnP is relevent" as you put 
it but that seems to otherwise make sense.

If it does though, it might for all I know also be possible to check 
against some ARCH_SPECIFIC_INVALID_IO_ADDRESS instead of plain unadorned 
0 (or just recheck the actual BAR again if not stored anywhere).

But that's the issue as I understood it: we might miss them on some 
platforms if checking against 0...

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