2015-02-17 03:37:57

by Daniel J Blueman

[permalink] [raw]
Subject: [PATCH v2] x86: Prevent oops with >16 memory controllers

When ECC interrupts occur on memory controllers after EDAC_MAX_MCS (16), the
kernel fatally dereferences unallocated structures [1]; this occurs on at
least NumaConnect systems.

Fix by checking if a memory controller info structure was found; candidate for
stable.

v1->2: Use edac_mc_find() as per Boris's suggestion

Signed-off-by: Daniel J Blueman <[email protected]>

-- [1]

BUG: unable to handle kernel NULL pointer dereference at 0000000000000320
IP: [<ffffffff819f714f>] decode_bus_error+0x2f/0x2b0
PGD 2f8b5a3067 PUD 2f8b5a2067 PMD 0
Oops: 0000 [#2] SMP
Modules linked in:
CPU: 224 PID: 11930 Comm: stream_c.exe.gn Tainted: G D 3.19.0 #1
Hardware name: Supermicro H8QGL/H8QGL, BIOS 3.5b 01/28/2015
task: ffff8807dbfb8c00 ti: ffff8807dd16c000 task.ti: ffff8807dd16c000
RIP: 0010:[<ffffffff819f714f>] [<ffffffff819f714f>] decode_bus_error+0x2f/0x2b0
RSP: 0000:ffff8907dfc03c48 EFLAGS: 00010297
RAX: 0000000000000001 RBX: 9c67400010080a13 RCX: 0000000000001dc6
RDX: 000000001dc61dc6 RSI: ffff8907dfc03df0 RDI: 000000000000001c
RBP: ffff8907dfc03ce8 R08: 0000000000000000 R09: 0000000000000022
R10: ffff891fffa30380 R11: 00000000001cfc90 R12: 0000000000000008
R13: 0000000000000000 R14: 000000000000001c R15: 00009c6740001000
FS: 00007fa97ee18700(0000) GS:ffff8907dfc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000320 CR3: 0000003f889b8000 CR4: 00000000000407e0
Stack:
0000000000000000 ffff8907dfc03df0 0000000000000008 9c67400010080a13
000000000000001c 00009c6740001000 ffff8907dfc03c88 ffffffff810e4f9a
ffff8907dfc03ce8 ffffffff81b375b9 0000000000000000 0000000000000010
Call Trace:
<IRQ>
[<ffffffff810e4f9a>] ? vprintk_default+0x1a/0x20
[<ffffffff81b375b9>] ? printk+0x41/0x43
[<ffffffff819f65ff>] amd_decode_mce+0x58f/0x8e0
[<ffffffff810c79ed>] notifier_call_chain+0x4d/0x80
[<ffffffff810c7a45>] atomic_notifier_call_chain+0x15/0x20
[<ffffffff8105352d>] mce_log+0x1d/0x130
[<ffffffff810537d4>] machine_check_poll+0x194/0x260
[<ffffffff81053ae6>] mce_timer_fn+0x116/0x140
[<ffffffff810539d0>] ? mce_cpu_restart+0x40/0x40
[<ffffffff810f17e7>] call_timer_fn.isra.29+0x17/0x80
[<ffffffff810f1a9b>] run_timer_softirq+0x18b/0x220
[<ffffffff810afbb9>] __do_softirq+0xf9/0x200
[<ffffffff810afde6>] irq_exit+0x76/0xa0
[<ffffffff8105dfc1>] smp_apic_timer_interrupt+0x41/0x50
[<ffffffff81b49067>] apic_timer_interrupt+0x67/0x70
<EOI>
[<ffffffff810dfd05>] ? down_read_trylock+0x15/0x20
[<ffffffff8106901b>] __do_page_fault+0xbb/0x4c0
[<ffffffff81b4447a>] ? __schedule+0x25a/0x850
[<ffffffff8106945c>] do_page_fault+0xc/0x10
[<ffffffff81b4977f>] page_fault+0x1f/0x30
---
drivers/edac/amd64_edac.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 17638d7..e8325e5 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -2174,14 +2174,20 @@ static void __log_bus_error(struct mem_ctl_info *mci, struct err_info *err,

static inline void decode_bus_error(int node_id, struct mce *m)
{
- struct mem_ctl_info *mci = mcis[node_id];
- struct amd64_pvt *pvt = mci->pvt_info;
+ struct mem_ctl_info *mci = edac_mc_find(node_id);
+ struct amd64_pvt *pvt;
u8 ecc_type = (m->status >> 45) & 0x3;
u8 xec = XEC(m->status, 0x1f);
u16 ec = EC(m->status);
u64 sys_addr;
struct err_info err;

+ /* Unable to decode where no memory controller info is allocated (>EDAC_MAX_MCS) */
+ if (!mci)
+ return;
+
+ pvt = mci->pvt_info;
+
/* Bail out early if this was an 'observed' error */
if (PP(ec) == NBSL_PP_OBS)
return;
--
1.9.1


2015-02-18 09:01:20

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v2] x86: Prevent oops with >16 memory controllers

On Tue, Feb 17, 2015 at 11:34:38AM +0800, Daniel J Blueman wrote:
> When ECC interrupts occur on memory controllers after EDAC_MAX_MCS (16), the
> kernel fatally dereferences unallocated structures [1]; this occurs on at
> least NumaConnect systems.
>
> Fix by checking if a memory controller info structure was found; candidate for
> stable.
>
> v1->2: Use edac_mc_find() as per Boris's suggestion
>
> Signed-off-by: Daniel J Blueman <[email protected]>

Applied and queued for stable, thanks.

I went and killed that mcis array too, ontop.

---
From: Borislav Petkov <[email protected]>
Subject: [PATCH] EDAC, amd64_edac: Get rid of per-node driver instances

... and do the proper thing using EDAC core facilities.

Cc: Daniel J Blueman <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
---
drivers/edac/amd64_edac.c | 33 +++++++++++++--------------------
1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 3d6a511a9025..92772fffc52f 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -20,8 +20,7 @@ static struct msr __percpu *msrs;
*/
static atomic_t drv_instances = ATOMIC_INIT(0);

-/* Per-node driver instances */
-static struct mem_ctl_info **mcis;
+/* Per-node stuff */
static struct ecc_settings **ecc_stngs;

/*
@@ -903,9 +902,17 @@ static int k8_early_channel_count(struct amd64_pvt *pvt)
/* On F10h and later ErrAddr is MC4_ADDR[47:1] */
static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
{
- u64 addr;
+ u16 mce_nid = amd_get_nb_id(m->extcpu);
+ struct mem_ctl_info *mci;
u8 start_bit = 1;
u8 end_bit = 47;
+ u64 addr;
+
+ mci = edac_mc_find(mce_nid);
+ if (!mci)
+ return 0;
+
+ pvt = mci->pvt_info;

if (pvt->fam == 0xf) {
start_bit = 3;
@@ -918,17 +925,13 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m)
* Erratum 637 workaround
*/
if (pvt->fam == 0x15) {
- struct amd64_pvt *pvt;
u64 cc6_base, tmp_addr;
u32 tmp;
- u16 mce_nid;
u8 intlv_en;

if ((addr & GENMASK_ULL(47, 24)) >> 24 != 0x00fdf7)
return addr;

- mce_nid = amd_get_nb_id(m->extcpu);
- pvt = mcis[mce_nid]->pvt_info;

amd64_read_pci_cfg(pvt->F1, DRAM_LOCAL_NODE_LIM, &tmp);
intlv_en = tmp >> 21 & 0x7;
@@ -1511,7 +1514,7 @@ static int f1x_lookup_addr_in_dct(u64 in_addr, u8 nid, u8 dct)
int cs_found = -EINVAL;
int csrow;

- mci = mcis[nid];
+ mci = edac_mc_find(nid);
if (!mci)
return cs_found;

@@ -2837,8 +2840,6 @@ static int init_one_instance(struct pci_dev *F2)

amd_register_ecc_decoder(decode_bus_error);

- mcis[nid] = mci;
-
atomic_inc(&drv_instances);

return 0;
@@ -2936,7 +2937,6 @@ static void remove_one_instance(struct pci_dev *pdev)

/* Free the EDAC CORE resources */
mci->pvt_info = NULL;
- mcis[nid] = NULL;

kfree(pvt);
edac_mc_free(mci);
@@ -2974,7 +2974,7 @@ static void setup_pci_device(void)
if (pci_ctl)
return;

- mci = mcis[0];
+ mci = edac_mc_find(0);
if (!mci)
return;

@@ -2998,9 +2998,8 @@ static int __init amd64_edac_init(void)
goto err_ret;

err = -ENOMEM;
- mcis = kzalloc(amd_nb_num() * sizeof(mcis[0]), GFP_KERNEL);
ecc_stngs = kzalloc(amd_nb_num() * sizeof(ecc_stngs[0]), GFP_KERNEL);
- if (!(mcis && ecc_stngs))
+ if (!ecc_stngs)
goto err_free;

msrs = msrs_alloc();
@@ -3031,9 +3030,6 @@ err_pci:
msrs = NULL;

err_free:
- kfree(mcis);
- mcis = NULL;
-
kfree(ecc_stngs);
ecc_stngs = NULL;

@@ -3051,9 +3047,6 @@ static void __exit amd64_edac_exit(void)
kfree(ecc_stngs);
ecc_stngs = NULL;

- kfree(mcis);
- mcis = NULL;
-
msrs_free(msrs);
msrs = NULL;
}
--
2.2.0.33.gc18b867

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--