All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Ingo Molnar <mingo@elte.hu>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	David Long <dave.long@linaro.org>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	"Frank Ch. Eigler" <fche@redhat.com>,
	Jim Keniston <jkenisto@us.ibm.com>,
	Jonathan Lebon <jlebon@redhat.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 9/7] uprobes/x86: Teach arch_uprobe_post_xol() to restart if possible
Date: Thu, 3 Apr 2014 22:01:55 +0200	[thread overview]
Message-ID: <20140403200155.GE12639@redhat.com> (raw)
In-Reply-To: <20140403200015.GC12639@redhat.com>

SIGILL after the failed arch_uprobe_post_xol() should only be used as
a last resort, we should try to restart the probed insn if possible.

Currently only adjust_ret_addr() can fail, and this can only happen if
another thread unmapped our stack after we executed "call" out-of-line.
Most probably the application if buggy, but even in this case it can
have a handler for SIGSEGV/etc. And in theory it can be even correct
and do something non-trivial with its memory.

Of course we can't restart unconditionally, so arch_uprobe_post_xol()
does this only if ->post_xol() returns -ERESTART even if currently this
is the only possible error.

Note: this is not "perfect" because handler_chain() we actually do not
want the extra handler_chain() after restart, but I think this is the
best solution we can realistically do without too much uglifications.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 arch/x86/kernel/uprobes.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 3bdfb6e..78c4bb3 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -443,16 +443,17 @@ static int default_post_xol_op(struct arch_uprobe *auprobe, struct pt_regs *regs
 {
 	struct uprobe_task *utask = current->utask;
 	long correction = (long)(utask->vaddr - utask->xol_vaddr);
-	int ret = 0;
 
 	handle_riprel_post_xol(auprobe, regs, &correction);
 	if (auprobe->fixups & UPROBE_FIX_IP)
 		regs->ip += correction;
 
-	if (auprobe->fixups & UPROBE_FIX_CALL)
-		ret = adjust_ret_addr(regs->sp, correction);
+	if (auprobe->fixups & UPROBE_FIX_CALL) {
+		if (adjust_ret_addr(regs->sp, correction))
+			return -ERESTART;
+	}
 
-	return ret;
+	return 0;
 }
 
 static struct uprobe_xol_ops default_xol_ops = {
@@ -599,6 +600,12 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
 		int err = auprobe->ops->post_xol(auprobe, regs);
 		if (err) {
 			arch_uprobe_abort_xol(auprobe, regs);
+			/*
+			 * Restart the probed insn. ->post_xol() must ensure
+			 * this is really possible if it returns -ERESTART.
+			 */
+			if (err == -ERESTART)
+				return 0;
 			return err;
 		}
 	}
-- 
1.5.5.1



      parent reply	other threads:[~2014-04-03 20:02 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-31 19:43 [PATCH 0/7] uprobes/x86: introduce uprobe_xol_ops and arch_uprobe->ops Oleg Nesterov
2014-03-31 19:43 ` [PATCH 1/7] uprobes: Kill UPROBE_SKIP_SSTEP and can_skip_sstep() Oleg Nesterov
2014-04-01  1:41   ` Masami Hiramatsu
2014-04-01 15:39   ` David Long
2014-04-03 16:32   ` Srikar Dronamraju
2014-03-31 19:43 ` [PATCH 2/7] uprobes/x86: Fold prepare_fixups() into arch_uprobe_analyze_insn() Oleg Nesterov
2014-04-01  2:00   ` Masami Hiramatsu
2014-04-03 16:33   ` Srikar Dronamraju
2014-03-31 19:44 ` [PATCH 3/7] uprobes/x86: Conditionalize the usage of handle_riprel_insn() Oleg Nesterov
2014-04-01  3:17   ` Masami Hiramatsu
2014-04-01 14:33     ` Oleg Nesterov
2014-04-01 16:39       ` Oleg Nesterov
2014-04-02 17:57         ` Jim Keniston
2014-04-02 19:14           ` Oleg Nesterov
2014-03-31 19:44 ` [PATCH 4/7] uprobes/x86: Kill the "ia32_compat" check in handle_riprel_insn(), remove "mm" arg Oleg Nesterov
2014-04-01  3:10   ` Masami Hiramatsu
2014-04-03 16:33   ` Srikar Dronamraju
2014-03-31 19:44 ` [PATCH 5/7] uprobes/x86: Gather "riprel" functions together Oleg Nesterov
2014-04-01  3:21   ` Masami Hiramatsu
2014-04-02 19:53   ` Jim Keniston
2014-04-03 19:50     ` Oleg Nesterov
2014-03-31 19:44 ` [PATCH 6/7] uprobes/x86: move the UPROBE_FIX_{RIP,IP,CALL} code at the end of pre/post hooks Oleg Nesterov
2014-04-01  3:24   ` Masami Hiramatsu
2014-04-03 16:34   ` Srikar Dronamraju
2014-03-31 19:44 ` [PATCH 7/7] uprobes/x86: Introduce uprobe_xol_ops and arch_uprobe->ops Oleg Nesterov
2014-04-01  6:52   ` Masami Hiramatsu
2014-04-01 14:44     ` Oleg Nesterov
2014-04-01 15:01   ` Oleg Nesterov
2014-04-02 19:46   ` Jim Keniston
2014-04-03 19:49     ` Oleg Nesterov
2014-04-02 19:58 ` [PATCH 0/7] uprobes/x86: introduce " Jim Keniston
2014-04-03 20:00 ` [PATCH 8-9/7] " Oleg Nesterov
2014-04-03 20:00   ` [PATCH 8/7] uprobes/x86: Send SIGILL if arch_uprobe_post_xol() fails Oleg Nesterov
2014-04-03 20:01   ` Oleg Nesterov [this message]

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=20140403200155.GE12639@redhat.com \
    --to=oleg@redhat.com \
    --cc=ananth@in.ibm.com \
    --cc=dave.long@linaro.org \
    --cc=dvlasenk@redhat.com \
    --cc=fche@redhat.com \
    --cc=jkenisto@us.ibm.com \
    --cc=jlebon@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@elte.hu \
    --cc=srikar@linux.vnet.ibm.com \
    /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.