Skip to contentSkip to conversation

x86/PCI: MMCONFIG: shift wrapping in mcfg_ioremap()

PATCH In review Dan Carpenter 3 participants 3 messages Jun 18, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 x86/PCI: MMCONFIG: shift wrapping in mcfg_ioremap()

This is a static checker fix. "size" is a u64 here, but "num_buses" is only 32 bits so we would hit shift wrapping bugs trying to use larger sizes.

arch/x86/pci/mmconfig_64.c +1 -2
index bea5249..430ed2b 100644
@@ -98,8 +98,7 @@ const struct pci_raw_ops pci_mmcfg = {
 static void __iomem *mcfg_ioremap(struct pci_mmcfg_region *cfg)
 {
 	void __iomem *addr;
-	u64 start, size;
-	int num_buses;
+	u64 start, size, num_buses;
 
 	start = cfg->address + PCI_MMCFG_BUS_OFFSET(cfg->start_bus);
 	num_buses = cfg->end_bus - cfg->start_bus + 1;
maintainer
#1 lore raw
Quoted 11 lines · Dan Carpenter

This is a static checker fix. "size" is a u64 here, but "num_buses" is only 32 bits so we would hit shift wrapping bugs trying to use larger sizes.

arch/x86/pci/mmconfig_64.c +0 -0
index bea5249..430ed2b 100644

arch/x86/pci/mmconfig_64.c const struct pci_raw_ops pci_mmcfg = { · line 104

Quoted 9 lines
arch/x86/pci/mmconfig_64.c +1 -2
@@ -98,8 +98,7 @@ const struct pci_raw_ops pci_mmcfg = {
 static void __iomem *mcfg_ioremap(struct pci_mmcfg_region *cfg)
 {
        void __iomem *addr;
-       u64 start, size;
-       int num_buses;
+       u64 start, size, num_buses;
 
        start = cfg->address + PCI_MMCFG_BUS_OFFSET(cfg->start_bus);
        num_buses = cfg->end_bus - cfg->start_bus + 1;

Is there an actual bug here? I don't see it, but I'm not a C language lawyer.

I guess I'm not necessarily opposed to changing it just to satisfy checkers, but I'd like to be able to figure out the correct types to use in the first place, and I don't know to figure this one out.

Bjorn

#2 lore raw
In reply to Dan Carpenter
Quoted 12 lines · Dan Carpenter

This is a static checker fix. "size" is a u64 here, but "num_buses" is only 32 bits so we would hit shift wrapping bugs trying to use larger sizes.

arch/x86/pci/mmconfig_64.c +0 -0
index bea5249..430ed2b 100644

arch/x86/pci/mmconfig_64.c const struct pci_raw_ops pci_mmcfg = { · line 104

Quoted 9 lines
arch/x86/pci/mmconfig_64.c +1 -2
@@ -98,8 +98,7 @@ const struct pci_raw_ops pci_mmcfg = {
 static void __iomem *mcfg_ioremap(struct pci_mmcfg_region *cfg)
 {
 	void __iomem *addr;
-	u64 start, size;
-	int num_buses;
+	u64 start, size, num_buses;
 
 	start = cfg->address + PCI_MMCFG_BUS_OFFSET(cfg->start_bus);
 	num_buses = cfg->end_bus - cfg->start_bus + 1;

This seems crazy. PCI is limited to 256 busses per domain, so I suspect this is a non-issue.

-hpa