From: Masayoshi Mizuma <m.miz...@jp.fujitsu.com> KASAN reported the following slab-out-of-bounds when sb_edac module was loaded on Broadwell machine which has two PCI segments.
================================================================== BUG: KASAN: slab-out-of-bounds in sbridge_get_all_devices.constprop.14+0x75f/0x96a [sb_edac] Read of size 8 at addr ffff8c0d44dfe850 by task modprobe/4221 CPU: 19 PID: 4221 Comm: modprobe Not tainted 4.18.0-rc5 #2 Call Trace: dump_stack+0xc2/0x16b ? show_regs_print_info+0x5/0x5 ? kmsg_dump_rewind_nolock+0xd9/0xd9 ? pci_get_dev_by_id+0x57/0x70 ? pci_get_device+0x155/0x210 print_address_description+0x6a/0x270 kasan_report+0x258/0x380 ? sbridge_get_all_devices.constprop.14+0x75f/0x96a [sb_edac] sbridge_get_all_devices.constprop.14+0x75f/0x96a [sb_edac] ... Allocated by task 4221: kasan_kmalloc+0xa0/0xd0 __kmalloc+0x112/0x230 sbridge_get_all_devices.constprop.14+0x555/0x96a [sb_edac] sbridge_init+0x1ba/0x4000 [sb_edac] do_one_initcall+0xaa/0x401 do_init_module+0x20b/0x676 load_module+0x443f/0x6dc0 __do_sys_finit_module+0x25f/0x2c0 do_syscall_64+0x14e/0x4b0 entry_SYSCALL_64_after_hwframe+0x44/0xa9 ... ================================================================== The out bounds happened because the index of sbridge_dev->pdev[] is over than the allocated size. --- static int sbridge_get_onedevice(struct pci_dev **prev, ... if (sbridge_dev->pdev[sbridge_dev->i_devs]) { <= !! out bounds HERE !! sbridge_printk(KERN_ERR, "Duplicated device for %04x:%04x\n", PCI_VENDOR_ID_INTEL, dev_descr->dev_id); --- If a sbridge_dev which has the same bus as the new device is already exists, the sbridge_dev is assigned to the new one. --- static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bus, struct sbridge_dev *prev) ... list_for_each_entry_from(sbridge_dev, &sbridge_edac_list, list) { if (sbridge_dev->bus == bus && (dom == SOCK || dom == sbridge_dev->dom)) return sbridge_dev; } --- If the system has multi PCI segment and the each bus number is same, the same sbridge_dev is assigned. So, the index of sbridge_dev->pdev[] is incremented by each segments. As the result, the index is over than the allocated size, then the out-of-bounds happens. This patch introduces a segment member to sbridge_dev to separate it each PCI segments. Fixes: e2f747b1f42a ("EDAC, sb_edac: Assign EDAC memory controller per h/w controller") Signed-off-by: Masayoshi Mizuma <m.miz...@jp.fujitsu.com> --- drivers/edac/sb_edac.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c index 4a89c80..07726fb 100644 --- a/drivers/edac/sb_edac.c +++ b/drivers/edac/sb_edac.c @@ -352,6 +352,7 @@ struct pci_id_table { struct sbridge_dev { struct list_head list; + int seg; u8 bus, mc; u8 node_id, source_id; struct pci_dev **pdev; @@ -729,7 +730,8 @@ static inline int numcol(u32 mtr) return 1 << cols; } -static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bus, +static struct sbridge_dev *get_sbridge_dev(int seg, u8 bus, enum domain dom, + int multi_bus, struct sbridge_dev *prev) { struct sbridge_dev *sbridge_dev; @@ -747,14 +749,15 @@ static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bu : sbridge_edac_list.next, struct sbridge_dev, list); list_for_each_entry_from(sbridge_dev, &sbridge_edac_list, list) { - if (sbridge_dev->bus == bus && (dom == SOCK || dom == sbridge_dev->dom)) + if ((sbridge_dev->seg == seg) && (sbridge_dev->bus == bus) && + (dom == SOCK || dom == sbridge_dev->dom)) return sbridge_dev; } return NULL; } -static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom, +static struct sbridge_dev *alloc_sbridge_dev(int seg, u8 bus, enum domain dom, const struct pci_id_table *table) { struct sbridge_dev *sbridge_dev; @@ -771,6 +774,7 @@ static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom, return NULL; } + sbridge_dev->seg = seg; sbridge_dev->bus = bus; sbridge_dev->dom = dom; sbridge_dev->n_devs = table->n_devs_per_imc; @@ -2246,6 +2250,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev, struct sbridge_dev *sbridge_dev = NULL; const struct pci_id_descr *dev_descr = &table->descr[devno]; struct pci_dev *pdev = NULL; + int seg = 0; u8 bus = 0; int i = 0; @@ -2276,10 +2281,12 @@ static int sbridge_get_onedevice(struct pci_dev **prev, /* End of list, leave */ return -ENODEV; } + seg = pci_domain_nr(pdev->bus); bus = pdev->bus->number; next_imc: - sbridge_dev = get_sbridge_dev(bus, dev_descr->dom, multi_bus, sbridge_dev); + sbridge_dev = get_sbridge_dev(seg, bus, dev_descr->dom, + multi_bus, sbridge_dev); if (!sbridge_dev) { /* If the HA1 wasn't found, don't create EDAC second memory controller */ if (dev_descr->dom == IMC1 && devno != 1) { @@ -2292,7 +2299,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev, if (dev_descr->dom == SOCK) goto out_imc; - sbridge_dev = alloc_sbridge_dev(bus, dev_descr->dom, table); + sbridge_dev = alloc_sbridge_dev(seg, bus, dev_descr->dom, table); if (!sbridge_dev) { pci_dev_put(pdev); return -ENOMEM; -- 2.18.0