public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Todd Inglett <tinglett@vnet.ibm.com>
To: linux-kernel@vger.kernel.org
Subject: pci_read_bridge_bases bug and sign extend problems
Date: Thu, 06 Sep 2001 15:57:44 -0500	[thread overview]
Message-ID: <3B97E348.9E3E7A18@vnet.ibm.com> (raw)

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

                 reply	other threads:[~2001-09-06 20:57 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3B97E348.9E3E7A18@vnet.ibm.com \
    --to=tinglett@vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox