linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>,
	Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Anton Arapov <aarapov@redhat.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>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [GIT PULL] uprobes: fix the handling of relative jmp/call's
Date: Fri, 18 Apr 2014 10:35:10 +0200	[thread overview]
Message-ID: <20140418083510.GA19006@gmail.com> (raw)
In-Reply-To: <20140417200228.GA31097@redhat.com>


* Oleg Nesterov <oleg@redhat.com> wrote:

> Ingo, please pull from
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc uprobes/core
> 
> 
> To remind, we have a serious bug. Any probed jmp/call can kill the
> application, see the changelog in 11/15.
> 
> I tried to test this as much as possible, seems to work. I also wrote
> the test-case which explicitly checks every conditional jump with all
> possible bits combinations in eflags:
> 
> 	#include <asm/processor-flags.h>
> 	#include <stdio.h>
> 
> 	static int nojmp;
> 
> 	#define __MK_FUNC(opc, name)				\
> 		asm (						\
> 			".text\n"				\
> 			".globl " #name "; " #name ":\n"	\
> 			".byte 0x" #opc "\n"			\
> 			".byte 0x0a \n"	/* offs to 1f below */	\
> 			"movl $1, nojmp(%rip)\n"		\
> 			"1: ret\n"				\
> 		);						\
> 
> 	#define MK_FUNC(opc)	__MK_FUNC(opc, probe_func_ ## opc)
> 
> 	MK_FUNC(70) MK_FUNC(71) MK_FUNC(72) MK_FUNC(73)
> 	MK_FUNC(74) MK_FUNC(75) MK_FUNC(76) MK_FUNC(77)
> 	MK_FUNC(78) MK_FUNC(79) MK_FUNC(7a) MK_FUNC(7b)
> 	MK_FUNC(7c) MK_FUNC(7d) MK_FUNC(7e) MK_FUNC(7f)
> 
> 
> 	#define __CALL(flags, func)	\
> 		asm volatile ("pushf; push %0; popf; call " #func "; popf"	\
> 				: : "m" (flags) : "memory");
> 
> 	#define CALL(opc)		\
> 	({						\
> 		nojmp = 0;				\
> 		__CALL(flags, probe_func_ ## opc);	\
> 		!nojmp;					\
> 	})
> 
> 	long test_flags(unsigned long flags)
> 	{
> 		printf("%04lx %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d\n",
> 			flags,
> 			CALL(70), CALL(71), CALL(72), CALL(73),
> 			CALL(74), CALL(75), CALL(76), CALL(77),
> 			CALL(78), CALL(79), CALL(7a), CALL(7b),
> 			CALL(7c), CALL(7d), CALL(7e), CALL(7f));
> 
> 		return 0;
> 	}
> 
> 	#define XF(xf)		(X86_EFLAGS_ ## xf)
> 	#define XF_MASK		(XF(CF) | XF(OF) | XF(PF) | XF(SF) | XF(ZF))
> 
> 	int main(void)
> 	{
> 		unsigned int bits;
> 		unsigned long __flags, flags;
> 
> 		asm volatile("pushf; pop %0" : "=rm" (__flags) : : "memory");
> 
> 		for (bits = 0; bits < (1 << 5); bits++) {
> 			flags = __flags & ~XF_MASK;
> 
> 			#define	CPY_BIT(nr, xf) \
> 				if (bits & (1 << nr)) flags |= XF(xf)
> 
> 			CPY_BIT(0, CF);
> 			CPY_BIT(1, OF);
> 			CPY_BIT(2, PF);
> 			CPY_BIT(3, SF);
> 			CPY_BIT(4, ZF);
> 
> 			test_flags(flags);
> 		}
> 
> 		return 0;
> 	}
> 
> The output is the same with probe_func_70..probe_func_7f probed.
> 
> This series only fixes the problem. I'll send more changes to address
> some of TODO's mentioned in the changelogs later. In particular, we
> need to do something with "callw", see "Note: in 13/15.
> 
> Oleg Nesterov (15):
>       uprobes: Kill UPROBE_SKIP_SSTEP and can_skip_sstep()
>       uprobes/x86: Fold prepare_fixups() into arch_uprobe_analyze_insn()
>       uprobes/x86: Kill the "ia32_compat" check in handle_riprel_insn(), remove "mm" arg
>       uprobes/x86: Gather "riprel" functions together
>       uprobes/x86: move the UPROBE_FIX_{RIP,IP,CALL} code at the end of pre/post hooks
>       uprobes/x86: Introduce uprobe_xol_ops and arch_uprobe->ops
>       uprobes/x86: Conditionalize the usage of handle_riprel_insn()
>       uprobes/x86: Send SIGILL if arch_uprobe_post_xol() fails
>       uprobes/x86: Teach arch_uprobe_post_xol() to restart if possible
>       uprobes/x86: Introduce sizeof_long(), cleanup adjust_ret_addr() and arch_uretprobe_hijack_return_addr()
>       uprobes/x86: Emulate unconditional relative jmp's
>       uprobes/x86: Emulate nop's using ops->emulate()
>       uprobes/x86: Emulate relative call's
>       uprobes/x86: Emulate relative conditional "short" jmp's
>       uprobes/x86: Emulate relative conditional "near" jmp's
> 
>  arch/x86/include/asm/uprobes.h |   16 +-
>  arch/x86/kernel/uprobes.c      |  551 +++++++++++++++++++++++++---------------
>  kernel/events/uprobes.c        |   31 +--
>  3 files changed, 372 insertions(+), 226 deletions(-)

Pulled into tip:perf/core, thanks a lot Oleg!

	Ingo

  reply	other threads:[~2014-04-18  8:35 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-17 20:02 [GIT PULL] uprobes: fix the handling of relative jmp/call's Oleg Nesterov
2014-04-18  8:35 ` Ingo Molnar [this message]
2014-04-19 17:01 ` [PATCH 0/5] uprobes/x86: cleanup validate_insn_* paths, fix X86_X32 case Oleg Nesterov
2014-04-19 17:01   ` [PATCH 1/5] uprobes/x86: Add uprobe_init_insn(), kill validate_insn_{32,64}bits() Oleg Nesterov
2014-04-29 10:04     ` Srikar Dronamraju
2014-04-19 17:01   ` [PATCH 2/5] uprobes/x86: Add is_64bit_mm(), kill validate_insn_bits() Oleg Nesterov
2014-04-29 10:05     ` Srikar Dronamraju
2014-04-19 17:01   ` [PATCH 3/5] uprobes/x86: Shift "insn_complete" from branch_setup_xol_ops() to uprobe_init_insn() Oleg Nesterov
2014-04-29 10:05     ` Srikar Dronamraju
2014-04-19 17:01   ` [PATCH 4/5] uprobes/x86: Make good_insns_* depend on CONFIG_X86_* Oleg Nesterov
2014-04-29 10:06     ` Srikar Dronamraju
2014-04-19 17:02   ` [PATCH 5/5] uprobes/x86: Fix is_64bit_mm() with CONFIG_X86_X32 Oleg Nesterov
2014-04-29 10:06     ` Srikar Dronamraju
2014-04-24 21:36   ` [PATCH 0/5] uprobes/x86: cleanup validate_insn_* paths, fix X86_X32 case Jim Keniston
2014-04-22 14:47 ` [PATCH 0/5] uprobes/x86: completely untangle branch_xol_ops and default_xol_ops Oleg Nesterov
2014-04-22 14:47   ` [PATCH 1/5] uprobes/x86: Don't change the task's state if ->pre_xol() fails Oleg Nesterov
2014-04-22 14:47   ` [PATCH 2/5] uprobes/x86: Introduce uprobe_xol_ops->abort() and default_abort_op() Oleg Nesterov
2014-04-22 14:47   ` [PATCH 3/5] uprobes/x86: Don't use arch_uprobe_abort_xol() in arch_uprobe_post_xol() Oleg Nesterov
2014-04-22 14:47   ` [PATCH 4/5] uprobes/x86: Move UPROBE_FIX_SETF logic from arch_uprobe_post_xol() to default_post_xol_op() Oleg Nesterov
2014-04-22 14:47   ` [PATCH 5/5] uprobes/x86: Move default_xol_ops's data into arch_uprobe->def Oleg Nesterov
2014-04-24 23:30     ` Jim Keniston
2014-04-25 19:53       ` Oleg Nesterov
2014-04-25 17:47 ` [PATCH 0/4] uprobes/x86: UPROBE_FIX_IP/UPROBE_FIX_CALL cleanups Oleg Nesterov
2014-04-25 17:47   ` [PATCH 1/4] uprobes/x86: Cleanup the usage of arch_uprobe->def.fixups, make it u8 Oleg Nesterov
2014-04-25 17:47   ` [PATCH 2/4] uprobes/x86: Introduce push_ret_address() Oleg Nesterov
2014-04-25 17:47   ` [PATCH 3/4] uprobes/x86: Kill adjust_ret_addr(), simplify UPROBE_FIX_CALL logic Oleg Nesterov
2014-04-25 17:47   ` [PATCH 4/4] uprobes/x86: Cleanup the usage of UPROBE_FIX_IP/UPROBE_FIX_CALL Oleg Nesterov
2014-04-27 13:51   ` [PATCH 0/4] uprobes/x86: UPROBE_FIX_IP/UPROBE_FIX_CALL cleanups Oleg Nesterov
2014-04-27 16:52 ` [PATCH 0/3] uprobes/x86: cleanup "riprel" functions Oleg Nesterov
2014-04-27 16:52   ` [PATCH 1/3] uprobes/x86: Rename *riprel* helpers to make the naming consistent Oleg Nesterov
2014-04-28  6:34     ` Srikar Dronamraju
2014-05-01  0:07       ` Jim Keniston
2014-04-27 16:52   ` [PATCH 2/3] uprobes/x86: Kill the "autask" arg of riprel_pre_xol() Oleg Nesterov
2014-04-28  6:35     ` Srikar Dronamraju
2014-05-01  0:07       ` Jim Keniston
2014-04-27 16:52   ` [PATCH 3/3] uprobes/x86: Simplify riprel_{pre,post}_xol() and make them similar Oleg Nesterov
2014-04-28  6:36     ` Srikar Dronamraju
2014-05-01  0:08       ` Jim Keniston

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=20140418083510.GA19006@gmail.com \
    --to=mingo@kernel.org \
    --cc=aarapov@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=oleg@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).