From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Frederic Weisbecker <fweisbec@gmail.com>,
Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@elte.hu>,
yrl.pp-manager.tt@hitachi.com
Subject: Re: [BUG] kprobes crashing because of preempt count
Date: Fri, 01 Jul 2011 10:12:03 +0900 [thread overview]
Message-ID: <4E0D1EE3.6080607@hitachi.com> (raw)
In-Reply-To: <1309440213.26417.76.camel@gandalf.stny.rr.com>
(2011/06/30 22:23), Steven Rostedt wrote:
> Hi Masami,
>
> While testing some changes in -rt against kprobes, I hit a crash that
> exists in mainline. If we stick a probe in a location that reads
> preempt_count, we corrupt the kernel itself.
>
> Reason is that the kprobe single step handler disables preemption, sets
> up the single step, returns to the code to execute that single step,
> takes the trap, enables preemption, and continues.
>
> The issue, is because we disabled preemption in the trap, returned, then
> enabled it again in another trap, we just changed what the code sees
> that does that single step.
>
> If we add a kprobe on a inc_preempt_count() call:
>
> [ preempt_count = 0 ]
>
> ld preempt_count, %eax <<--- trap
>
> <trap>
> preempt_disable();
> [ preempt_count = 1]
> setup_singlestep();
> <trap return>
>
> [ preempt_count = 1 ]
>
> ld preempt_count, %eax
>
> [ %eax = 1 ]
>
> <trap>
> post_kprobe_handler()
> preempt_enable_no_resched();
> [ preempt_count = 0 ]
> <trap return>
>
> [ %eax = 1 ]
>
> add %eax,1
>
> [ %eax = 2 ]
>
> st %eax, preempt_count
>
> [ preempt_count = 2 ]
>
>
> We just caused preempt count to increment twice when it should have only
> incremented once, and this screws everything else up.
Ah! right!
> Do we really need to have preemption disabled throughout this? Is it
> because we don't want to migrate or call schedule? Not sure what the
> best way to fix this is. Perhaps we add a kprobe_preempt_disable() that
> is checked as well?
I think the best way to do that is just removing preemption disabling
code, because
- breakpoint exception itself disables interrupt (at least on x86)
- While single stepping, interrupts also be disabled.
(BTW, theoretically, boosted and optimized kprobes shouldn't have
this problem, because those doesn't execute single-stepping)
So, I think there is no reason of disabling preemption.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
next prev parent reply other threads:[~2011-07-01 1:12 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-30 13:23 [BUG] kprobes crashing because of preempt count Steven Rostedt
2011-06-30 15:51 ` [RFC][PATCH] kprobes: Add separate preempt_disabling for kprobes Steven Rostedt
2011-06-30 16:14 ` Frederic Weisbecker
2011-06-30 16:46 ` Steven Rostedt
2011-06-30 19:40 ` Jason Baron
2011-06-30 19:42 ` Steven Rostedt
2011-06-30 21:56 ` Peter Zijlstra
2011-07-01 1:22 ` Masami Hiramatsu
2011-07-01 1:38 ` Steven Rostedt
2011-07-01 1:52 ` Masami Hiramatsu
2011-07-01 5:09 ` Masami Hiramatsu
2011-07-01 11:13 ` Masami Hiramatsu
2011-07-01 12:54 ` Steven Rostedt
2011-07-01 12:19 ` Steven Rostedt
2011-07-01 13:15 ` Masami Hiramatsu
2011-07-01 13:14 ` [RFC PATCH -tip ] [BUGFIX] x86: Remove preempt disabling from kprobes Masami Hiramatsu
2011-07-01 13:43 ` Steven Rostedt
2011-07-01 13:53 ` Steven Rostedt
2011-07-03 2:05 ` Masami Hiramatsu
2011-07-02 6:09 ` Ananth N Mavinakayanahalli
2011-07-01 1:12 ` Masami Hiramatsu [this message]
2011-07-01 1:33 ` [BUG] kprobes crashing because of preempt count Steven Rostedt
2011-07-01 2:23 ` Masami Hiramatsu
2011-07-01 11:36 ` Ananth N Mavinakayanahalli
2011-07-01 12:01 ` Masami Hiramatsu
2011-07-01 13:03 ` Ananth N Mavinakayanahalli
2011-07-01 13:19 ` Steven Rostedt
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=4E0D1EE3.6080607@hitachi.com \
--to=masami.hiramatsu.pt@hitachi.com \
--cc=a.p.zijlstra@chello.nl \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=yrl.pp-manager.tt@hitachi.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.