On Mon, 8 Mar 2021 11:52:10 +0900
Masami Hiramatsu wrote:
> So, here is my idea;
>
> 1) Change the trampline code to prepare stack frame at first and save
>registers on it, instead of "push". This will makes ORC easy to setup
>stackframe information for this cod
ation for this code.
2) change the return addres fixup timing. Instead of using return value
of trampoline handler, before removing the real return address from
current->kretprobe_instances.
3) Then, if orc_find() finds the ip is in the kretprobe_trampoline, it
checks the contents of the end of stackframe (at the place of regs->sp)
is same as the address of it. If it is, it can find the correct address
from current->kretprobe_instances. If not, there is a correct address.
I need to find how the ORC info is prepared for 1), maybe UNWIND_HINT macro
may help?
Thank you,
--
Masami Hiramatsu
On Fri, 5 Mar 2021 11:16:45 -0800
Daniel Xu wrote:
> Hi Masami,
>
> On Sat, Mar 06, 2021 at 12:38:57AM +0900, Masami Hiramatsu wrote:
> > Hello,
> >
> > Here is a series of patches for kprobes and stacktracer to fix the kretprobe
> > entries in the kernel stac
Since the stacktrace API fixup the kretprobed address correctly,
there is no need to convert the "kretprobe_trampoline" to
"[unknown/kretprobe'd]" anymore. Remove it.
Signed-off-by: Masami Hiramatsu
---
kernel/trace/trace_output.c | 27 -
xae
=> 0
The trampoline_handler+0x48 is actual call site address,
not modified by kretprobe.
Reported-by: Daniel Xu
Signed-off-by: Masami Hiramatsu
---
include/linux/kprobes.h | 13
kernel/kprobes.c| 79 ---
kernel/stacktrace.c
Remove trampoline_address from kretprobe_trampoline_handler().
Instead of passing the address, kretprobe_trampoline_handler()
can use new kretprobe_trampoline_addr().
Signed-off-by: Masami Hiramatsu
---
arch/arc/kernel/kprobes.c |2 +-
arch/arm/probes/kprobes/core.c |3
Replace arch_deref_entry_point() with dereference_function_descriptor()
because those are doing same thing.
Signed-off-by: Masami Hiramatsu
---
arch/ia64/kernel/kprobes.c|5 -
arch/powerpc/kernel/kprobes.c | 11 ---
include/linux/kprobes.h |1 -
kernel/kprobes.c
dependent
stacktrace implementation must fixup by themselves.
Thank you,
---
Masami Hiramatsu (5):
ia64: kprobes: Fix to pass correct trampoline address to the handler
kprobes: treewide: Replace arch_deref_entry_point() with
dereference_function_descriptor()
kprobes: treewi
so changes to use correct symbol dereference function to get the
function address from the kretprobe_trampoline.
Fixes: e792ff804f49 ("ia64: kprobes: Use generic kretprobe trampoline handler")
Signed-off-by: Masami Hiramatsu
---
arch/ia64/kernel/kprobes.c |9 +
1 file c
On Fri, 5 Mar 2021 18:28:06 +0900
Masami Hiramatsu wrote:
> Hi Daniel,
>
> On Thu, 4 Mar 2021 16:07:52 -0800
> Daniel Xu wrote:
>
> > Getting a stack trace from inside a kretprobe used to work with frame
> > pointer stack walks. After the default unwinder was
> - node->next = NULL;
> -
> /* Run them.. */
> while (first) {
> ri = container_of(first, struct kretprobe_instance, llist);
> @@ -1917,6 +1913,10 @@ unsigned long __kretprobe_trampoline_handler(struct
> pt_regs *regs,
> recycle_rp_inst(ri);
> }
>
> + /* Unlink all nodes for this frame. */
> + current->kretprobe_instances.first = node->next;
> + node->next = NULL;
Nack, this is a bit dangerous. We should unlink the chunk of kretprobe
instances and
recycle it as I did in my patch, see below;
https://lore.kernel.org/bpf/20210304221947.5a177ce2e1e94314e57c3...@kernel.org/
I would like to fix this issue in the generic part, not for x86 only.
Let me refresh my series for fixing it.
Thank you,
--
Masami Hiramatsu
On Wed, 3 Mar 2021 09:26:04 -0500
Steven Rostedt wrote:
> On Wed, 3 Mar 2021 13:48:28 +0900
> Masami Hiramatsu wrote:
>
> >
> > >
> > > I think (can't prove) this used to work:
>
> Would be good to find out if it did.
>
> >
> >
Hi ShaoBo,
Thanks for backporting and real bug report!
On Wed, 3 Mar 2021 15:10:52 +0800
ShaoBo Huang wrote:
> From: Masami Hiramatsu
>
> commit f3fbd7ec62dec1528fb8044034e2885f2b257941 upstream
>
> This is arm port of commit 6a5022a56ac3 ("kprobes/x86: Allow to
> han
nterrupt+54
> asm_sysvec_apic_timer_interrupt+18
> ]: 1
> @[
> ftrace_trampoline+10799
> bpf_get_stackid_raw_tp+121
> ftrace_trampoline+10799
> __tun_chr_ioctl.isra.0.cold+33312
> __tcp_retransmit_skb+5
> <...>
>
> which makes me suspect it's a kprobe specific issue.
>
> Thanks,
> Daniel
--
Masami Hiramatsu
e is same.
Thus, I think this is just a cosmetic cleanup.
Signed-off-by: Masami Hiramatsu
---
arch/x86/kernel/kprobes/core.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 08674e7a5d7b..be76568d57a5 10
used), and post_handlers are
rarely used (I don't see it except for the test code).
Suggested-by: Andy Lutomirski
Signed-off-by: Masami Hiramatsu
---
Changes in v1:
- According to Peter's suggestion, merge the conditional jump
emulation code and use range style of switch-
t. We have to refer
the insn->modrm.bytes[1] instead.
Fixes: abd82e533d88 ("x86/kprobes: Do not decode opcode in resume_execution()")
Signed-off-by: Masami Hiramatsu
---
arch/x86/kernel/kprobes/core.c |6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x
some instructions becomes not able
to be probed, but as far as I can see those are not rare case.
Thank you,
---
Masami Hiramatsu (3):
x86/kprobes: Retrieve correct opcode for group instruction
x86/kprobes: Identify far indirect JMP correctly
x86/kprobes: Use int3 instead of debug tra
t; #endif
> break;
> default:
> - if ((opcode & 0xf0) == 0x70) {
> - /* 1 byte conditional jump */
> - p->ainsn.emulate_op = kprobe_emulate_jcc;
> - p->ainsn.jcc.type = opcode & 0xf;
> - p->ainsn.rel32 = *(char *)insn->immediate.bytes;
> - }
> + break;
> }
> p->ainsn.size = insn->length;
>
--
Masami Hiramatsu
to fix the
> problems with non-canonical addresses, and this emulation it still incomplete.
Yeah, I found another implementation of the emulation afterwards. Of cource
since uprobes only treat user-space, it maybe need more care.
Thank you,
--
Masami Hiramatsu
used), and post_handlers are
rarely used (I don't see it except for the test code).
Suggested-by: Andy Lutomirski
Signed-off-by: Masami Hiramatsu
---
arch/x86/include/asm/kprobes.h | 21 +-
arch/x86/kernel/kprobes/core.c | 515 ++--
arch/x86/kern
int3_emulate_*)
but it can be overengineering.
- Add testcases for emulator. Current kprobe smoke test is not arch specific.
Maybe better to probe an assembly target code so that it can test "boosted",
"emulated" or "int3-single-stepped" cases.
Thank you,
---
M
On Fri, 26 Feb 2021 19:30:06 +0100
Borislav Petkov wrote:
> On Sat, Feb 27, 2021 at 12:45:06AM +0900, Masami Hiramatsu wrote:
> > OK, but I think it should return -EINVAL or -EILSEQ for bad instruction.
>
> It does return -EINVAL when insn_complete() returns 0.
>
> >
rn 0 if failed */
> +/* Decode imm v32(Iz). Return a negative value if failed. */
> static int __get_immv32(struct insn *insn)
> {
> switch (insn->opnd_bytes) {
> @@ -480,10 +553,10 @@ static int __get_immv32(struct insn *insn)
> return 1;
>
> err_out:
> - return 0;
> + return -ENODATA;
Ditto.
> }
>
> -/* Decode imm v64(Iv/Ov), Return 0 if failed */
> +/* Decode imm v64(Iv/Ov). Return a negative value if failed. */
> static int __get_immv(struct insn *insn)
> {
> switch (insn->opnd_bytes) {
> @@ -507,11 +580,12 @@ static int __get_immv(struct insn *insn)
> insn->immediate1.got = insn->immediate2.got = 1;
>
> return 1;
> +
> err_out:
> - return 0;
> + return -ENODATA;
Ditto.
> }
>
> -/* Decode ptr16:16/32(Ap) */
> +/* Decode ptr16:16/32(Ap). Return a negative value if failed. */
> static int __get_immptr(struct insn *insn)
> {
> switch (insn->opnd_bytes) {
> @@ -534,25 +608,36 @@ static int __get_immptr(struct insn *insn)
> insn->immediate1.got = insn->immediate2.got = 1;
>
> return 1;
> +
> err_out:
> - return 0;
> + return -ENODATA;
Ditto.
Thank you,
--
Masami Hiramatsu
t. We have to refer
the insn->modrm.bytes[1] instead.
Fixes: abd82e533d88 ("x86/kprobes: Do not decode opcode in resume_execution()")
Signed-off-by: Masami Hiramatsu
---
arch/x86/kernel/kprobes/core.c |6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/x
e is same.
Thus, I think this is just a cosmetic cleanup.
Signed-off-by: Masami Hiramatsu
---
arch/x86/kernel/kprobes/core.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 08674e7a5d7b..be76568d57a5 10
thing
like a cosmetic patch, because the original code was mis-
understanding the opcode encoding, but the result is same.
Thank you,
---
Masami Hiramatsu (2):
x86/kprobes: Retrieve correct opcode for group instruction
x86/kprobes: Identify far indirect JMP correctly
arch/x86/kern
On Wed, 24 Feb 2021 22:03:12 -0800
Andy Lutomirski wrote:
> On Wed, Feb 24, 2021 at 6:22 PM Masami Hiramatsu wrote:
> >
> > On Wed, 24 Feb 2021 11:45:10 -0800
> > Andy Lutomirski wrote:
> >
> > > On Tue, Feb 23, 2021 at 5:18 PM Masami Hiramatsu
> &g
On Wed, 24 Feb 2021 11:45:10 -0800
Andy Lutomirski wrote:
> On Tue, Feb 23, 2021 at 5:18 PM Masami Hiramatsu wrote:
> >
> > On Tue, 23 Feb 2021 15:24:19 -0800
> > Andy Lutomirski wrote:
> >
> > > A while back, I let myself be convinced that kprobes gen
On Tue, 23 Feb 2021 13:45:46 -0600
Josh Poimboeuf wrote:
> On Tue, Feb 23, 2021 at 04:36:19PM +0900, Masami Hiramatsu wrote:
> > On Tue, 23 Feb 2021 10:23:31 +0900
> > Masami Hiramatsu wrote:
> >
> > > On Mon, 22 Feb 2021 11:51:50 -0600
> > > Josh Poimb
l and ret, changes the ip
register (and stack), we have to do a fixup afterwards.
But yes, it is possible to emulate, as same as arm/arm64 does. I just
concern about side-effects of the emulation, need to be carefully
implemented.
Thank you,
--
Masami Hiramatsu
roto_log_invalid (on nf_l4proto_log_invalid in nf_conntrack)
You can now use it in all perf tools, such as:
perf record -e probe:nf_l4proto_log_invalid -aR sleep 1
Reported-by: Evgenii Shatokhin
Signed-off-by: Masami Hiramatsu
---
tools/perf/util/symbol-elf.c |4 +++-
1 file changed
On Tue, 23 Feb 2021 10:23:31 +0900
Masami Hiramatsu wrote:
> On Mon, 22 Feb 2021 11:51:50 -0600
> Josh Poimboeuf wrote:
>
> > On Tue, Feb 23, 2021 at 12:05:08AM +0900, Masami Hiramatsu wrote:
> > > > Of course, one could place probes using absolute addresses of the
Please ignore this. I will send a better fix.
Thanks,
On Tue, 23 Feb 2021 10:48:30 +0900
Masami Hiramatsu wrote:
> If an error occurs on post processing (this converts probe point to
> _text relative address for identifying non-unique symbols) for the
> probes on module, failback
Hi,
On Tue, 23 Feb 2021 00:05:08 +0900
Masami Hiramatsu wrote:
>
> /* Adjust symbol name and address */
> static int post_process_probe_trace_point(struct probe_trace_point *tp,
>struct map *map, unsigned long
> offs)
>
probe can failback to the symbol
based probe.
Signed-off-by: Masami Hiramatsu
---
tools/perf/util/probe-event.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a9cff3a50ddf..af946f68e32e 100644
On Mon, 22 Feb 2021 11:51:50 -0600
Josh Poimboeuf wrote:
> On Tue, Feb 23, 2021 at 12:05:08AM +0900, Masami Hiramatsu wrote:
> > > Of course, one could place probes using absolute addresses of the
> > > functions but that would be less convenient.
> > >
> >
On Tue, 23 Feb 2021 00:05:08 +0900
Masami Hiramatsu wrote:
> Hi Evgenii,
>
> On Thu, 18 Feb 2021 20:09:17 +0300
> Evgenii Shatokhin wrote:
>
> > Hi,
> >
> > It seems, 'perf probe' can only see functions from .text section in the
> > kernel mo
-based" probe. (Are there any way to check the FGKASLR is on?)
The problem of "symbol-based" probe is that local (static) symbols
may share a same name sometimes. In that case, it can not find correct
symbol. (Maybe I can find a candidate from its size.)
Anyway, sometimes the security and usability are trade-off.
Thank you,
--
Masami Hiramatsu
ese were already added. They must have appeared in the thread somewhere,
> > as my internal patchwork picked them up already.
>
> Even better, thank you!
Thank you for fixing and pulling it Steve and Paul!
I couldn't find the original thread, so it is helpful :)
Thanks!
--
Masami Hiramatsu
On Thu, 18 Feb 2021 09:36:36 +0100
Uladzislau Rezki wrote:
> On Thu, Feb 18, 2021 at 02:03:07PM +0900, Masami Hiramatsu wrote:
> > On Wed, 17 Feb 2021 10:17:38 -0800
> > "Paul E. McKenney" wrote:
> >
> > > > > 1.Spawn ksoftirqd earlier.
&
ization is delayed.
Fixes: 36dadef23fcc ("kprobes: Init kprobes in early_initcall")
Reported-by: Paul E. McKenney
Signed-off-by: Masami Hiramatsu
Cc: sta...@vger.kernel.org
---
kernel/kprobes.c | 31 +--
1 file changed, 21 insertions(+), 10 deletions(-)
boot.
Who could I ask for testing this patch, Uladzislau?
I think the test machine which enough slow or the kernel has much initcall
to run optimization thread while booting.
In my environment, I could not reproduce that issue because the optimizer
was sheduled after some tick passed. At that point, ksoftirqd has already
been initialized.
Thank you,
--
Masami Hiramatsu
stop optimizer in early stage
because I just want to enable kprobes in early stage, but not optprobes.
Does the following patch help?
>From e5fafcda3ff918cd52619f795a3f22fb95c72b11 Mon Sep 17 00:00:00 2001
From: Masami Hiramatsu
Date: Wed, 17 Feb 2021 23:35:20 +0900
Subject: [PATCH] kprobes: Fi
/show_bug.cgi?id=98776
>
> Currently arm64 and PA-RISC may enable fpatchable-function-entry option.
> The kernel compiled with clang does not have this issue.
>
> FIX:
>
> This GCC issue only cause the registration failure of the kretprobe event
> which doesn't
gt;
> On Thu, 15 Oct 2020 23:55:25 +0900
> Masami Hiramatsu wrote:
>
> > Add tracefs/options/hash-ptr option to show hashed pointer
> > value by %p in event printk format string.
> >
> > For the security reason, normal printk will show the hashed
> >
gcc.gnu.org/bugzilla/show_bug.cgi?id=98776
>
> Currently arm64 and PA-RISC may enable fpatchable-function-entry option.
> The kernel compiled with clang does not have this issue.
>
> FIX:
>
> This GCC issue only cause the registration failure of the kretprobe event
&
On Tue, 9 Feb 2021 02:51:31 +
Jianlin Lv wrote:
>
>
> > -Original Message-
> > From: Masami Hiramatsu
> > Sent: Monday, February 8, 2021 8:33 PM
> > To: Jianlin Lv
> > Cc: pet...@infradead.org; mi...@redhat.com; a...@kernel.org
; + }
> + }
> + off = noff;
> + }
> + return false;
> +}
> +
> /* Try to find perf_probe_event with debuginfo */
> static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> struct probe_trace_event **tevs)
> @@ -902,6 +956,12 @@ static int try_to_find_probe_trace_events(struct
> perf_probe_event *pev,
> return 0;
> }
>
> + /* workaround for gcc #98776 issue */
> + if (retprobe_gcc_fpatchable_issue_workaround(dinfo, pev) &&
> !need_dwarf) {
> + debuginfo__delete(dinfo);
> + return 0;
> + }
> +
> pr_debug("Try to find probe point from debuginfo.\n");
> /* Searching trace events corresponding to a probe event */
> ntevs = debuginfo__find_trace_events(dinfo, pev, tevs);
> --
> 2.25.1
>
--
Masami Hiramatsu
otection for loop CUs.
>
Thanks for the quick update!
Acked-by: Masami Hiramatsu
Thank you,
> Signed-off-by: Jianlin Lv
> ---
> v2: removed the statement that updates variable in the function argument.
> ---
> tools/perf/util/probe-finder.c | 8 ++--
> 1 file chan
Warn if the kprobe is reregistered, since there must be
a software bug (actively used resource must not be re-registered)
and caller must be fixed.
Signed-off-by: Masami Hiramatsu
---
kernel/kprobes.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/kernel
bes or any other kernel
instrumention does
__preempt_count_add(KEX_OFFSET + NMI_OFFSET + HARDIRQ_OFFSET);
And we can distinguish the KEX from NMI, and get the original status of the
context.
What would you think about?
Thank you,
--
Masami Hiramatsu
pdate it?
Thank you,
> if (dwarf_nextcu(dbg->dbg, off, &noff, &cuhl,
>NULL, NULL, NULL) != 0)
> break;
> @@ -1967,7 +1967,6 @@ int debuginfo__find_line_range(struct debuginfo *dbg,
> struct line_range *lr)
> ret = find_line_range_by_line(NULL, &lf);
> }
> }
> - off = noff;
> }
>
> found:
> --
> 2.25.1
>
--
Masami Hiramatsu
On Fri, 29 Jan 2021 19:08:40 -0800
Alexei Starovoitov wrote:
> On Sat, Jan 30, 2021 at 11:02:49AM +0900, Masami Hiramatsu wrote:
> > On Fri, 29 Jan 2021 18:59:43 +0100
> > Peter Zijlstra wrote:
> >
> > > On Fri, Jan 29, 2021 at 09:45:48AM -0800, Alexei Starovoi
execute bpf prog */
__this_cpu_write(under_running_bpf, false);
}
Does this work for you?
Thank you,
--
Masami Hiramatsu
unsigned int ret;
>
> - if (in_nmi()) /* not supported yet */
> + if (in_nmi() && !kprobe_running()) /* not supported yet */
This doesn't make sense, because kprobe_running() always true in the kprobe
handler.
The problem is that the in_nmi() checks whether the current context is NMI
context,
but you want to know the context where the kprobe is invoked, is NMI context or
not.
Thank you,
--
Masami Hiramatsu
Hi,
On Thu, 28 Jan 2021 10:34:15 +0900
Masami Hiramatsu wrote:
> On Wed, 27 Jan 2021 19:57:56 +0200
> Nikolay Borisov wrote:
>
> >
> >
> > On 27.01.21 г. 17:24 ч., Masami Hiramatsu wrote:
> > > On Thu, 28 Jan 2021 00:13:53 +0900
> > > Masa
On Wed, 27 Jan 2021 19:57:56 +0200
Nikolay Borisov wrote:
>
>
> On 27.01.21 г. 17:24 ч., Masami Hiramatsu wrote:
> > On Thu, 28 Jan 2021 00:13:53 +0900
> > Masami Hiramatsu wrote:
> >
> >> Hi Nikolay,
> >>
> >> On Wed, 27 Jan 2021 15:43:
On Wed, 27 Jan 2021 17:29:50 -0500
Steven Rostedt wrote:
> On Thu, 28 Jan 2021 00:37:51 +0900
> Masami Hiramatsu wrote:
>
> > Fix kprobe_on_func_entry() returns error code instead of false so that
> > register_kretprobe() can return an appropriate error code.
> &g
0 (succeeded, given address is on the entry).
Reported-by: Jianlin Lv
Signed-off-by: Masami Hiramatsu
---
include/linux/kprobes.h |2 +-
kernel/kprobes.c| 34 +-
kernel/trace/trace_kprobe.c | 10 ++
3 files changed, 32 insertions(+), 1
flags |= TPARG_FL_FENTRY;
> - if (offset && is_return && !(flags & TPARG_FL_FENTRY)) {
> + /* Check whether symbol is really bad or from a module */
> + if (!strchr(symbol, ':') && is_return && !(flags &
> TPARG_FL_FENTRY)) {
> trace_probe_log_err(0, BAD_RETPROBE);
> goto parse_error;
> }
> --
> 2.25.1
>
--
Masami Hiramatsu
On Thu, 28 Jan 2021 00:13:53 +0900
Masami Hiramatsu wrote:
> Hi Nikolay,
>
> On Wed, 27 Jan 2021 15:43:29 +0200
> Nikolay Borisov wrote:
>
> > Hello,
> >
> > I'm currently seeing latest Linus' master being somewhat broken w.r.t
> > krpo
vent/trigger
(or echo 1 > options/stacktrace , if trigger file doesn't exist)
Could you also share your kernel config, so that we can reproduce it?
>
> I started bisecting this and arrived at the following commit:
>
> 0d00449c7a28 ("x86: Replace ist_enter() with nmi_enter()")
>
> FWIW the following series is applied on the kernel I was testing:
> https://lore.kernel.org/lkml/159870598914.1229682.15230803449082078353.stgit@devnote2/
>
> but it's still broken.
Peter, would you have any idea?
Thank you,
--
Masami Hiramatsu
On Wed, 27 Jan 2021 02:46:10 +
Jianlin Lv wrote:
>
>
> > -Original Message-
> > From: Masami Hiramatsu
> > Sent: Wednesday, January 27, 2021 10:02 AM
> > To: Oleg Nesterov
> > Cc: Steven Rostedt ; Jianlin Lv ;
> > mi...@redhat.com; li
On Tue, 26 Jan 2021 21:20:59 +0100
Oleg Nesterov wrote:
> On 01/26, Masami Hiramatsu wrote:
> >
> > > >
> > > > IOW, the "offset != 0" check removed by this patch is obviously wrong,
> > > > right?
> > > >
> >
ad the module which provides this symbol.
> >
> > But even if I am right, I agree with the strchr(symbol,":") check.
>
> I see what you are saying. If "MOD" is not loaded yet, the
> kprobe_on_func_entry() should succeed.
No, that makes me more confused. kprobes_on_func_entry() returns true
only if it confirms the given address is on the function entry, because
it is used in the register_kretprobe() too.
OK, I will make a separate check which detects an error that the
module is loaded but the symbol does not exist.
(current code doesn't check the module is loaded or not)
That makes the code clearer.
Thank you,
--
Masami Hiramatsu
> >
> > I was really puzzled until I found the this email from Masami:
> > https://lore.kernel.org/lkml/20210120131406.5a992c1e434681750a0cd...@kernel.org/
> >
> > So I leave this to you and Masami, but perhaps you can document this check
> > at
> > least in the changelog?
> >
>
> No, you are correct. That needs to be documented in the code.
Agreed. There should be commented that is defered until the module is loaded.
>
> I was about to comment that the check requires a comment ;-)
>
> Jianlin,
>
> Care to send a v4 of the patch with a comment to why we are checking the
> symbol for ':'.
Thank you!
>
> Thanks!
>
> -- Steve
>
--
Masami Hiramatsu
e return is -ECANCELED.
The -ECANCELED is not an error, it means "it is not mine, but maybe others."
See create_dyn_event(int argc, char **argv) in kernel/trace/trace_dynevent.c.
Thank you,
--
Masami Hiramatsu
gt; Reverting commit ba090f9cafd5 ("arm64: kprobes: Remove redundant
> kprobe_step_ctx") seemed to fix the problem for me.
>
> Further analysis showed that kcb->kprobe_status is set to
> KPROBE_REENTER when the error occurs. By teaching
> kprobe_breakpoint_ss_handler()
(&cmd, ";");
> + if (!name_and_field) {
> + ret = -EINVAL;
> + goto free;
> + }
> +
> + if (name_and_field[0] == '!')
> + goto free;
> +
> + argv = argv_split(GFP_KERNEL, name_and_field, &argc);
> + if (!argv) {
> + ret = -ENOMEM;
> + goto free;
> + }
> +
> + if (argc < 3)
> + ret = -EINVAL;
> +free:
> + kfree(saved_cmd);
> + if (argv)
> + argv_free(argv);
> +
> + return ret;
> +}
But I'm not sure why this (yet another parser) is needed. What you are expecting
for this check_command()? Could you tell me some examples?
Thank you,
--
Masami Hiramatsu
e synthetic event commands, which is basically an event
> name followed by a set of semicolon-separated fields.
>
> Since we're also now passed the raw command, we can also save it
> directly and can get rid of save_cmdstr().
>
This looks good to me.
Reviewed-by: Masami Hiramat
^
> >
> > Signed-off-by: Jianlin Lv
> >
> > v2:add !strchr(symbol, ':') to check really bad symbol or not.
>
> Also, the "changes since" section should be below the "---" so that they
> don't get pulled into the comm
On Wed, 20 Jan 2021 12:24:15 +0900
Masami Hiramatsu wrote:
> Hi,
>
> On Tue, 19 Jan 2021 10:41:06 -0500
> Steven Rostedt wrote:
>
> > Masami,
> >
> > Looks fine to me. What do you think?
>
> Agreed. Since register_kretprobe() checks the address by
&
Hi,
On Tue, 19 Jan 2021 10:41:06 -0500
Steven Rostedt wrote:
> Masami,
>
> Looks fine to me. What do you think?
Agreed. Since register_kretprobe() checks the address by kprobe_on_func_entry(),
if it is not passed, it should always fail to register at last.
Acked-by: Masami Hiramats
ve
>
>
> On Wed, 9 Dec 2020 14:27:44 +0900
> Masami Hiramatsu wrote:
>
> > Add ftrace.instance.INSTANCE.tracing_on support to ftrace2bconf.sh
> > and bconf2ftrace.sh.
> >
> > commit 8490db06f914 ("tracing/boot: Add per-instance tracing_on
> > o
reusing it. (I recommend to re-alloc new data structure
each time)
For example, if you re-register your driver/filesystem without releasing, it
will
break the kernel.
Thank you,
>
> -- Steve
>
>
> On Tue, 22 Dec 2020 20:03:56 +0900
> Masami Hiramatsu wrote:
>
> &
On Fri, 8 Jan 2021 19:59:50 +0100
Borislav Petkov wrote:
> On Wed, Jan 06, 2021 at 02:21:14PM +0900, Masami Hiramatsu wrote:
> > So I think it is possible to introduce a keyword in a comment
> > for ignoring sync check something like below. This will allow us
> > a gen
ports kprobes on ftrace.
This also changes the dependency of Kconfig. Because kprobe event
uses the function tracer's address list for identifying notrace
function, if the CONFIG_DYNAMIC_FTRACE=n, it can not check whether
the target function is notrace or not.
Signed-off-by: Masami Hiramatsu
On Thu, 7 Jan 2021 09:13:30 -0500
Steven Rostedt wrote:
> On Wed, 6 Jan 2021 12:20:40 +0900
> Masami Hiramatsu wrote:
>
> > Enable the notrace function check on the architecture which doesn't
> > support kprobes on ftrace. This notrace function check is not only
>
-- a/arch/x86/include/asm/inat.h
+++ b/arch/x86/include/asm/inat.h
@@ -6,7 +6,7 @@
*
* Written by Masami Hiramatsu
*/
-#include
+#include /* Different from tools */
/*
* Internal bits. Don't use bitmasks directly, because these bits are
diff --git a/arch/x86/include/asm/ins
This also changes the dependency of Kconfig. Because kprobe event
uses the function tracer's address list for identifying notrace
function, if the CONFIG_FUNCTION_TRACER=n, it can not check whether
the target function is notrace or not.
Signed-off-by: Masami Hiramatsu
Acked-by: Naveen N. Rao
-
The following commit has been merged into the perf/kprobes branch of tip:
Commit-ID: abd82e533d88df1521e3da6799b83ce88852ab88
Gitweb:
https://git.kernel.org/tip/abd82e533d88df1521e3da6799b83ce88852ab88
Author:Masami Hiramatsu
AuthorDate:Fri, 18 Dec 2020 23:12:05 +09:00
On Tue, 05 Jan 2021 15:42:44 +0530
"Naveen N. Rao" wrote:
> Masami Hiramatsu wrote:
> > On Tue, 5 Jan 2021 12:27:30 +0530
> > "Naveen N. Rao" wrote:
> >
> >> Not all symbols are blacklisted on powerpc. Disable multiple_kprobes
> >> t
On Tue, 05 Jan 2021 16:51:50 +0530
"Naveen N. Rao" wrote:
> Masami Hiramatsu wrote:
> > On Tue, 5 Jan 2021 19:01:56 +0900
> > Masami Hiramatsu wrote:
> >
> >> On Tue, 5 Jan 2021 12:27:30 +0530
> >> "Naveen N. Rao" wrote:
>
On Tue, 5 Jan 2021 19:01:56 +0900
Masami Hiramatsu wrote:
> On Tue, 5 Jan 2021 12:27:30 +0530
> "Naveen N. Rao" wrote:
>
> > Not all symbols are blacklisted on powerpc. Disable multiple_kprobes
> > test until that is sorted, so that rest of ftrace and
ppc*) exit_unsupported;;
>*) OFFS=0;;
> esac
>
>
> base-commit: 36bbbd0e234d817938bdc52121a0f5473b3e58f5
> --
> 2.25.4
>
--
Masami Hiramatsu
On Thu, 31 Dec 2020 17:09:23 +0100
Borislav Petkov wrote:
> On Fri, Dec 18, 2020 at 11:12:05PM +0900, Masami Hiramatsu wrote:
> > @@ -467,8 +489,8 @@ static int arch_copy_kprobe(struct kprobe *p)
> > */
> > len = prepare_boost(buf, p, &insn);
> >
ry character, so when I use the
> +0(%x0):string notation, I get only the first character.
>
> Do you know if there's a way around this? maybe use another function in the
> sysfs framework that I could use? I'm debugging a third-party, user-mode *.so
> file.
>
> Best,
> Bar
--
Masami Hiramatsu
On Tue, 29 Dec 2020 21:06:54 +0100
Borislav Petkov wrote:
> On Mon, Dec 28, 2020 at 10:15:10AM +0900, Masami Hiramatsu wrote:
> > BTW, insn_decode() can return -EINVAL if !insn_complete(), is that OK?
>
> It does with this change. Or are you asking whether it returning -EINVAL
&g
On Wed, 23 Dec 2020 18:42:16 +0100
Borislav Petkov wrote:
> From: Borislav Petkov
>
> It wasn't documented so add it. No functional changes.
>
Thank you for fixing!
Acked-by: Masami Hiramatsu
> Signed-off-by: Borislav Petkov
> ---
> arch/x86/lib/insn.c |
or fetching the next insn byte and
> the insn falls short, return -ENODATA to denote that.
-ENODATA sounds good to me :)
Acked-by: Masami Hiramatsu
BTW, insn_decode() can return -EINVAL if !insn_complete(), is that OK?
>
> While at it, make insn_get_opcode() more stricter as to whethe
On Wed, 23 Dec 2020 17:17:27 -0600
Tom Zanussi wrote:
> Hi Masami,
>
> On Tue, 2020-12-22 at 21:42 +0900, Masami Hiramatsu wrote:
> > Hi Tom,
> >
> > On Mon, 21 Dec 2020 15:44:28 -0600
> > Tom Zanussi wrote:
> >
> >
> &
e arguments for this special
> case. Patch 02 is to add argument support for Arm64 SDT.
Both patches look good to me.
Acked-by: Masami Hiramatsu for the seires.
Thank you!
>
> This patch set has been verified on Arm64/x86_64 platforms with a
> testing program usdt_test [1
0;
> + char *name = NULL, *fields, *p;
> + int ret = 0;
>
> - argv = argv_split(GFP_KERNEL, raw_command, &argc);
> - if (!argv)
> - return -ENOMEM;
> + raw_command = skip_spaces(raw_command);
> + if (raw_command[0] == '\0')
> + return ret;
>
> - if (!argc)
> - goto free;
> + last_cmd_set(raw_command);
>
> - name = argv[0];
> + p = strpbrk(raw_command, " \t");
> + if (!p)
> + return -EINVAL;
Hmm, this may drop the ability to delete an event with "!name",
it always requires some spaces after the name.
Thank you,
--
Masami Hiramatsu
bol_name
because it is not allowed (it can lead inconsistent setting).
How about this code? Is this work for you?
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 41fdbb7953c6..73500be564be 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -2103,6 +2103,14 @@ int register_kretprobe(struct kretprobe *rp)
int i;
void *addr;
+ /* It is not allowed to specify addr and symbol_name at the same time */
+ if (rp->kp.addr && rp->kp.symbol_name)
+ return -EINVAL;
+
+ /* If only rp->kp.addr is specified, check reregistering kprobes */
+ if (rp->kp.addr && check_kprobe_rereg(&rp->kp))
+ return -EINVAL;
+
if (!kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name,
rp->kp.offset))
return -EINVAL;
Thank you,
--
Masami Hiramatsu
opcode for resuming process.
Signed-off-by: Masami Hiramatsu
Acked-by: Steven Rostedt (VMware)
---
Changes in v2:
- Fix the timing of memset() for avoiding unexpected
cleanup of kprobe.ainsn.insn.
---
arch/x86/include/asm/kprobes.h | 11 ++-
arch/x86/kernel/kprobes/core.c | 167
ution()")
> url:
> https://github.com/0day-ci/linux/commits/Masami-Hiramatsu/x86-kprobes-Classify-opcode-while-preparing-kprobe/20201206-221324
> base: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git
> 238c91115cd05c71447ea071624a4c9fe661f970
>
> in testcase:
On Thu, 26 Nov 2020 14:26:03 -0300
Arnaldo Carvalho de Melo wrote:
> Em Thu, Nov 26, 2020 at 09:21:25AM +0900, Masami Hiramatsu escreveu:
> > Hi Arnaldo,
> >
> > On Wed, 25 Nov 2020 14:27:55 -0300
> > Arnaldo Carvalho de Melo wrote:
> >
> > >
>
; -- ShaoBo
>
> 在 2020/12/2 7:32, Masami Hiramatsu 写道:
> > On Mon, 30 Nov 2020 16:18:50 -0500
> > Steven Rostedt wrote:
> >
> >> Masami,
> >>
> >> Can you review this patch, and also, should this go to -rc and stable?
> >>
> >>
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 0d07c0ec4381f630c801539c79ad8dcc627f6e4a
Gitweb:
https://git.kernel.org/tip/0d07c0ec4381f630c801539c79ad8dcc627f6e4a
Author:Masami Hiramatsu
AuthorDate:Fri, 11 Dec 2020 16:04:17 +09:00
601 - 700 of 3287 matches
Mail list logo
