All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandre Chartre <alexandre.chartre@oracle.com>
To: linux-kernel@vger.kernel.org
Cc: Alexandre Chartre <alexandre.chartre@oracle.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	x86@kernel.org
Subject: [PATCH] x86/alternatives: check int3 breakpoint physical addresses
Date: Fri, 25 Jan 2019 15:57:29 +0100	[thread overview]
Message-ID: <1548428249-8258-1-git-send-email-alexandre.chartre@oracle.com> (raw)

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

             reply	other threads:[~2019-01-25 14:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-25 14:57 Alexandre Chartre [this message]
2019-02-10 21:23 ` [PATCH] x86/alternatives: check int3 breakpoint physical addresses Thomas Gleixner
2019-02-11  9:08   ` Alexandre Chartre
2019-02-11  9:15     ` Thomas Gleixner
2019-02-11  9:57       ` Alexandre Chartre
2019-02-21 10:14         ` Alexandre Chartre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1548428249-8258-1-git-send-email-alexandre.chartre@oracle.com \
    --to=alexandre.chartre@oracle.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.