[PATCH] x86/alternatives: check int3 breakpoint physical addresses

* [PATCH] x86/alternatives: check int3 breakpoint physical addresses
@ 2019-01-25 14:57 Alexandre Chartre
  2019-02-10 21:23 ` Thomas Gleixner
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Chartre @ 2019-01-25 14:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Alexandre Chartre, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Josh Poimboeuf, x86

Modifying multi-byte instructions is achieved by temporarily replacing
the first instruction to patch with an int3 instruction. Then, if an
int3 interrupt is raised, the int3 handler will check if the interrupt
was caused by this temporary patch by comparing the address that raises
the interrupt (the interrupt address) and the address that was patched
(the patched address).

The int3 handler compares virtual addresses of the interrupt and
patched addresses. However, this doesn't work if the code to modify
is mapped to multiple virtual addresses because, in that case, the
patched virtual address can be different from the interrupt virtual
address.

A simple solution is then to also compare physical addresses when
virtual addresses do not match. Note that we still compare virtual
addresses because that's a faster comparison than having to fully
resolve and compare physical addresses.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: x86@kernel.org
---

This is a potential issue and I don't know if it can be triggered with the
current kernel: is there any code mapped to multiple virtual addresses
which can be updated with text_poke_bp()? However as the patch is small and
simple, it might be worth applying it anyway to prevent any such issue.

Note that this issue has been observed and reproduced with a custom kernel
with some code mapped to different virtual addresses and using jump labels
As jump labels use text_poke_bp(), crashes were sometimes observed when
updating jump labels.

 arch/x86/kernel/alternative.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index ebeac48..e563573 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -757,7 +757,18 @@ int poke_int3_handler(struct pt_regs *regs)
 	if (likely(!bp_patching_in_progress))
 		return 0;
 
-	if (user_mode(regs) || regs->ip != (unsigned long)bp_int3_addr)
+	if (user_mode(regs))
+		return 0;
+
+	/*
+	 * If virtual addresses are different, check if physical addresses
+	 * are the same in case we have the same code mapped to different
+	 * virtual addresses. Note that we could just compare physical
+	 * addresses, however we first compare virtual addresses because
+	 * this is much faster and very likely to succeed.
+	 */
+	if (regs->ip != (unsigned long)bp_int3_addr &&
+	    slow_virt_to_phys((void *)regs->ip) != slow_virt_to_phys(bp_int3_addr))
 		return 0;
 
 	/* set up the specified breakpoint handler */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread