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

login
register
mail settings
Submitter Rafał Miłecki
Date Nov. 27, 2018, 10:17 a.m.
Message ID <20181127101700.2964-1-zajec5@gmail.com>
Download mbox | patch
Permalink /patch/665799/
State New
Headers show

Comments

Rafał Miłecki - Nov. 27, 2018, 10:17 a.m.
From: Rafał Miłecki <rafal@milecki.pl>

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 <clemej@gmail.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201291
Fixes: 18807ddb7f88 ("x86/mce/AMD: Reset Threshold Limit after logging error")
Cc: stable@vger.kernel.org # v4.10+
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
Borislav Petkov - Nov. 27, 2018, 12:09 p.m.
On Tue, Nov 27, 2018 at 11:17:00AM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> 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.

Yeah, and this code is wrong as hell. :-\

We should be initializing the banks and *then* setup the thresholding
interrupt vector, when all setup has succeeded, not before.

Lemme think about it a bit.

Thx for the report.

Patch

diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index dd33c357548f..379f54afc266 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -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;