From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 009E6C43142 for ; Thu, 21 Jun 2018 19:17:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AB23221A85 for ; Thu, 21 Jun 2018 19:17:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AB23221A85 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=alien8.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933227AbeFUTRR (ORCPT ); Thu, 21 Jun 2018 15:17:17 -0400 Received: from mail.skyhub.de ([5.9.137.197]:43576 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932902AbeFUTRP (ORCPT ); Thu, 21 Jun 2018 15:17:15 -0400 X-Virus-Scanned: Nedap ESD1 at mail.skyhub.de Received: from mail.skyhub.de ([127.0.0.1]) by localhost (blast.alien8.de [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id T0OwyuyZvPIO; Thu, 21 Jun 2018 21:17:14 +0200 (CEST) Received: from zn.tnic (p200300EC2BD18C00329C23FFFEA6A903.dip0.t-ipconnect.de [IPv6:2003:ec:2bd1:8c00:329c:23ff:fea6:a903]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.skyhub.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id 4C0701EC019F; Thu, 21 Jun 2018 21:17:14 +0200 (CEST) Date: Thu, 21 Jun 2018 21:17:11 +0200 From: Borislav Petkov To: "Luck, Tony" Cc: LKML Subject: Re: [PATCH] x86/mce: Do not overwrite MCi_STATUS in mce_no_way_out() Message-ID: <20180621191710.GI12742@zn.tnic> References: <20180621124020.6244-1-bp@alien8.de> <20180621181808.GA20097@agluck-desk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20180621181808.GA20097@agluck-desk> User-Agent: Mutt/1.9.5 (2018-04-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 21, 2018 at 11:18:09AM -0700, Luck, Tony wrote: > Counter proposal. We don't need the temp mci_status because we exit the > loop early. Nor the "ret" variable. > > > How does this look? Yap, better. I'll test it later or tomorrow: --- From: Borislav Petkov Date: Thu, 21 Jun 2018 14:18:47 +0200 Subject: [PATCH] x86/mce: Do not overwrite MCi_STATUS in mce_no_way_out() mce_no_way_out() does a quick check during #MC to see whether some of the MCEs logged would require the kernel to panic immediately. And it passes a struct mce where MCi_STATUS gets written. However, after having saved a valid status value, the next iteration of the loop which goes over the MCA banks on the CPU, overwrites the valid status value because we're using struct mce as storage instead of a temporary variable. Which leads to MCE records with an empty status value: mce: [Hardware Error]: CPU 0: Machine Check Exception: 6 Bank 0: 0000000000000000 mce: [Hardware Error]: RIP 10: {trigger_mce+0x7/0x10} In order to prevent the loss of the status register value, return immediately when severity is a panic one so that we can panic immediately with the first fatal MCE logged. This is also the intention of this function and not to noodle over the banks while a fatal MCE is already logged. Suggested-by: Tony Luck Signed-off-by: Borislav Petkov Cc: --- arch/x86/kernel/cpu/mcheck/mce.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c index 5c38d1f861f2..e75418096ec6 100644 --- a/arch/x86/kernel/cpu/mcheck/mce.c +++ b/arch/x86/kernel/cpu/mcheck/mce.c @@ -772,23 +772,24 @@ EXPORT_SYMBOL_GPL(machine_check_poll); static int mce_no_way_out(struct mce *m, char **msg, unsigned long *validp, struct pt_regs *regs) { - int i, ret = 0; char *tmp; + int i; for (i = 0; i < mca_cfg.banks; i++) { m->status = mce_rdmsrl(msr_ops.status(i)); - if (m->status & MCI_STATUS_VAL) { - __set_bit(i, validp); - if (quirk_no_way_out) - quirk_no_way_out(i, m, regs); - } + if (!(m->status & MCI_STATUS_VAL)) + continue; + + __set_bit(i, validp); + if (quirk_no_way_out) + quirk_no_way_out(i, m, regs); if (mce_severity(m, mca_cfg.tolerant, &tmp, true) >= MCE_PANIC_SEVERITY) { *msg = tmp; - ret = 1; + return 1; } } - return ret; + return 0; } /* -- 2.17.0.582.gccdcbd54c -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.