All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Pratyush Anand <panand@redhat.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v3 08/14] uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp()
Date: Tue, 21 Jul 2015 15:40:21 +0200	[thread overview]
Message-ID: <20150721134021.GA4773@redhat.com> (raw)
In-Reply-To: <20150721133945.GA4718@redhat.com>

Test-case:

	#include <stdio.h>
	#include <setjmp.h>

	jmp_buf jmp;

	void func_2(void)
	{
		longjmp(jmp, 1);
	}

	void func_1(void)
	{
		if (setjmp(jmp))
			return;
		func_2();
		printf("ERR!! I am running on the caller's stack\n");
	}

	int main(void)
	{
		func_1();
		return 0;
	}

fails if you probe func_1() and func_2() because handle_trampoline()
assumes that the probed function should must return and hit the bp
installed be prepare_uretprobe(). But in this case func_2() does not
return, so when func_1() returns the kernel uses the no longer valid
return_instance of func_2().

Change handle_trampoline() to unwind ->return_instances until we know
that the next chain is alive or NULL, this ensures that the current
chain is the last we need to report and free.

Alternatively, every return_instance could use unique trampoline_vaddr,
in this case we could use it as a key. And this could solve the problem
with sigaltstack() automatically.

But this approach needs more changes, and it puts the "hard" limit on
MAX_URETPROBE_DEPTH. Plus it can not solve another problem partially
fixed by the next patch.

Note: this change has no effect on !x86, the arch-agnostic version of
arch_uretprobe_is_alive() just returns "true".

TODO: as documented by the previous change, arch_uretprobe_is_alive()
can be fooled by sigaltstack/etc.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Anton Arapov <arapov@gmail.com>
Tested-by: Pratyush Anand <panand@redhat.com>
---
 kernel/events/uprobes.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index c5f316e..93d939c 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1774,6 +1774,7 @@ static void handle_trampoline(struct pt_regs *regs)
 {
 	struct uprobe_task *utask;
 	struct return_instance *ri, *next;
+	bool valid;
 
 	utask = current->utask;
 	if (!utask)
@@ -1783,18 +1784,24 @@ static void handle_trampoline(struct pt_regs *regs)
 	if (!ri)
 		goto sigill;
 
-	next = find_next_ret_chain(ri);
-	/*
-	 * TODO: we should throw out return_instance's invalidated by
-	 * longjmp(), currently we assume that the probed function always
-	 * returns.
-	 */
-	instruction_pointer_set(regs, ri->orig_ret_vaddr);
 	do {
-		handle_uretprobe_chain(ri, regs);
-		ri = free_ret_instance(ri);
-		utask->depth--;
-	} while (ri != next);
+		/*
+		 * We should throw out the frames invalidated by longjmp().
+		 * If this chain is valid, then the next one should be alive
+		 * or NULL; the latter case means that nobody but ri->func
+		 * could hit this trampoline on return. TODO: sigaltstack().
+		 */
+		next = find_next_ret_chain(ri);
+		valid = !next || arch_uretprobe_is_alive(next, regs);
+
+		instruction_pointer_set(regs, ri->orig_ret_vaddr);
+		do {
+			if (valid)
+				handle_uretprobe_chain(ri, regs);
+			ri = free_ret_instance(ri);
+			utask->depth--;
+		} while (ri != next);
+	} while (!valid);
 
 	utask->return_instances = ri;
 	return;
-- 
1.5.5.1


  parent reply	other threads:[~2015-07-21 13:42 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-21 13:39 [PATCH v3 00/14] uprobes: longjmp / special-mapping fixes Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 01/14] uprobes: Introduce get_uprobe() Oleg Nesterov
2015-07-31 13:57   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 02/14] uprobes: Introduce free_ret_instance() Oleg Nesterov
2015-07-31 13:58   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 03/14] uprobes: Send SIGILL if handle_trampoline() fails Oleg Nesterov
2015-07-31 13:58   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 04/14] uprobes: Change prepare_uretprobe() to use uprobe_warn() Oleg Nesterov
2015-07-31 13:58   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 05/14] uprobes: Change handle_trampoline() to find the next chain beforehand Oleg Nesterov
2015-07-31 13:59   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 06/14] uprobes: Export struct return_instance, introduce arch_uretprobe_is_alive() Oleg Nesterov
2015-07-31 13:59   ` [tip:perf/core] uprobes: Export 'struct return_instance', " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 07/14] uprobes/x86: Reimplement arch_uretprobe_is_alive() Oleg Nesterov
2015-07-31 13:59   ` [tip:perf/core] uprobes/x86: Reimplement arch_uretprobe_is_alive( ) tip-bot for Oleg Nesterov
2015-07-21 13:40 ` Oleg Nesterov [this message]
2015-07-31 14:00   ` [tip:perf/core] uprobes: Change handle_trampoline() to flush the frames invalidated by longjmp() tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 09/14] uprobes: Change prepare_uretprobe() to (try to) flush the dead frames Oleg Nesterov
2015-07-31 14:00   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 10/14] uprobes: Add the "enum rp_check ctx" arg to arch_uretprobe_is_alive() Oleg Nesterov
2015-07-31 14:00   ` [tip:perf/core] " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 11/14] uprobes/x86: Make arch_uretprobe_is_alive(RP_CHECK_CALL) more clever Oleg Nesterov
2015-07-31 14:01   ` [tip:perf/core] uprobes/x86: Make arch_uretprobe_is_alive( RP_CHECK_CALL) " tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 12/14] uprobes: fix the usage of install_special_mapping() Oleg Nesterov
2015-07-31 14:01   ` [tip:perf/core] uprobes: Fix the usage of install_special_mapping () tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 13/14] uprobes: use vm_special_mapping to name the xol vma Oleg Nesterov
2015-07-31 14:01   ` [tip:perf/core] uprobes: Use vm_special_mapping to name the XOL vma tip-bot for Oleg Nesterov
2015-07-21 13:40 ` [PATCH v3 14/14] uprobes: fix the waitqueue_active() check in xol_free_insn_slot() Oleg Nesterov
2015-07-31 14:02   ` [tip:perf/core] uprobes: Fix " tip-bot for Oleg Nesterov

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=20150721134021.GA4773@redhat.com \
    --to=oleg@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@kernel.org \
    --cc=panand@redhat.com \
    --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.