static int trace_kprobe_module_callback(struct notifier_block *nb,
> unsigned long val, void *data)
> @@ -704,6 +712,7 @@ static struct notifier_block trace_kprobe_module_nb = {
> .notifier_call = trace_kprobe_module_callback,
> .priority = 1 /* Invoked after kprobe module callback */
> };
> +#endif
>
> static int count_symbols(void *data, unsigned long unused)
> {
> @@ -1933,8 +1942,10 @@ static __init int init_kprobe_trace_early(void)
> if (ret)
> return ret;
>
> +#ifdef CONFIG_MODULES
> if (register_module_notifier(_kprobe_module_nb))
> return -EINVAL;
> +#endif
>
> return 0;
> }
> --
> 2.43.0
>
>
--
Masami Hiramatsu
opcode_t *)ftrace_addr;
As I said, this must be
if (ftrace_addr != addr)
return -EILSEQ;
This will prevent users from being confused by the results of probing
that 'func' and 'func+4' are the same. (now only 'func' is allowed to
be probed.)
Thank you,
> +#endif
> +
> if (addr)
> return addr;
>
> --
> 2.35.1
>
--
Masami Hiramatsu
rec, unsigned
> long old_addr,
> /*
>* Out of range jumps are called from modules.
>*/
> - if (!rec->arch.mod) {
> + if (!ftrace_mod_addr_get(rec)) {
> pr_err("No module loaded\n");
> return -EINVAL;
> }
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index f9feb197b2daaf..68f20cf34b0c47 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -1510,6 +1510,7 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long
> ip, void *regs)
> }
>
>
> +#ifndef ftrace_cmp_recs
> static int ftrace_cmp_recs(const void *a, const void *b)
> {
> const struct dyn_ftrace *key = a;
> @@ -1521,6 +1522,7 @@ static int ftrace_cmp_recs(const void *a, const void *b)
> return 1;
> return 0;
> }
> +#endif
>
> static struct dyn_ftrace *lookup_rec(unsigned long start, unsigned long end)
> {
> --
> 2.35.1
>
--
Masami Hiramatsu
3: warning: no previous prototype for
> >> 'free_optinsn_page' [-Wmissing-prototypes]
> 329 | void __weak free_optinsn_page(void *page)
> | ^~~~~~~~~
Ah, we need a prototype for those in include/linux/kprobes.h
as same as alloc_insn_page() and free_insn_page().
Thank you,
--
Masami Hiramatsu
obes, extable: Identify
> kprobes trampolines as kernel text area")
>
This looks good to me.
Acked-by: Masami Hiramatsu
> Suggested-by: Masami Hiramatsu
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/kernel/optprobes.c | 23 +--
> 1 file c
ee_optinsn_page(), that
> fall back on alloc_insn_page() and free_insn_page() when not
> overriden by the architecture.
>
Looks good to me :)
Acked-by: Masami Hiramatsu
> Suggested-by: Masami Hiramatsu
> Signed-off-by: Christophe Leroy
> ---
> kernel/kprobes.c | 14 --
&g
ttps://lkml.kernel.org/r/20201028115613.140212...@goodmis.org
>
Looks good to me.
Acked-by: Masami Hiramatsu
Thank you!
> Cc: Andrew Morton
> Cc: Masami Hiramatsu
> Cc: Guo Ren
> Cc: "James E.J. Bottomley"
> Cc: Helge Deller
> Cc: Michael Ellerman
> Cc: Benjami
nel.org/r/20201028115613.140212...@goodmis.org
>
> Cc: Andrew Morton
> Cc: Masami Hiramatsu
> Cc: Guo Ren
> Cc: "James E.J. Bottomley"
> Cc: Helge Deller
> Cc: Michael Ellerman
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Heiko Carstens
&
On Thu, 29 Oct 2020 09:40:01 -0400
Steven Rostedt wrote:
> On Thu, 29 Oct 2020 16:58:03 +0900
> Masami Hiramatsu wrote:
>
> > Hi Steve,
> >
> > On Wed, 28 Oct 2020 07:52:49 -0400
> > Steven Rostedt wrote:
> >
> > > From: "Steven Rost
obe_ctlblk *kcb;
> + int bit;
>
> - /* Preempt is disabled by ftrace */
> + bit = ftrace_test_recursion_trylock();
> + if (bit < 0)
> + return;
> +
> + preempt_disable_notrace();
> p = get_kprobe((kprobe_opcode_t *)ip);
> if (unlikely(!p) || kprobe_disabled(p))
> - return;
> + goto out;
>
> kcb = get_kprobe_ctlblk();
> if (kprobe_running()) {
> @@ -52,6 +57,9 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long
> parent_ip,
>*/
> __this_cpu_write(current_kprobe, NULL);
> }
> +out:
> + preempt_enable_notrace();
> + ftrace_test_recursion_unlock(bit);
> }
> NOKPROBE_SYMBOL(kprobe_ftrace_handler);
>
> --
> 2.28.0
>
>
--
Masami Hiramatsu
xt happens to be in arch specific code.
>
> If you want something special for ftrace, you could just add your own
> function. But for x86, a text_alloc_immediate() would work.
> (BTW, I like the function names over the enums)
kprobes will need the text_alloc_immediate() too, since it will use
the trampoline buffer where jumps to/from kernel code/modules.
Thanks,
--
Masami Hiramatsu
red to handle events in real mode and functions
> running in real mode should have been blacklisted, so kprobe_handler()
> can safely bail out telling 'this trap is not mine' for any trap that
> happened while in real-mode.
>
> If the trap happened with MSR_IR or MSR_DR cleared,
proposal below ? If a trap is
> >> encoutered in real mode, if checks if the matching virtual address
> >> corresponds to a valid kprobe. If it is, it skips it. If not, it returns
> >> 0 to tell "it's no me". You are also talking about incrementing the
> >> missed count. Who do we do that ?
> >
> > I rather like your first patch. If there is a kprobes, we can not skip
> > the instruction, because there is an instruction which must be executed.
> > (or single-skipped, but I'm not sure the emulator works correctly on
> > real mode)
>
> Oops, yes of course.
Thank you,
--
Masami Hiramatsu
about incrementing the
> missed count. Who do we do that ?
I rather like your first patch. If there is a kprobes, we can not skip
the instruction, because there is an instruction which must be executed.
(or single-skipped, but I'm not sure the emulator works correctly on
real mode)
Thank you,
>
>
>
> @@ -264,6 +265,13 @@ int kprobe_handler(struct pt_regs *regs)
> if (user_mode(regs))
> return 0;
>
> +if (!(regs->msr & MSR_IR)) {
> +if (!get_kprobe(phys_to_virt(regs->nip)))
> +return 0;
> +regs->nip += 4;
> +return 1;
> +}
> +
> /*
>* We don't want to be preempted for the entire
>* duration of kprobe processing
>
>
> >
> > BTW, can the emulater handle the real mode code correctly?
>
> I don't know, how do I test that ?
>
> Christophe
--
Masami Hiramatsu
On Mon, 17 Feb 2020 16:38:50 +0100
Christophe Leroy wrote:
>
>
> Le 17/02/2020 à 11:27, Masami Hiramatsu a écrit :
> > On Mon, 17 Feb 2020 10:03:22 +0100
> > Christophe Leroy wrote:
> >
> >>
> >>
> >> Le 16/02/2020 à 13:34, Masami Hirama
On Mon, 17 Feb 2020 10:03:22 +0100
Christophe Leroy wrote:
>
>
> Le 16/02/2020 à 13:34, Masami Hiramatsu a écrit :
> > On Sat, 15 Feb 2020 11:28:49 +0100
> > Christophe Leroy wrote:
> >
> >> Hi,
> >>
> >> Le 14/02/2020 à 14:54, Masami Hi
On Sat, 15 Feb 2020 11:28:49 +0100
Christophe Leroy wrote:
> Hi,
>
> Le 14/02/2020 à 14:54, Masami Hiramatsu a écrit :
> > Hi,
> >
> > On Fri, 14 Feb 2020 12:47:49 + (UTC)
> > Christophe Leroy wrote:
> >
> >> When a program check exceptio
>
> DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
> @@ -264,6 +265,9 @@ int kprobe_handler(struct pt_regs *regs)
> if (user_mode(regs))
> return 0;
>
> + if (!(regs->msr & MSR_IR))
> + addr = phys_to_virt(regs->nip);
> +
> /*
>* We don't want to be preempted for the entire
>* duration of kprobe processing
> --
> 2.25.0
>
--
Masami Hiramatsu
t"
>
> From: Arnaldo Carvalho de Melo
> Message-ID:
>
> On January 10, 2020 9:23:27 PM GMT-03:00, Song Liu
> wrote:
> >
> >
> >> On Jan 10, 2020, at 3:47 PM, Masami Hiramatsu
> >wrote:
> >>
> >> On Fri, 10 Jan 2020 13:45:31 -0
On Fri, 10 Jan 2020 13:45:31 -0300
Arnaldo Carvalho de Melo wrote:
> Em Sat, Jan 11, 2020 at 12:52:13AM +0900, Masami Hiramatsu escreveu:
> > On Fri, 10 Jan 2020 15:02:34 +0100 Peter Zijlstra
> > wrote:
> > > Again, this only allows attaching to previously
eating kprobes, right?
>
> That is; I don't think CAP_SYS_PERFMON should be allowed to create
> kprobes.
>
> As might be clear; I don't actually know what the user-ABI is for
> creating kprobes.
There are 2 ABIs nowadays, ftrace and ebpf. perf-probe uses ftrace interface to
define new kprobe events, and those events are treated as completely same as
tracepoint events. On the other hand, ebpf tries to define new probe event
via perf_event interface. Above one is that interface. IOW, it creates new
kprobe.
Thank you,
--
Masami Hiramatsu
asm-generic header (Will);
> * Used APIs to access pt_regs elements (Will);
> * Fixed typos in the comments (Will).
This looks good to me.
Reviewed-by: Masami Hiramatsu
Thank you!
>
>
> Leo Yan (3):
> error-injection: Consolidate override function definitio
s stud
> >> definitions for generic kporbe_fault_handler() and kprobes_built_in() can
> >> just be dropped. Only on x86 it needs to be added back locally as it gets
> >> used in a !CONFIG_KPROBES function do_general_protection().
> >
> > If you want to remove kprobes_built_in(), you should replace it with
> > IS_ENABLED(CONFIG_KPROBES), instead of this...
>
> Apart from kprobes_built_in() the intent was to remove !CONFIG_KPROBES
> stub for kprobe_fault_handler() as well which required making generic
> kprobe_page_fault() to be empty in such case.
No, I meant that "IS_ENABLED(CONFIG_KPROBES)" is generic and is equal to
what kprobes_built_in() does.
Thank you,
--
Masami Hiramatsu
lerman
> Cc: Heiko Carstens
> Cc: Vasily Gorbik
> Cc: Christian Borntraeger
> Cc: Yoshinori Sato
> Cc: Rich Felker
> Cc: "David S. Miller"
> Cc: Thomas Gleixner
> Cc: Ingo Molnar
> Cc: Borislav Petkov
> Cc: "H. Peter Anvin"
> Cc: "
ocation()
>
> In addition, we update kprobe_ftrace_handler() to detect this scenarios
> and to pass the proper nip to the pre and post probe handlers.
>
> Signed-off-by: Naveen N. Rao
Looks good to me.
Reviewed-by: Masami Hiramatsu
Thank you!
> ---
> arch/powerpc/kern
_handler returns !0, it changes regs->nip. We have to
> @@ -57,6 +82,11 @@ NOKPROBE_SYMBOL(kprobe_ftrace_handler);
>
> int arch_prepare_kprobe_ftrace(struct kprobe *p)
> {
> + if ((unsigned long)p->addr & 0x03) {
> + printk("Attempt to register kprobe at an unaligned address\n");
> + return -EILSEQ;
> + }
> +
> p->ainsn.insn = NULL;
> p->ainsn.boostable = -1;
> return 0;
> --
> 2.22.0
>
--
Masami Hiramatsu
e_function() to
> identify the filter IP.
This looks good to me.
Acked-by: Masami Hiramatsu
Thank you!
>
> Signed-off-by: Naveen N. Rao
> ---
> kernel/kprobes.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/kprobes.c
: for powerpc and x86, this removes all preempt_disable
from kprobe_ftrace_handler because ftrace callbacks are
called under preempt disabled.
Signed-off-by: Masami Hiramatsu
Acked-by: "Naveen N. Rao"
Cc: Vineet Gupta
Cc: Russell King
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Tony Luck
C
Don't call the ->break_handler() from the powerpc kprobes code,
because it was only used by jprobes which got removed.
This also removes skip_singlestep() and embeds it in the
caller, kprobe_ftrace_handler(), which simplifies regs->nip
operation around there.
Signed-off-by: Masami Hir
Remove arch dependent setjump/longjump functions
and unused fields in kprobe_ctlblk for jprobes
from arch/powerpc. This also reverts commits
related __is_active_jprobe() function.
Signed-off-by: Masami Hiramatsu
Acked-by: "Naveen N. Rao"
Cc: Benjamin Herrenschmidt
Cc: Paul Mac
On Thu, 07 Jun 2018 22:07:26 +0530
"Naveen N. Rao" wrote:
> Masami Hiramatsu wrote:
> > On Thu, 07 Jun 2018 17:07:00 +0530
> > "Naveen N. Rao" wrote:
> >
> >> Masami Hiramatsu wrote:
> >> &
On Thu, 07 Jun 2018 17:07:00 +0530
"Naveen N. Rao" wrote:
> Masami Hiramatsu wrote:
> > Don't call the ->break_handler() from the arm kprobes code,
> ^^^ powerpc
>
> > because it was only used by jprobes which got
On Thu, 07 Jun 2018 17:01:23 +0530
"Naveen N. Rao" wrote:
> Masami Hiramatsu wrote:
> > Remove arch dependent setjump/longjump functions
> > and unused fields in kprobe_ctlblk for jprobes
> > from arch/powerpc. This also reverts commits
> > related __is_acti
: for powerpc and x86, this removes all preempt_disable
from kprobe_ftrace_handler because ftrace callbacks are
called under preempt disabled.
Signed-off-by: Masami Hiramatsu
Cc: Vineet Gupta
Cc: Russell King
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Tony Luck
Cc: Fenghua Yu
Cc: Ralf Baechle
Cc
Don't call the ->break_handler() from the arm kprobes code,
because it was only used by jprobes which got removed.
This also makes skip_singlestep() a static function since
only ftrace-kprobe.c is using this function.
Signed-off-by: Masami Hiramatsu
Cc: Benjamin Herrenschmidt
Cc: P
Remove arch dependent setjump/longjump functions
and unused fields in kprobe_ctlblk for jprobes
from arch/powerpc. This also reverts commits
related __is_active_jprobe() function.
Signed-off-by: Masami Hiramatsu
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: "Nav
On Thu, 31 May 2018 15:39:03 +0530
"Naveen N. Rao" wrote:
> Masami Hiramatsu wrote:
> > On Tue, 29 May 2018 18:06:02 +0530
> > "Naveen N. Rao" wrote:
> >
> >> We already have an arch-independent way to set the instruction pointer
> >>
t pt_regs *, regs, unsigned long, rc)
> {
> regs_set_return_value(regs, rc);
> - override_function_with_return(regs);
> + instruction_pointer_set(regs, (unsigned long)_return_func);
> return 0;
> }
>
> diff --git a/lib/error-inject.c b/lib/error-inject.c
> index c0d4600f4896..7fdc92b5babc 100644
> --- a/lib/error-inject.c
> +++ b/lib/error-inject.c
> @@ -20,6 +20,14 @@ struct ei_entry {
> void *priv;
> };
>
> +asm(
> + ".type just_return_func, @function\n"
> + ".globl just_return_func\n"
> + "just_return_func:\n"
> + ARCH_FUNC_RET "\n"
> + ".size just_return_func, .-just_return_func\n"
> +);
> +
> bool within_error_injection_list(unsigned long addr)
> {
> struct ei_entry *ent;
> --
> 2.17.0
>
--
Masami Hiramatsu
ing earlier. However, this function was not added
> to the kprobes blacklist. Add it so as to prevent it from being probed.
>
Looks good to me.
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Thank you!
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
>
; + ret = strscpy(dot_name + len, c, KSYM_NAME_LEN);
> + if (ret > 0)
> + addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name);
> +
> + /* Fallback to the original non-dot symbol lookup */
> + if (!addr && dot_appended)
> addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
Then we can remove dot_appended.
Thank you,
> - }
> #else
> addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
> #endif
> --
> 2.14.2
>
--
Masami Hiramatsu <mhira...@kernel.org>
b2e3d783964 ("kprobes/x86: Remove
> IRQ disabling from ftrace-based/optimized kprobes"). Do the same for
> powerpc.
Yes, and this requires to make preempt disable :)
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Thank you!
>
> Signed-off-by: Naveen N. Rao <navee
Actually, if you local_irq_save(), it also disables preempt. So unless you
enables irqs, it should be safe.
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Thank you,
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/optprobes.c |
On Thu, 14 Sep 2017 15:55:39 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> On 2017/09/13 05:05PM, Masami Hiramatsu wrote:
> > On Thu, 14 Sep 2017 02:50:35 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> &
On Thu, 14 Sep 2017 12:17:20 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> On 2017/09/13 05:36PM, Masami Hiramatsu wrote:
> > On Thu, 14 Sep 2017 02:50:34 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
>
On Thu, 14 Sep 2017 12:08:07 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> On 2017/09/13 04:53PM, Masami Hiramatsu wrote:
> > On Thu, 14 Sep 2017 02:50:33 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> &g
On Thu, 14 Sep 2017 02:50:36 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> Fix a circa 2005 FIXME by implementing a check to ensure that we
> actually got into the jprobe break handler() due to the trap in
> jprobe_return().
>
Looks good to me.
obe_running();
> - return (p && (unsigned long)p->addr == addr) ? 1 : 0;
> + if (!preemptible()) {
> + struct kprobe *p = raw_cpu_read(current_kprobe);
> + return (p && (unsigned long)p->addr == addr) ? 1 : 0;
> + }
> +
> + return 0;
> }
>
> bool arch_within_kprobe_blacklist(unsigned long addr)
> --
> 2.14.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
3d22001b 39299240 8129 2f89 409effc8 3c82ffcb
> 3c62ffcb
> [3.149879] 3884bc68 3863bc18 4803d5fd 6000 <0fe0> 4ba8
> 6000 6000
> [3.149890] ---[ end trace 432dd46b4ce3d29f ]---
> [3.166003] Kprobe smoke test: passed successfully
+ * then this is just an error with the current run. We return
> + * 0 so that this is now single-stepped, but continue to try
> + * emulating it in subsequent probe hits.
> + */
> + if (unlikely(p->ainsn.boostable != 1))
> + p->ainsn.boostable = -1;
> + }
>
> return ret;
> }
> --
> 2.14.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
.@linux.vnet.ibm.com>
Reviewed-by: Masami Hiramatsu <mhira...@kernel.org>
Thanks!
> ---
> arch/powerpc/kernel/kprobes.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> in
On Mon, 3 Jul 2017 12:27:33 +0900
Masami Hiramatsu <mhira...@kernel.org> wrote:
> On Thu, 29 Jun 2017 19:05:37 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
>
> > Add a kprobes test to ensure that we are able to add a probe on a
> >
On Thu, 29 Jun 2017 19:05:39 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> From: Masami Hiramatsu <mhira...@kernel.org>
>
> Add a testcase for kprobe event naming. This testcase
> checks whether the kprobe events can automatically ganerate
&
on ftrace location. The current offset of 4 will fall before the
> function local entry point and won't fire, while an offset of 12 or 16
> will fall on ftrace location. Offset 8 is currently guaranteed to not be
> the ftrace location.
OK, looks good to me.
Acked-by: Masami Hiramatsu <mhi
On Thu, 29 Jun 2017 19:05:37 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> Add a kprobes test to ensure that we are able to add a probe on a
> module function using 'p :' format, without having to
> specify a probe name.
>
> Suggested-by: Masami
On Thu, 29 Jun 2017 00:13:24 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> On 2017/06/28 11:16PM, Masami Hiramatsu wrote:
> > > > diff --git
> > > > a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc
> > &
On Wed, 28 Jun 2017 14:58:46 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> On 2017/06/24 08:06PM, Masami Hiramatsu wrote:
> > On Sat, 24 Jun 2017 02:30:21 +0900
> > Masami Hiramatsu <mhira...@kernel.org> wrote:
> >
> > > On
On Sat, 24 Jun 2017 02:30:21 +0900
Masami Hiramatsu <mhira...@kernel.org> wrote:
> On Thu, 22 Jun 2017 22:33:25 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
>
> > On 2017/06/22 06:07PM, Masami Hiramatsu wrote:
> > > On Thu, 2
On Fri, 23 Jun 2017 00:33:45 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> On 2017/06/22 06:29PM, Masami Hiramatsu wrote:
> > On Thu, 22 Jun 2017 00:20:27 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
>
On Thu, 22 Jun 2017 22:33:25 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> On 2017/06/22 06:07PM, Masami Hiramatsu wrote:
> > On Thu, 22 Jun 2017 00:20:28 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
s not rejected.
Oops, ok, this is my mistake.
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
This must be marked as bugfix for stable trees.
Could you also add a testcase for this (module name) bug?
MODNAME=`lsmod | head -n 2 | tail -n 1 | cut -f 1 -d " "`
FUNCNAME=`grep -m
i}+${OFFS} ; done > kprobe_events ||:
> +grep t /proc/kallsyms | cut -f3 -d" " | head -n 256 | \
> +while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||:
>
> echo 1 > events/kprobes/enable
> echo 0 > events/kprobes/enable
> --
> 2.13.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
On Wed, 14 Jun 2017 11:40:08 +0900
Masami Hiramatsu <mhira...@kernel.org> wrote:
> On Fri, 9 Jun 2017 00:53:08 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
>
> > Add a test to verify that the registers passed in pt_regs
t... skipping optprobe test\n");
Yes, this may take a while... you may need to wait for optimizer,
like wait_for_kprobe_optimizer().
Thank you,
> + unregister_kprobe();
> + goto summary;
> + }
> +
> + preh_val = 0;
> + arch_kprobe_regs_function();
> + unregister_kprobe();
> +
> + if (preh_val == 0) {
> + pr_err("optprobe pre_handler regs validation failed\n");
> + handler_errors++;
> + }
> +
> +summary:
> + if (errors)
> + pr_err("BUG: %d out of %d tests failed\n", errors, num_tests);
> + else if (handler_errors)
> + pr_err("BUG: %d error(s) running handlers\n", handler_errors);
> + else
> + pr_info("passed successfully\n");
> +}
> +#endif
> +#endif
> +
> int init_test_probes(void)
> {
> int ret;
> @@ -378,12 +539,34 @@ int init_test_probes(void)
> errors++;
> #endif /* CONFIG_KRETPROBES */
>
> +#ifdef HAVE_KPROBES_REGS_SANITY_TEST
> + num_tests++;
> + ret = test_kprobe_regs();
> + if (ret < 0)
> + errors++;
> +
> +#ifdef CONFIG_KPROBES_ON_FTRACE
> + num_tests++;
> + ret = test_kp_on_ftrace_regs();
> + if (ret < 0)
> + errors++;
> +#endif
> +
> +#ifdef CONFIG_OPTPROBES
> + num_tests++;
> + test_optprobe_regs_setup();
> + schedule_delayed_work(_optprobe_regs_work, 10);
> +#endif
> +#endif
> +
> +#if !defined(HAVE_KPROBES_REGS_SANITY_TEST) || !defined(CONFIG_OPTPROBES)
> if (errors)
> pr_err("BUG: %d out of %d tests failed\n", errors, num_tests);
> else if (handler_errors)
> pr_err("BUG: %d error(s) running handlers\n", handler_errors);
> else
> pr_info("passed successfully\n");
> +#endif
>
> return 0;
> }
> --
> 2.12.2
>
--
Masami Hiramatsu <mhira...@kernel.org>
seems good to me, but I weould like to leave it for
PPC64 maintainer too.
Reviewed-by: Masami Hiramatsu <mhira...@kernel.org>
Thanks!
>
> Fixes: ead514d5fb30a ("powerpc/kprobes: Add support for
> KPROBES_ON_FTRACE")
> Signed-off-by: Naveen N. Rao <naveen.n@l
cing before invoking the jprobe hook and re-enable it
> when returning back to the original jprobe'd function.
>
> Fixes: 6794c78243bfd ("powerpc64: port of the function graph tracer")
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
Acked-by: Ma
IOW, please return silently, or just add a
debug message.
Thank you,
> + }
> +
> memcpy(regs, >jprobe_saved_regs, sizeof(struct pt_regs));
> sp = kernel_stack_pointer(regs);
> memcpy((void *)sp, >jprobes_stack, MIN_STACK_SIZE(sp));
> --
> 2.12.2
>
--
Masami Hiramatsu <mhira...@kernel.org>
Could you add CUR_STACK_SIZE(addr) as x86 does instead of repeating similar
code?
Thank you,
--
Masami Hiramatsu <mhira...@kernel.org>
s into private -- these are labels that I
> felt are not necessary to read stack traces. If any of those are
> important to have, please let me know.
At least from the viewpoint of kprobe-blacklist macros, it seems
good to me :)
Reviewed-by: Masami Hiramatsu <mhira...@kernel.org>
for this
t the code too
> much.
>
> Thanks for the review!
> - Naveen
>
> --
> [PATCH v2] kallsyms: optimize kallsyms_lookup_name() for a few cases
>
> 1. Fail early for invalid/zero length symbols.
> 2. Detect names of the form and skip checking for kernel
> sym
addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name);
> +
> + /* Fallback to the original non-dot symbol lookup */
> + if (!addr && dot_appended)
> addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
> - }
> #else
> addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
> #endif
> --
> 2.12.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
rn about
> > "longer" string (for performance reason), we can focus on
> > such case.
> > (BTW, could you also check the name != NULL at first?)
> >
> > So, what I think it can be;
> >
> > if (strnlen(s+1, KSYM_NAME_LEN) == KSYM_NAME_LEN ||
> > (size_t)(s - name) >= MODULE_NAME_LEN)
> > return false;
>
> Sure, thanks. I clearly need to refactor this code better!
>
> - Naveen
>
>
--
Masami Hiramatsu <mhira...@kernel.org>
b/kernel/trace/trace_kprobe.c
> @@ -704,6 +704,10 @@ static int create_trace_kprobe(int argc, char **argv)
> pr_info("Return probe must be used without offset.\n");
> return -EINVAL;
> }
> + if (!is_valid_kprobe_symbol_name(symbol)) {
> + pr_info("Symbol name is too long.\n");
> + return -EINVAL;
> + }
> }
> argc -= 2; argv += 2;
>
> --
> 2.12.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
> felt that Michael Ellerman prefers to keep functional changes separate
> > from refactoring. I'm fine with either approach.
> >
> > Michael?
>
> Yeah I'd definitely like this to be two patches. Otherwise when reading
> the combined diff it's much harder to s
turn ERR_PTR(-ENOENT);
> >> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> >> index 5f688cc724f0..bf73e5f31128 100644
> >> --- a/kernel/trace/trace_kprobe.c
> >> +++ b/kernel/trace/trace_kprobe.c
> >> @@ -704,6 +704,10 @@ static int create_trace_kprobe(int argc, char **argv)
> >>pr_info("Return probe must be used without offset.\n");
> >>return -EINVAL;
> >>}
> >> + if (!is_valid_kprobe_symbol_name(symbol)) {
> >> + pr_info("Symbol name is too long.\n");
> >> + return -EINVAL;
> >> + }
> >>}
> >>argc -= 2; argv += 2;
> >>
> >> --
> >> 2.12.1
> >>
> >
> > Thanks,
> >
> > --
> > Masami Hiramatsu <mhira...@kernel.org>
> >
> >
>
--
Masami Hiramatsu <mhira...@kernel.org>
* good to catch them, just in case...
> - */
> - printk("Can't step on instruction %x\n", insn);
> - BUG();
> - } else if (ret == 0)
> - /* This instruction can't be boosted */
> - p->ainsn.boostable = -1;
> + }
> }
> prepare_singlestep(p, regs);
> kcb->kprobe_status = KPROBE_HIT_SS;
> --
> 2.12.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
On Wed, 19 Apr 2017 18:21:06 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> set_current_kprobe() already saves regs->msr into kprobe_saved_msr. Remove
> the redundant save.
>
Looks good to me.
Reviewed-by: Masami Hiramatsu <mhira...@kernel
if (ret > 0) {
> + restore_previous_kprobe(kcb);
> + return 1;
> + }
> + }
> return 1;
> } else {
> if (*addr != BREAKPOINT_INSTRUCTION) {
> --
> 2.12.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
;);
> return -EINVAL;
> }
> + if (!is_valid_kprobe_symbol_name(symbol)) {
> + pr_info("Symbol name is too long.\n");
> + return -EINVAL;
> + }
> }
> argc -= 2; argv += 2;
>
> --
> 2.12.1
>
Thanks,
--
Masami Hiramatsu <mhira...@kernel.org>
);
> +
> + if (ret > 0) {
> + restore_previous_kprobe(kcb);
> + return 1;
> + }
> + }
> return 1;
> } else {
> if (*addr != BREAKPOINT_INSTRUCTION) {
> --
> 2.12.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
mtmsr(d)/rfi(d), etc.
> - * So, we should never get here... but, its still
> - * good to catch them, just in case...
> - */
> - printk("Can't step on instruction %x\n", insn);
> - BUG();
> - } else if (ret == 0)
> - /* This instruction can't be boosted */
> - p->ainsn.boostable = -1;
> + }
> }
> prepare_singlestep(p, regs);
> kcb->kprobe_status = KPROBE_HIT_SS;
> --
> 2.12.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
e("optimized_callback", 0);
> - emulate_step_addr = kprobe_lookup_name("emulate_step", 0);
> + op_callback_addr = (kprobe_opcode_t
> *)ppc_kallsyms_lookup_name("optimized_callback");
> + emulate_step_addr = (kprobe_opcode_t
> *)ppc_kallsyms_lookup_name("emulate_step");
> if (!op_callback_addr || !emulate_step_addr) {
> - WARN(1, "kprobe_lookup_name() failed\n");
> + WARN(1, "Unable to lookup
> optimized_callback()/emulate_step()\n");
> goto error;
> }
>
> --
> 2.12.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
(struct kprobe_ctlblk, kprobe_ctlblk);
>
> struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
>
> -kprobe_opcode_t *kprobe_lookup_name(const char *name)
> +kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
Hmm, if we do this change, it is natural that kprobe_lookup_name()
returns the address + offset.
Thank you,
--
Masami Hiramatsu <mhira...@kernel.org>
nction.
Looks good to me.
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Thanks!
>
> Suggested-by: Masami Hiramatsu <mhira...@kernel.org>
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/kprobes.h | 53 -
| 6 +-
> include/linux/kprobes.h | 1 +
> kernel/kprobes.c | 21 +++---
> 6 files changed, 147 insertions(+), 90 deletions(-)
>
> --
> 2.12.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
so as to ensure the probe gets hit. Do it only
> if the kernel supports specifying offsets with kretprobes.
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
This patch is OK. And I found that most of functions in sym-handling.c
are used only when libelf is supported. (e.g. probe-event.
OKPROBE_SYMBOL() or nokprobe_inline. The latter
> forces inlining, in which case the caller needs to be added to
> NOKPROBE_SYMBOL().
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
OK, this is a good starting point so far. please consider that those
functions really have to be protected by
ce_readme_table[i].avail =
> + strglobmatch(buf,
> ftrace_readme_table[i].pattern);
> + scanned = true;
> +
> + fclose(fp);
> + free(buf);
> +
> +result:
> + if (type >= FTRACE_README_END)
> + return false;
> +
> + return ftrace_readme_table[type].avail;
> +}
> +
> +bool kretprobe_offset_is_supported(void)
> +{
> + return scan_ftrace_readme(FTRACE_README_KRETPROBE_OFFSET);
> +}
> +
> +bool probe_type_x_is_supported(void)
> +{
> + return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
> +}
> diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
> index 1fbc044f9eb0..ecda2d822daf 100644
> --- a/tools/perf/util/trace-event.h
> +++ b/tools/perf/util/trace-event.h
> @@ -37,6 +37,10 @@ int parse_ftrace_file(struct pevent *pevent, char *buf,
> unsigned long size);
> int parse_event_file(struct pevent *pevent,
>char *buf, unsigned long size, char *sys);
>
> +int open_trace_file(const char *trace_file, bool readwrite);
> +bool kretprobe_offset_is_supported(void);
> +bool probe_type_x_is_supported(void);
> +
> unsigned long long
> raw_field_value(struct event_format *event, const char *name, void *data);
>
> --
> 2.11.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
ed = true;
> +
> + fclose(fp);
> + free(buf);
> +
> +result:
> + if (type >= FTRACE_README_END)
> + return false;
> +
> + return ftrace_readme_table[type].avail;
> +}
> +
> +bool kretprobe_offset_is_supported(void)
> +{
> + return scan_ftrace_readme(FTRACE_README_KRETPROBE_OFFSET);
> +}
> +
> +bool probe_type_x_is_supported(void)
> +{
> + return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
> +}
> diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
> index 1fbc044f9eb0..ecda2d822daf 100644
> --- a/tools/perf/util/trace-event.h
> +++ b/tools/perf/util/trace-event.h
> @@ -37,6 +37,10 @@ int parse_ftrace_file(struct pevent *pevent, char *buf,
> unsigned long size);
> int parse_event_file(struct pevent *pevent,
>char *buf, unsigned long size, char *sys);
>
> +int open_trace_file(const char *trace_file, bool readwrite);
> +bool kretprobe_offset_is_supported(void);
> +bool probe_type_x_is_supported(void);
> +
> unsigned long long
> raw_field_value(struct event_format *event, const char *name, void *data);
>
> --
> 2.11.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
On Mon, 6 Mar 2017 20:34:10 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> On 2017/03/04 09:49AM, Masami Hiramatsu wrote:
> > On Thu, 2 Mar 2017 23:25:06 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> >
tch is fine?
OK, looks good to me:)
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Thanks!
>
> Thanks,
> Naveen
>
> --
> trace/kprobes: fix check for kretprobe offset within function entry
>
> perf specifies an offset from _text and since this offset is fed
> d
On Sat, 4 Mar 2017 11:35:51 +0900
Masami Hiramatsu <mhira...@kernel.org> wrote:
> On Sat, 4 Mar 2017 09:49:11 +0900
> Masami Hiramatsu <mhira...@kernel.org> wrote:
>
> > On Thu, 2 Mar 2017 23:25:06 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.
On Sat, 4 Mar 2017 11:35:51 +0900
Masami Hiramatsu <mhira...@kernel.org> wrote:
> On Sat, 4 Mar 2017 09:49:11 +0900
> Masami Hiramatsu <mhira...@kernel.org> wrote:
>
> > On Thu, 2 Mar 2017 23:25:06 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.
On Sat, 4 Mar 2017 09:49:11 +0900
Masami Hiramatsu <mhira...@kernel.org> wrote:
> On Thu, 2 Mar 2017 23:25:06 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
>
> > We indicate support for accepting sym+offset with kretprobes through a
>
so as to ensure the probe gets hit. Do it only
> if the kernel supports specifying offsets with kretprobes.
Looks good to me.
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Thanks!
>
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
> ---
> tools/perf/a
*probe_cache__find_by_name(struct
> probe_cache *pcache,
> const char *group, const char *event);
> int probe_cache__show_all_caches(struct strfilter *filter);
> bool probe_type_is_available(enum probe_type type);
> +bool kretprobe_offset_is_supported(void);
> #else/* ! HAVE_LIBELF_SUPPORT */
> static inline struct probe_cache *probe_cache__new(const char *tgt
> __maybe_unused)
> {
> --
> 2.11.1
>
--
Masami Hiramatsu <mhira...@kernel.org>
t.
>
> Since we are only interested in availability of probe argument type x,
> we will only scan for that.
Ah, OK, this can simplify and shorten the actual scanning time.
If there are any needs for checking those in the future, we can
add it again at that moment.
Acked-by: Masami Hiramats
On Mon, 27 Feb 2017 11:52:04 -0500
"Steven Rostedt (VMware)" <rost...@goodmis.org> wrote:
> Let's not remove the warning about offsets and return probes when the
> offset is invalid.
Agreed, This looks good to me.
Acked-by: Masami Hiramatsu <mhira...@kernel.org&g
On Fri, 24 Feb 2017 17:11:03 -0300
Arnaldo Carvalho de Melo <a...@kernel.org> wrote:
> Em Sat, Feb 25, 2017 at 02:29:17AM +0900, Masami Hiramatsu escreveu:
> > On Fri, 24 Feb 2017 00:46:08 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrot
On Fri, 24 Feb 2017 00:46:08 +0530
"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> On 2017/02/23 06:10PM, Masami Hiramatsu wrote:
> > On Wed, 22 Feb 2017 19:23:40 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> >
nel_get_ref_reloc_sym();
> if (!reloc_sym) {
> pr_warning("Relocated base symbol is not found!\n");
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 5d4e94061402..449d4f311355 100644
> --- a/tools/perf/util/probe-e
1 - 100 of 180 matches
Mail list logo
