x86/PCI: Prevent mmconfig memory corruption
authorThomas Gleixner <tglx@linutronix.de>
Thu, 25 Feb 2010 15:42:11 +0000 (16:42 +0100)
committerJesse Barnes <jbarnes@virtuousgeek.org>
Thu, 25 Feb 2010 16:30:58 +0000 (08:30 -0800)
commit ff097ddd4 (x86/PCI: MMCONFIG: manage pci_mmcfg_region as a
list, not a table) introduced a nasty memory corruption when
pci_mmcfg_list is empty.

pci_mmcfg_check_end_bus_number() dereferences pci_mmcfg_list.prev even
when the list is empty. The following write hits some variable near to
pci_mmcfg_list.

Further down a similar problem exists, where cfg->list.next is
dereferenced unconditionally and a comparison with some variable near
to pci_mmcfg_list happens.

Add a check for the last element into the for_each_entry() loop and
remove all the other crappy logic which is just a leftover of the old
array based code which was replaced by the list conversion.

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Bjorn Helgaas <bjorn.helgaas@hp.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: stable@kernel.org
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
arch/x86/pci/mmconfig-shared.c

index b19d1e54201ee95a80d8123e9ba26716811c52fe..8f3f9a50b1e0b4891be7fbba0d62ee16bff6a273 100644 (file)
@@ -303,22 +303,17 @@ static void __init pci_mmcfg_check_end_bus_number(void)
 {
        struct pci_mmcfg_region *cfg, *cfgx;
 
-       /* last one*/
-       cfg = list_entry(pci_mmcfg_list.prev, typeof(*cfg), list);
-       if (cfg)
-               if (cfg->end_bus < cfg->start_bus)
-                       cfg->end_bus = 255;
-
-       if (list_is_singular(&pci_mmcfg_list))
-               return;
-
-       /* don't overlap please */
+       /* Fixup overlaps */
        list_for_each_entry(cfg, &pci_mmcfg_list, list) {
                if (cfg->end_bus < cfg->start_bus)
                        cfg->end_bus = 255;
 
+               /* Don't access the list head ! */
+               if (cfg->list.next == &pci_mmcfg_list)
+                       break;
+
                cfgx = list_entry(cfg->list.next, typeof(*cfg), list);
-               if (cfg != cfgx && cfg->end_bus >= cfgx->start_bus)
+               if (cfg->end_bus >= cfgx->start_bus)
                        cfg->end_bus = cfgx->start_bus - 1;
        }
 }