pci_read_bridge_bases bug and sign extend problems

From: Todd Inglett (tinglett@vnet.ibm.com)
Date: Thu Sep 06 2001 - 15:57:44 EST


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

--- 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);

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



This archive was generated by hypermail 2b29 : Fri Sep 07 2001 - 21:00:37 EST