Skip to content

Commit 2777cae

Browse files
author
Ingo Molnar
committed
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 <[email protected]> Cc: Borislav Petkov <[email protected]> Cc: Brian Gerst <[email protected]> Cc: Dave Hansen <[email protected]> Cc: Denys Vlasenko <[email protected]> Cc: H. Peter Anvin <[email protected]> Cc: Juergen Gross <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Rik van Riel <[email protected]> Cc: Thomas Gleixner <[email protected]> Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Ingo Molnar <[email protected]>
1 parent e051964 commit 2777cae

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

arch/x86/kernel/paravirt.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,10 @@ unsigned paravirt_patch_insns(void *insnbuf, unsigned len,
163163
{
164164
unsigned insn_len = end - start;
165165

166-
if (insn_len > len || start == NULL)
167-
insn_len = len;
168-
else
169-
memcpy(insnbuf, start, insn_len);
166+
/* Alternative instruction is too large for the patch site and we cannot continue: */
167+
BUG_ON(insn_len > len || start == NULL);
168+
169+
memcpy(insnbuf, start, insn_len);
170170

171171
return insn_len;
172172
}

0 commit comments

Comments
 (0)