From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S940163AbdEXMvk (ORCPT ); Wed, 24 May 2017 08:51:40 -0400 Received: from mail-wr0-f180.google.com ([209.85.128.180]:33864 "EHLO mail-wr0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S940116AbdEXMvX (ORCPT ); Wed, 24 May 2017 08:51:23 -0400 From: Mateusz Jurczyk To: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Andy Lutomirski , Borislav Petkov Cc: x86@kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] x86: Prevent uninitialized stack byte read in apply_alternatives() Date: Wed, 24 May 2017 14:51:20 +0200 Message-Id: <20170524125120.23246-1-mjurczyk@google.com> X-Mailer: git-send-email 2.13.0.219.gdb65acc882-goog Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org In the current form of the code, if a->replacementlen is 0, the reference to *insnbuf for comparison touches potentially garbage memory. While it doesn't affect the execution flow due to the subsequent a->replacementlen comparison, it is (rightly) detected as use of uninitialized memory by a runtime instrumentation currently under my development, and could be detected as such by other tools in the future, too (e.g. KMSAN). Fix the "false-positive" by reordering the conditions to first check the replacement instruction length before referencing specific opcode bytes. Signed-off-by: Mateusz Jurczyk --- arch/x86/kernel/alternative.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index c5b8f760473c..d03ba6bc97d8 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -410,7 +410,7 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, insnbuf_sz = a->replacementlen; /* 0xe8 is a relative jump; fix the offset. */ - if (*insnbuf == 0xe8 && a->replacementlen == 5) { + if (a->replacementlen == 5 && *insnbuf == 0xe8) { *(s32 *)(insnbuf + 1) += replacement - instr; DPRINTK("Fix CALL offset: 0x%x, CALL 0x%lx", *(s32 *)(insnbuf + 1), -- 2.13.0.219.gdb65acc882-goog