linux-kernel - [tip:x86/paravirt] x86/paravirt: Detect over-sized patching bugs in paravirt_patch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 May 2019 00:58:44 -0700
From:   tip-bot for Ingo Molnar <tipbot@...or.com>
To:     linux-tip-commits@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, dave.hansen@...ux.intel.com,
        a.p.zijlstra@...llo.nl, mingo@...nel.org, riel@...riel.com,
        hpa@...or.com, brgerst@...il.com, torvalds@...ux-foundation.org,
        jgross@...e.com, luto@...nel.org, tglx@...utronix.de, bp@...en8.de,
        dvlasenk@...hat.com, peterz@...radead.org
Subject: [tip:x86/paravirt] x86/paravirt: Detect over-sized patching bugs in
 paravirt_patch_insns()

Commit-ID:  2777cae2b19d4a08ad233b3504c19c6f7a6a2ef3
Gitweb:     https://git.kernel.org/tip/2777cae2b19d4a08ad233b3504c19c6f7a6a2ef3
Author:     Ingo Molnar <mingo@...nel.org>
AuthorDate: Thu, 25 Apr 2019 11:17:17 +0200
Committer:  Ingo Molnar <mingo@...nel.org>
CommitDate: Thu, 25 Apr 2019 12:00:31 +0200

x86/paravirt: Detect over-sized patching bugs in paravirt_patch_insns()

So paravirt_patch_insns() contains this gem of logic:

unsigned paravirt_patch_insns(void *insnbuf, unsigned len,
                              const char *start, const char *end)
{
        unsigned insn_len = end - start;

        if (insn_len > len || start == NULL)
                insn_len = len;
        else
                memcpy(insnbuf, start, insn_len);

        return insn_len;
}

Note how 'len' (size of the original instruction) is checked against the new
instruction, and silently discarded with no warning printed whatsoever.

This crashes the kernel in funny ways if the patching template is buggy,
and usually in much later places.

Instead do a direct BUG_ON(), there's no way to continue successfully at that point.

I've tested this patch, with the vanilla kernel check never triggers, and
if I intentionally increase the size of one of the patch templates to a
too high value the assert triggers:

[    0.164385] kernel BUG at arch/x86/kernel/paravirt.c:167!

Without this patch a broken kernel randomly crashes in later places,
after the silent patching failure.

Cc: Andy Lutomirski <luto@...nel.org>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Brian Gerst <brgerst@...il.com>
Cc: Dave Hansen <dave.hansen@...ux.intel.com>
Cc: Denys Vlasenko <dvlasenk@...hat.com>
Cc: H. Peter Anvin <hpa@...or.com>
Cc: Juergen Gross <jgross@...e.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Rik van Riel <riel@...riel.com>
Cc: Thomas Gleixner <tglx@...utronix.de>
Link: http://lkml.kernel.org/r/20190425091717.GA72229@gmail.com
Signed-off-by: Ingo Molnar <mingo@...nel.org>
---
 arch/x86/kernel/paravirt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index c0e0101133f3..7f9121f2fdac 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -163,10 +163,10 @@ unsigned paravirt_patch_insns(void *insnbuf, unsigned len,
 {
 	unsigned insn_len = end - start;
 
-	if (insn_len > len || start == NULL)
-		insn_len = len;
-	else
-		memcpy(insnbuf, start, insn_len);
+	/* Alternative instruction is too large for the patch site and we cannot continue: */
+	BUG_ON(insn_len > len || start == NULL);
+
+	memcpy(insnbuf, start, insn_len);
 
 	return insn_len;
 }