Re: [PATCH] EDAC, sb_edac: Fix out of bound in PCI multi segment env
Hi Tony, Thank you for your review! On 07/24/2018 01:01 PM, Tony Luck wrote: > On Thu, Jul 19, 2018 at 7:07 AM, Masayoshi Mizuma > wrote: >> From: Masayoshi Mizuma >> >> KASAN reported the following slab-out-of-bounds when sb_edac >> module was loaded on Broadwell machine which has two PCI segments. > > Although you found this with KASAN as an out of bounds array reference, > the real problem is that the sb_edac.c driver didn't know about systems > with segmented PCI busses. > > So the Subject: for the e-mail (and thus the commit message) should > be: > > [PATCH] EDAC, sb_edac: Add support for systems with segmented PCI busses You are right. I will modify the subject and commit message, and resend it. > > Otherwise this looks fine. > > Reviewed-by: Tony Luck Thanks! Masa > > -Tony >
Re: [PATCH] EDAC, sb_edac: Fix out of bound in PCI multi segment env
Hi Tony, Thank you for your review! On 07/24/2018 01:01 PM, Tony Luck wrote: > On Thu, Jul 19, 2018 at 7:07 AM, Masayoshi Mizuma > wrote: >> From: Masayoshi Mizuma >> >> KASAN reported the following slab-out-of-bounds when sb_edac >> module was loaded on Broadwell machine which has two PCI segments. > > Although you found this with KASAN as an out of bounds array reference, > the real problem is that the sb_edac.c driver didn't know about systems > with segmented PCI busses. > > So the Subject: for the e-mail (and thus the commit message) should > be: > > [PATCH] EDAC, sb_edac: Add support for systems with segmented PCI busses You are right. I will modify the subject and commit message, and resend it. > > Otherwise this looks fine. > > Reviewed-by: Tony Luck Thanks! Masa > > -Tony >
Re: [PATCH] EDAC, sb_edac: Fix out of bound in PCI multi segment env
On Thu, Jul 19, 2018 at 7:07 AM, Masayoshi Mizuma wrote: > From: Masayoshi Mizuma > > KASAN reported the following slab-out-of-bounds when sb_edac > module was loaded on Broadwell machine which has two PCI segments. Although you found this with KASAN as an out of bounds array reference, the real problem is that the sb_edac.c driver didn't know about systems with segmented PCI busses. So the Subject: for the e-mail (and thus the commit message) should be: [PATCH] EDAC, sb_edac: Add support for systems with segmented PCI busses Otherwise this looks fine. Reviewed-by: Tony Luck -Tony
Re: [PATCH] EDAC, sb_edac: Fix out of bound in PCI multi segment env
On Thu, Jul 19, 2018 at 7:07 AM, Masayoshi Mizuma wrote: > From: Masayoshi Mizuma > > KASAN reported the following slab-out-of-bounds when sb_edac > module was loaded on Broadwell machine which has two PCI segments. Although you found this with KASAN as an out of bounds array reference, the real problem is that the sb_edac.c driver didn't know about systems with segmented PCI busses. So the Subject: for the e-mail (and thus the commit message) should be: [PATCH] EDAC, sb_edac: Add support for systems with segmented PCI busses Otherwise this looks fine. Reviewed-by: Tony Luck -Tony
[PATCH] EDAC, sb_edac: Fix out of bound in PCI multi segment env
From: Masayoshi Mizuma
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 8c0d44dfe850 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, _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
---
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_headlist;
+ 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, _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 = >descr[devno];
struct pci_dev *pdev = NULL;
+ int seg = 0;
u8 bus = 0;
int i = 0;
@@ -2276,10
[PATCH] EDAC, sb_edac: Fix out of bound in PCI multi segment env
From: Masayoshi Mizuma
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 8c0d44dfe850 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, _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
---
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_headlist;
+ 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, _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 = >descr[devno];
struct pci_dev *pdev = NULL;
+ int seg = 0;
u8 bus = 0;
int i = 0;
@@ -2276,10
