pci_read_bridge_bases bug and sign extend problems

pci_read_bridge_bases bug and sign extend problems

[Todd Inglett]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

In 2.4.9-ac7 I've found the following two problems in
pci_read_bridge_bases().  First, the code is testing "base" for the
PCI_IO_RANGE_TYPE_32 when it should be testing "io_base_lo" (base is
already shifted up 8 bits).  The tests on the mem bases are ok.

Second, the code does some shifting which is not safe when sizeof(long)
< sizeof(int) (i.e. 64 bit).  As I understand it (and observe) in the
assignment like "base = (mem_base_lo << 16)" the u16 mem_base_lo gets
promoted to int before the assignment.  The resulting signed int gets
sign extended to fit into the unsigned long base.  The solution is to
cast the u16's to unsigned long.

Finally, it seems odd that resource[1] here is handled differently than
resource[2].  I'm not a pci wizard, though, so I may be missing
something.  But recently this code has been patched to handle io range
types better and it seems resource[1] was missed.  I don't include
anything in the patch for this, though.

Patch attached (relative to 2.4.9-ac7 though ac9 appears the same).
-- 
-todd

[-- Attachment #2: pci.patch --]
[-- Type: text/plain, Size: 2352 bytes --]

--- drivers/pci/pci.c	27 Aug 2001 19:06:08 -0000	1.8
+++ drivers/pci/pci.c	6 Sep 2001 19:23:57 -0000
@@ -967,10 +967,10 @@
 	res = child->resource[0];
 	pci_read_config_byte(dev, PCI_IO_BASE, &io_base_lo);
 	pci_read_config_byte(dev, PCI_IO_LIMIT, &io_limit_lo);
-	base = (io_base_lo & PCI_IO_RANGE_MASK) << 8;
-	limit = (io_limit_lo & PCI_IO_RANGE_MASK) << 8;
+	base = (unsigned long)(io_base_lo & PCI_IO_RANGE_MASK) << 8;
+	limit = (unsigned long)(io_limit_lo & PCI_IO_RANGE_MASK) << 8;
 
-	if ((base & PCI_IO_RANGE_TYPE_MASK) == PCI_IO_RANGE_TYPE_32) {
+	if ((io_base_lo & PCI_IO_RANGE_TYPE_MASK) == PCI_IO_RANGE_TYPE_32) {
 		u16 io_base_hi, io_limit_hi;
 		pci_read_config_word(dev, PCI_IO_BASE_UPPER16, &io_base_hi);
 		pci_read_config_word(dev, PCI_IO_LIMIT_UPPER16, &io_limit_hi);
@@ -995,8 +995,8 @@
 	res = child->resource[1];
 	pci_read_config_word(dev, PCI_MEMORY_BASE, &mem_base_lo);
 	pci_read_config_word(dev, PCI_MEMORY_LIMIT, &mem_limit_lo);
-	base = (mem_base_lo & PCI_MEMORY_RANGE_MASK) << 16;
-	limit = (mem_limit_lo & PCI_MEMORY_RANGE_MASK) << 16;
+	base = (unsigned long)(mem_base_lo & PCI_MEMORY_RANGE_MASK) << 16;
+	limit = (unsigned long)(mem_limit_lo & PCI_MEMORY_RANGE_MASK) << 16;
 	if (base && base <= limit) {
 		res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM;
 		res->start = base;
@@ -1011,16 +1011,16 @@
 	res = child->resource[2];
 	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
 	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
-	base = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
-	limit = (mem_limit_lo & PCI_PREF_RANGE_MASK) << 16;
+	base = (unsigned long)(mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
+	limit = (unsigned long)(mem_limit_lo & PCI_PREF_RANGE_MASK) << 16;
 
 	if ((mem_base_lo & PCI_PREF_RANGE_TYPE_MASK) == PCI_PREF_RANGE_TYPE_64) {
 		u32 mem_base_hi, mem_limit_hi;
 		pci_read_config_dword(dev, PCI_PREF_BASE_UPPER32, &mem_base_hi);
 		pci_read_config_dword(dev, PCI_PREF_LIMIT_UPPER32, &mem_limit_hi);
 #if BITS_PER_LONG == 64
-		base |= ((long) mem_base_hi) << 32;
-		limit |= ((long) mem_limit_hi) << 32;
+		base |= ((unsigned long) mem_base_hi) << 32;
+		limit |= ((unsigned long) mem_limit_hi) << 32;
 #else
 		if (mem_base_hi || mem_limit_hi) {
 			printk(KERN_ERR "PCI: Unable to handle 64-bit address space for %s\n", child->name);