Skip to contentSkip to conversation

x86/mce/AMD: Make sure banks were initialized before accessing them

PATCH Rafał Miłecki 2 participants 2 messages Nov 27, 2018 – Nov 28, 2018

Subsystems (2)
x86 EDAC-core
lore
Share & tools
lore

Rafał Miłecki

author
#0 lore raw

From: Rafał Miłecki <[email protected]>

It may happen amd_threshold_interrupt() fires before banks get setup as part of the threshold_init_device() call. Such a case requires a proper check to avoid NULL pointer dereference.

This problem has been noticed on HP EliteBook 745 G5 (Ryzen 2500U) with BIOS 1.03.01. It doesn't boot unless mce is disabled (e.g. with "mce=off").

This fixes a regression introduced in kernel 4.10. It was the first kernel referencing "threshold_banks" variable in the amd_threshold_interrupt().

Reported-by John Clemens <[email protected]>

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201291

Fixes 18807ddb7f88 ("x86/mce/AMD: Reset Threshold Limit after logging error")
Cc [email protected] # v4.10+
Signed-off-by Rafał Miłecki <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/mcheck/mce_amd.c +8 -1
index dd33c357548f..379f54afc266 100644
@@ -935,12 +935,19 @@ static void amd_threshold_interrupt(void)
 {
 	struct threshold_block *first_block = NULL, *block = NULL, *tmp = NULL;
 	unsigned int bank, cpu = smp_processor_id();
+	struct threshold_bank **banks;
+
+	banks = per_cpu(threshold_banks, cpu);
+	if (!banks) {
+		pr_warn("Threshold interrupt fired before MCE setup\n");
+		return;
+	}
 
 	for (bank = 0; bank < mca_cfg.banks; ++bank) {
 		if (!(per_cpu(bank_map, cpu) & (1 << bank)))
 			continue;
 
-		first_block = per_cpu(threshold_banks, cpu)[bank]->blocks;
+		first_block = banks[bank]->blocks;
 		if (!first_block)
 			continue;
merge bot
#1 lore raw

[tip:x86/urgent] x86/MCE/AMD: Fix the thresholding machinery initialization order

x86/MCE/AMD: Fix the thresholding machinery initialization order
Commit-ID:  60c8144afc287ef09ce8c1230c6aa972659ba1bb
Gitweb:     https://git.kernel.org/tip/60c8144afc287ef09ce8c1230c6aa972659ba1bb
Author:     Borislav Petkov <[email protected]>
AuthorDate: Tue, 27 Nov 2018 14:41:37 +0100
Committer:  Borislav Petkov <[email protected]>
CommitDate: Wed, 28 Nov 2018 10:10:36 +0100

x86/MCE/AMD: Fix the thresholding machinery initialization order

Currently, the code sets up the thresholding interrupt vector and only then goes about initializing the thresholding banks. Which is wrong, because an early thresholding interrupt would cause a NULL pointer dereference when accessing those banks and prevent the machine from booting.

Therefore, set the thresholding interrupt vector only *after* having initialized the banks successfully.

Fixes 18807ddb7f88 ("x86/mce/AMD: Reset Threshold Limit after logging error")
Reported-by Rafał Miłecki <[email protected]>
Reported-by John Clemens <[email protected]>
Signed-off-by Borislav Petkov <[email protected]>
Tested-by Rafał Miłecki <[email protected]>
Tested-by John Clemens <[email protected]>
Cc Aravind Gopalakrishnan <[email protected]>
Cc [email protected]
Cc [email protected]
Cc Tony Luck <[email protected]>
Cc [email protected]
Cc Yazen Ghannam <[email protected]>
Link https://lkml.kernel.org/r/[email protected] open here
Link https://bugzilla.kernel.org/show_bug.cgi?id=201291
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)
arch/x86/kernel/cpu/mcheck/mce_amd.c +6 -13
index dd33c357548f..e12454e21b8a 100644
@@ -56,7 +56,7 @@
 /* Threshold LVT offset is at MSR0xC0000410[15:12] */
 #define SMCA_THR_LVT_OFF	0xF000
 
-static bool thresholding_en;
+static bool thresholding_irq_en;
 
 static const char * const th_names[] = {
 	"load_store",

@@ -534,9 +534,8 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
 
 set_offset:
 	offset = setup_APIC_mce_threshold(offset, new);
-
-	if ((offset == new) && (mce_threshold_vector != amd_threshold_interrupt))
-		mce_threshold_vector = amd_threshold_interrupt;
+	if (offset == new)
+		thresholding_irq_en = true;
 
 done:
 	mce_threshold_block_init(&b, offset);

@@ -1357,9 +1356,6 @@ int mce_threshold_remove_device(unsigned int cpu)
 {
 	unsigned int bank;
 
-	if (!thresholding_en)
-		return 0;
-
 	for (bank = 0; bank < mca_cfg.banks; ++bank) {
 		if (!(per_cpu(bank_map, cpu) & (1 << bank)))
 			continue;

@@ -1377,9 +1373,6 @@ int mce_threshold_create_device(unsigned int cpu)
 	struct threshold_bank **bp;
 	int err = 0;
 
-	if (!thresholding_en)
-		return 0;
-
 	bp = per_cpu(threshold_banks, cpu);
 	if (bp)
 		return 0;

@@ -1408,9 +1401,6 @@ static __init int threshold_init_device(void)
 {
 	unsigned lcpu = 0;
 
-	if (mce_threshold_vector == amd_threshold_interrupt)
-		thresholding_en = true;
-
 	/* to hit CPUs online before the notifier is up */
 	for_each_online_cpu(lcpu) {
 		int err = mce_threshold_create_device(lcpu);

@@ -1419,6 +1409,9 @@ static __init int threshold_init_device(void)
 			return err;
 	}
 
+	if (thresholding_irq_en)
+		mce_threshold_vector = amd_threshold_interrupt;
+
 	return 0;
 }
 /*