linux-kernel - Re: a7839e96 (PNP: increase max resources) breaks my ALSA intel8x0 sound card

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:	Thu, 14 Feb 2008 16:40:36 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Bjorn Helgaas <bjorn.helgaas@...com>
cc:	Robert Hancock <hancockr@...w.ca>,
	Andrew Morton <akpm@...ux-foundation.org>, avuton@...il.com,
	yakui.zhao@...el.com, shaohua.li@...el.com, trenn@...e.de,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	alsa-devel@...a-project.org
Subject: Re: a7839e96 (PNP: increase max resources) breaks my ALSA intel8x0
 sound card

On Thu, 14 Feb 2008, Bjorn Helgaas wrote:
>
> That makes sense, but ...  there are BIOSes that actually list nested
> resources, e.g., http://bugzilla.kernel.org/show_bug.cgi?id=9514#c29 :
> 
>   [000000290 - 000000294]  Motherboard resources
>   [000000290 - 00000029F]  Motherboard resources

Heh. 

Not that I can really see us ever being confused by this in practice. 
Sure, we'd skip the second resource (because it's busy), but that's what 
we used to do before anyway, wasn't it? And even if we do, there's very 
little reason we'd ever actually try to reserve that 294-29f range.

In practice, the only resources we actually tend to end up really 
allocating tends to be things like Cardbus (or a whole PCI bus either due 
to hotplug or due to the BIOS not bothering), which tend want a big window 
for the bus window, so the "off-by-one" kinds of things wouldn't really 
worry me. Very seldom do we actually have small resources to worry about 
on a PC platform, since they all tend to be pre-allocated by the BIOS 
anyway.

That said, if we really want to handle this, we could certainly add a 
whole new ioresource flag bit that says "allow inserting resources into 
this", and we could set that bit not just for the PnP reservations 
themselves, but in PCI bus resources too.

Basically, there are two different ways of inserting a resource:

 - the "register this subresource" thing that "request_resource()" does, 
   which just works on the one given resource and honors the BUSY bit.

 - the "insert this resource into the tree" (insert_resource()) that 
   starts from the root and tries to find the right location. It honors 
   the BUSY bit too, but we could extend it to _only_ extend into 
   resources that allow it.

Here's the trivial patch to add the infrastructure for this, but I did 
*not* do the required "mark PCI bus resources as IORESOURCE_INSERT" etc 
(nor obviously the markers to make PnP resources themselves be marked as 
"insert").

But if you want to play around with it, I think this is at least 
*conceptually* a sane idea. Another way to see it is that a resource with 
the IORESOURCE_INSERT bit is "subdivisible", and then it's very obvious 
that a normal PCI device resource should not have it set: it is a "leaf" 
resources that can not be split up.

So the only way to populate "leaf" resources is by explicitly specifying 
them and doing a "request_resource()" on the resource, while the non-leaf 
IORESOURCE_INSERT resources can be populated from below with 
ioresrouce_insert().

Is this over-designed? I dunno. The _implementation_ on a resource level 
is certainly trivial.

Oh, and if it wasn't obvious: the patch is untested. It compiles. That's 
all I'm going to say about it, especially since without marking any 
resources with the new IORESOURCE_INSERT bit, the only thing you can do 
with "insert_resource()" is to insert into the root level (because we 
don't check the level we are passing into)

		Linus

---
 include/linux/ioport.h |    1 +
 kernel/resource.c      |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 605d237..0a56410 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -45,6 +45,7 @@ struct resource_list {
 #define IORESOURCE_RANGELENGTH	0x00008000
 #define IORESOURCE_SHADOWABLE	0x00010000
 #define IORESOURCE_BUS_HAS_VGA	0x00080000
+#define IORESOURCE_INSERT	0x00100000	/* Allow insert_resource() */

 #define IORESOURCE_DISABLED	0x10000000
 #define IORESOURCE_UNSET	0x20000000
diff --git a/kernel/resource.c b/kernel/resource.c
index 82aea81..d6da786 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -397,6 +397,8 @@ int insert_resource(struct resource *parent, struct resource *new)
 		result = -EBUSY;
 		if (first == parent)
 			goto out;
+		if (!(first->flags & IORESOURCE_INSERT))
+			goto out;

 		if ((first->start > new->start) || (first->end < new->end))
 			break;
--
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/