mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Masami Hiramatsu <mhiramat@kernel.org>,
	Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
	"David S . Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org
Subject: [PATCH -tip v2 1/2] kprobes/x86: Don't forget to set memory back to RO on failure
Date: Sat, 12 Aug 2017 09:44:50 +0900	[thread overview]
Message-ID: <150249868002.18166.14033573481681561556.stgit@devbox> (raw)
In-Reply-To: <150249861299.18166.13400123503538960282.stgit@devbox>

Do not forget to set kprobes insn buffer memory back
to RO on failure path. Without this fix, if there is
an unexpected error on copying instructions, kprobes
insn buffer kept RW, which can allow unexpected modifying
instruction buffer.

Fixes: d0381c81c2f7 ("kprobes/x86: Set kprobes pages read-only")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
  Changes in v2:
   - Use a helper variable instead of using p->ainsn.insn directly.
---
 arch/x86/kernel/kprobes/core.c |   15 +++++++++------
 arch/x86/kernel/kprobes/opt.c  |    1 +
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index f0153714ddac..5d8194b9a068 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -428,15 +428,18 @@ void free_insn_page(void *page)
 
 static int arch_copy_kprobe(struct kprobe *p)
 {
+	kprobe_opcode_t *buf = p->ainsn.insn;
 	struct insn insn;
 	int len;
 
-	set_memory_rw((unsigned long)p->ainsn.insn & PAGE_MASK, 1);
+	set_memory_rw((unsigned long)buf & PAGE_MASK, 1);
 
 	/* Copy an instruction with recovering if other optprobe modifies it.*/
-	len = __copy_instruction(p->ainsn.insn, p->addr, &insn);
-	if (!len)
+	len = __copy_instruction(buf, p->addr, &insn);
+	if (!len) {
+		set_memory_ro((unsigned long)buf & PAGE_MASK, 1);
 		return -EINVAL;
+	}
 
 	/*
 	 * __copy_instruction can modify the displacement of the instruction,
@@ -444,13 +447,13 @@ static int arch_copy_kprobe(struct kprobe *p)
 	 */
 	prepare_boost(p, &insn);
 
-	set_memory_ro((unsigned long)p->ainsn.insn & PAGE_MASK, 1);
+	set_memory_ro((unsigned long)buf & PAGE_MASK, 1);
 
 	/* Check whether the instruction modifies Interrupt Flag or not */
-	p->ainsn.if_modifier = is_IF_modifier(p->ainsn.insn);
+	p->ainsn.if_modifier = is_IF_modifier(buf);
 
 	/* Also, displacement change doesn't affect the first byte */
-	p->opcode = p->ainsn.insn[0];
+	p->opcode = buf[0];
 
 	return 0;
 }
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 4f98aad38237..86b9a883b712 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -371,6 +371,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op,
 	ret = copy_optimized_instructions(buf + TMPL_END_IDX, op->kp.addr);
 	if (ret < 0) {
 		__arch_remove_optimized_kprobe(op, 0);
+		set_memory_ro((unsigned long)buf & PAGE_MASK, 1);
 		return ret;
 	}
 	op->optinsn.size = ret;

  reply	other threads:[~2017-08-12  0:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-12  0:43 [PATCH -tip v2 0/2] kprobes/x86: RO text code bugfix and cleanup Masami Hiramatsu
2017-08-12  0:44 ` Masami Hiramatsu [this message]
2017-08-12  0:45 ` [PATCH -tip v2 2/2] kprobes/x86: Remove addressof operators Masami Hiramatsu
2017-08-17  9:55 ` [PATCH -tip v2 0/2] kprobes/x86: RO text code bugfix and cleanup Ingo Molnar
2017-08-17 15:07   ` Masami Hiramatsu

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=150249868002.18166.14033573481681561556.stgit@devbox \
    --to=mhiramat@kernel.org \
    --cc=ananth@in.ibm.com \
    --cc=anil.s.keshavamurthy@intel.com \
    --cc=davem@davemloft.net \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome