Re: [PATCH v2 0/5] ftrace: Clean up and comment code

2024-06-05 Thread Google
n names were not descriptive enough, or at least, they needed > comments. > > This series moves to solve some of that via changing a couple function > names and parameters and adding comments to many of them. > This series looks good to me. Acked-by: Masami Hiramatsu (Google) for this

Re: [PATCH v2 1/5] ftrace: Rename dup_hash() and comment it

2024-06-05 Thread Google
On Wed, 05 Jun 2024 14:03:35 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > The name "dup_hash()" is a misnomer as it does not duplicate the hash that > is passed in, but instead moves its entities from that hash to a newly > alloc

Re: [PATCH 0/6] ftrace: Minor fixes for sparse and kernel test robot

2024-06-05 Thread Google
the warnings. > > But the reports from kernel test robot need to be fixed. All looks good to me. Acked-by: Masami Hiramatsu (Google) Thank you! > > Steven Rostedt (Google) (6): > ftrace: Declare function_trace_op in header to quiet sparse warning > ftrace: Assign ftrace_li

Re: [PATCH 3/5] ftrace: Remove "filter_hash" parameter from ftrace_hash_rec_disable/enable()

2024-06-05 Thread Google
On Tue, 04 Jun 2024 17:28:20 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > The functions ftrace_hash_rec_disable() and ftrace_hash_rec_enable() > always has 1 passed to its "ftrace_hash" parameter. Remove the parameter > and pass

Re: [PATCH 1/5] ftrace: Rename dup_hash() and comment it

2024-06-05 Thread Google
On Tue, 04 Jun 2024 17:28:18 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > The name "dup_hash()" is a misnomer as it does not duplicate the hash that > is passed in, but instead moves its entities from that hash to a newly > alloc

Re: [PATCH v3 00/27] function_graph: Allow multiple users for function graph tracing

2024-06-04 Thread Google
On Tue, 4 Jun 2024 08:18:50 -0400 Steven Rostedt wrote: > > Masami, > > This series passed all my tests, are you comfortable with me pushing > them to linux-next? Yes, this series looks good to me too. Reviewed-by: Masami Hiramatsu (Google) for the series. Thank you!

Re: [PATCH 0/3] tracing: Fix some selftest issues

2024-06-04 Thread Google
On Tue, 4 Jun 2024 09:57:46 -0400 Steven Rostedt wrote: > On Fri, 31 May 2024 23:20:47 +0900 > Masami Hiramatsu (Google) wrote: > > > The major conflict happens when the boot-time test cleans up the kprobe > > events by > > > > dyn_events_release_all(_kp

Re: [PATCH v2 2/4] perf,uprobes: fix user stack traces in the presence of pending uretprobes

2024-06-04 Thread Google
milar approach to what fprobe/kretprobe infrastructure is > doing when capturing kernel stack traces in the presence of pending > return probes. > This looks good to me because this trampoline information is only managed in uprobes. And it should be provided when unwinding user stack.

Re: [PATCH v2 3/4] perf,x86: avoid missing caller address in stack traces captured in uprobe

2024-06-04 Thread Google
next) { > int rc = 0; > > @@ -2086,6 +2087,7 @@ static void handler_chain(struct uprobe *uprobe, struct > pt_regs *regs) > > remove &= rc; > } > + current->utask->auprobe = NULL; > > if (need_prep && !remove) > prepare_uretprobe(uprobe, regs); /* put bp at return */ > -- > 2.43.0 > -- Masami Hiramatsu (Google)

Re: [PATCH v2 2/3] tracing/fprobe: Support raw tracepoint events on modules

2024-06-03 Thread Google
On Mon, 3 Jun 2024 15:50:55 -0400 Mathieu Desnoyers wrote: > On 2024-06-01 04:22, Masami Hiramatsu (Google) wrote: > > From: Masami Hiramatsu (Google) > > > > Support raw tracepoint event on module by fprobe events. > > Since it only uses for_each_kernel_tracep

Re: [PATCH v2 11/27] ftrace: Allow subops filtering to be modified

2024-06-03 Thread Google
On Mon, 3 Jun 2024 10:52:50 -0400 Steven Rostedt wrote: > On Mon, 3 Jun 2024 11:37:23 +0900 > Masami Hiramatsu (Google) wrote: > > > On Sat, 01 Jun 2024 23:37:55 -0400 > > Steven Rostedt wrote: > > > > [...] > > > > > > +static int f

Re: [PATCH v2 24/27] function_graph: Use static_call and branch to optimize entry function

2024-06-03 Thread Google
is enough. Thank you, > > -- Steve -- Masami Hiramatsu (Google)

Re: [PATCH] lib: test_objpool: add missing MODULE_DESCRIPTION() macro

2024-06-03 Thread Google
8d937f8666 > > > > Looks good to me. Thanks for the update. > > I added Masami Hiramatsu and linux-trace in the loop. > > Reviewed-by: Matt Wu Thanks, let me pick this to probes/for-next branch. Thank you, > > Regards, > Matt Wu > -- Masami Hiramatsu (Google)

Re: [PATCH v2 24/27] function_graph: Use static_call and branch to optimize entry function

2024-06-02 Thread Google
On Sat, 01 Jun 2024 23:38:08 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > In most cases function graph is used by a single user. Instead of calling > a loop to call function graph callbacks in this case, call the function > entry callback direct

Re: [PATCH v2 10/27] ftrace: Add subops logic to allow one ops to manage many

2024-06-02 Thread Google
as thinking too much of strcmp() logic :-p Yeah, it's the curse of the C programmer :( (even it is good for sorting.) Thank you, > > > > > Thank you, > > Thanks for the review. > > -- Steve > > > > > > +{ > > > + struct ftrace_func_entry *entry; > > > + int size; > > > + int i; > > > + > > > + if (!A || A == EMPTY_HASH) > > > + return !(!B || B == EMPTY_HASH); > > > + > > > + if (!B || B == EMPTY_HASH) > > > + return !(!A || A == EMPTY_HASH); > > > + > > > + if (A->count != B->count) > > > + return 1; > > > + > > > + size = 1 << A->size_bits; > > > + for (i = 0; i < size; i++) { > > > + hlist_for_each_entry(entry, >buckets[i], hlist) { > > > + if (!__ftrace_lookup_ip(B, entry->ip)) > > > + return 1; > > > + } > > > + } > > > + > > > + return 0; > > > +} > > > + > > > > > -- Masami Hiramatsu (Google)

Re: [PATCH v2 11/27] ftrace: Allow subops filtering to be modified

2024-06-02 Thread Google
p_list, list) { > + if ((subops->flags & FTRACE_OPS_FL_ENABLED) && > + subops->func_hash == ops->func_hash) { > + return > ftrace_hash_move_and_update_subops(subops, orig_hash, hash, enable); > + } > + } > + } while_for_each_ftrace_op(op); > + } > + > old_hash = *orig_hash; > old_hash_ops.filter_hash = ops->func_hash->filter_hash; > old_hash_ops.notrace_hash = ops->func_hash->notrace_hash; > -- > 2.43.0 > > -- Masami Hiramatsu (Google)

Re: [PATCH v2 10/27] ftrace: Add subops logic to allow one ops to manage many

2024-06-02 Thread Google
ps(struct ftrace_hash *A, struct ftrace_hash *B) nit: Isn't it better to be `bool hash_equal()` and return true if A == B ? Thank you, > +{ > + struct ftrace_func_entry *entry; > + int size; > + int i; > + > + if (!A || A == EMPTY_HASH) > + return !(!B || B == EMPTY_HASH); > + > + if (!B || B == EMPTY_HASH) > + return !(!A || A == EMPTY_HASH); > + > + if (A->count != B->count) > + return 1; > + > + size = 1 << A->size_bits; > + for (i = 0; i < size; i++) { > + hlist_for_each_entry(entry, >buckets[i], hlist) { > + if (!__ftrace_lookup_ip(B, entry->ip)) > + return 1; > + } > + } > + > + return 0; > +} > + -- Masami Hiramatsu (Google)

Re: [PATCH RFC 2/4] error-injection: support static keys around injectable functions

2024-06-02 Thread Google
nt get_injectable_error_type(unsigned long addr, struct static_key > **key_addr); This seems like an add-hoc change. Since this is called in a cold path (only used when adding new function), can you add new `struct static_key *get_injection_key(unsigned long addr)` to find the static_key from the address? Other part looks good to me. Thank you, -- Masami Hiramatsu (Google)

Re: [syzbot] Monthly trace report (May 2024)

2024-06-01 Thread Google
.@googlegroups.com. > > To disable reminders for individual bugs, reply with the following command: > #syz set no-reminders > > To change bug's subsystems, reply with: > #syz set subsystems: new-subsystem > > You may send multiple commands in a single email message. -- Masami Hiramatsu (Google)

Re: [PATCH] KUnit: add missing MODULE_DESCRIPTION() macros for lib/test_*.ko

2024-06-01 Thread Google
for *probes"); nit: Could you make it "simple sanity test for k*probes" ? because there is test_fprobe.c. Thank you, -- Masami Hiramatsu (Google)

Re: [PATCH 10/20] function_graph: Have the instances use their own ftrace_ops for filtering

2024-06-01 Thread Google
On Fri, 31 May 2024 18:49:10 -0400 Steven Rostedt wrote: > On Fri, 31 May 2024 23:50:23 +0900 > Masami Hiramatsu (Google) wrote: > > > So is it similar to the fprobe/kprobe, use shared signle ftrace_ops, > > but keep each fgraph has own hash table? > > Sort of. >

Re: [PATCH] samples: kprobes: add missing MODULE_DESCRIPTION() macros

2024-06-01 Thread Google
) > +MODULE_DESCRIPTION("sample kernel module showing the use of return probes"); > MODULE_LICENSE("GPL"); > > --- > base-commit: b050496579632f86ee1ef7e7501906db579f3457 > change-id: 20240601-md-samples-kprobes-efb32cbcc8dd > -- Masami Hiramatsu (Google)

[PATCH v2 3/3] sefltests/tracing: Add a test for tracepoint events on modules

2024-06-01 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Add a test case for tracepoint events on modules. This checks if it can add and remove the events correctly. Signed-off-by: Masami Hiramatsu (Google) --- tools/testing/selftests/ftrace/config |1 + .../test.d/dynevent

[PATCH v2 2/3] tracing/fprobe: Support raw tracepoint events on modules

2024-06-01 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Support raw tracepoint event on module by fprobe events. Since it only uses for_each_kernel_tracepoint() to find a tracepoint, the tracepoints on modules are not handled. Thus if user specified a tracepoint on a module, it shows an error. This adds new

[PATCH v2 1/3] tracepoint: Support iterating over tracepoints on modules

2024-06-01 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Add for_each_module_tracepoint() for iterating over tracepoints on modules. This is similar to the for_each_kernel_tracepoint() but only for the tracepoints on modules (not including kernel built-in tracepoints). Signed-off-by: Masami Hiramatsu (Google

[PATCH v2 0/3] tracing/probes: Support tracepoint events on modules

2024-06-01 Thread Masami Hiramatsu (Google)
Hi, This series implements the tracepoint events on modules. This version separates a patch for tracepoint subsystem from fprobe-event patch, and adds a selftests for tracepoint events on modules. Thank you, --- Masami Hiramatsu (Google) (3): tracepoint: Support iterating over

Re: [PATCH] tracing/fprobe: Support raw tracepoint events on modules

2024-05-31 Thread Google
-lab-lkp/linux/commits/Masami-Hiramatsu-Google/tracing-fprobe-Support-raw-tracepoint-events-on-modules/20240531-175013 > base: linus/master > patch link: > https://lore.kernel.org/r/171714888633.198965.13093663631481169611.stgit%40devnote2 > patch subject: [PATCH] tracing/fprobe: S

Re: [PATCH 10/20] function_graph: Have the instances use their own ftrace_ops for filtering

2024-05-31 Thread Google
On Fri, 31 May 2024 02:03:46 -0400 Steven Rostedt wrote: > On Fri, 31 May 2024 12:12:41 +0900 > Masami Hiramatsu (Google) wrote: > > > On Thu, 30 May 2024 22:30:57 -0400 > > Steven Rostedt wrote: > > > > > On Fri, 24 May 2024 22:37:02 -0400 > > >

Re: [PATCH 0/3] tracing: Fix some selftest issues

2024-05-31 Thread Google
On Fri, 31 May 2024 03:24:25 -0400 Steven Rostedt wrote: > On Fri, 31 May 2024 11:37:21 +0900 > Masami Hiramatsu (Google) wrote: > > > So, in summary, it is designed to be a module. Steve, I think these tests > > should be kept as modules. There are many reason to do so.

[PATCH] tracing/fprobe: Support raw tracepoint events on modules

2024-05-31 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Support raw tracepoint event on module by fprobe events. Since it only uses for_each_kernel_tracepoint() to find a tracepoint, the tracepoints on modules are not handled. Thus if user specified a tracepoint on a module, it shows an error. This adds new

Re: [PATCH 10/20] function_graph: Have the instances use their own ftrace_ops for filtering

2024-05-30 Thread Google
> it avoid pushing on shadow stack multiple times on the same function. > > So I found a major design flaw in this patch. > > > > > Co-developed with Masami Hiramatsu: > > Link: > > https://lore.kernel.org/linux-trace-kernel/171509102088.162236.15758883237657317789.s

Re: [PATCH 0/3] tracing: Fix some selftest issues

2024-05-30 Thread Google
On Wed, 29 May 2024 11:01:43 -0500 Tom Zanussi wrote: > Hi Masami, > > On Wed, 2024-05-29 at 08:38 +0900, Masami Hiramatsu wrote: > > On Wed, 29 May 2024 01:46:40 +0900 > > Masami Hiramatsu (Google) wrote: > > > > > On Mon, 27 May 2024 19:29:

Re: How to trace kvm:kvm_exit using fprobe?

2024-05-30 Thread Google
-cmd: > > *trace-cmd stream -e kvm:kvm_exit* > But if I echo to dynamic_event will get "Invalid argument" error: > # cd /sys/kernel/debug/tracing > > *# echo 't:kvm kvm_exit' >> dynamic_events*-bash: echo: write error: > Invalid argument > > How to solve this problem? > > Thanks, > don -- Masami Hiramatsu (Google)

Re: [PATCH 0/3] tracing: Fix some selftest issues

2024-05-28 Thread Google
On Wed, 29 May 2024 01:46:40 +0900 Masami Hiramatsu (Google) wrote: > On Mon, 27 May 2024 19:29:07 -0400 > Steven Rostedt wrote: > > > On Sun, 26 May 2024 19:10:57 +0900 > > "Masami Hiramatsu (Google)" wrote: > > > > > Hi, > > > > &g

Re: [PATCH 0/3] tracing: Fix some selftest issues

2024-05-28 Thread Google
On Mon, 27 May 2024 19:29:07 -0400 Steven Rostedt wrote: > On Sun, 26 May 2024 19:10:57 +0900 > "Masami Hiramatsu (Google)" wrote: > > > Hi, > > > > Here is a series of some fixes/improvements for the test modules and boot > > time selftest

Re: [PATCH 1/2] objpool: enable inlining objpool_push() and objpool_pop() operations

2024-05-28 Thread Google
graph tracer[1] which will make fprobe not using objpool. And also I'm planning to replace kretprobe with the new fprobe eventually. So if SLUB will use objpool for frontend caching, it sounds good to me. (Maybe it can speed up the object allocation/free) > > > > I'm testing an updated objpool in our HIDS project for critical pathes, > > which is widely deployed on servers inside my company. The new version > > eliminates the raw_local_irq_save and raw_local_irq_restore pair of > > objpool_push and gains up to 5% of performance boost. > > Mind Ccing me and linux-mm once you are posting that? Can you add me too? Thank you, > > Thanks, > Vlastimil > -- Masami Hiramatsu (Google)

Re: [PATCH v2 2/2] selftests/user_events: Add non-spacing separator check

2024-05-27 Thread Google
d not return -EADDRINUSE. > > Add a non-spacing separator case to our self-test register case to ensure > it works going forward. > Looks good to me. Acked-by: Masami Hiramatsu (Google) Thanks! > Signed-off-by: Beau Belgrave > --- > tools/testing/selftests/user_events/ftr

Re: [PATCH v2] tracing/probes: fix error check in parse_btf_field()

2024-05-27 Thread Google
e_probe_log_err(ctx->offset, NO_BTF_FIELD); > return -ENOENT; > -- > 2.35.3 > -- Masami Hiramatsu (Google)

Re: [PATCH 00/20] function_graph: Allow multiple users for function graph tracing

2024-05-26 Thread Google
he patches! I think your changes are good. I just have some comments and replied. So, the plan is to push this series in the tracing/for-next? I will port my fprobe part on it and run some tests. Thank you, > > Based on Linus commit: 0eb03c7e8e2a4cc3653eb5eeb2d2001182071215 > > [1] https://

Re: [PATCH 04/20] function_graph: Allow multiple users to attach to function graph

2024-05-26 Thread Google
nage. > > Co-developed with Masami Hiramatsu: > Link: > https://lore.kernel.org/linux-trace-kernel/171509096221.162236.8806372072523195752.stgit@devnote2 > Thanks for update this. I have some comments below. > Signed-off-by: Steven Rostedt (VMware) > Signed-off-by: Masami Hiram

Re: [PATCH 20/20] function_graph: Use bitmask to loop on fgraph entry

2024-05-26 Thread Google
On Fri, 24 May 2024 22:37:12 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > Instead of looping through all the elements of fgraph_array[] to see if > there's an gops attached to one and then calling its gops->func(). Create > a fgraph_array

Re: [PATCH 19/20] function_graph: Use for_each_set_bit() in __ftrace_return_to_handler()

2024-05-26 Thread Google
On Fri, 24 May 2024 22:37:11 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > Instead of iterating through the entire fgraph_array[] and seeing if one > of the bitmap bits are set to know to call the array's retfunc() function, > use for_each_set_b

Re: [PATCH 19/20] function_graph: Use for_each_set_bit() in __ftrace_return_to_handler()

2024-05-26 Thread Google
On Fri, 24 May 2024 22:37:11 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > Instead of iterating through the entire fgraph_array[] and seeing if one > of the bitmap bits are set to know to call the array's retfunc() function, > use for_each_set_b

Re: [PATCH] ftrace: Fix stack trace entry generated by ftrace_pid_func()

2024-05-26 Thread Google
ACE_OPS_FL_PID) || !ops->private) >107return false; >108 >109tr = ops->private; >110 >111 return tr->function_pids != NULL || > tr->function_no_pids != NULL; >112} >113 > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests/wiki -- Masami Hiramatsu (Google)

Re: [PATCH] tracing/probes: fix error check in parse_btf_field()

2024-05-26 Thread Google
On Sun, 26 May 2024 14:27:56 +0200 Carlos López wrote: > > Hi, > > On 26/5/24 12:17, Masami Hiramatsu (Google) wrote: > > On Sat, 25 May 2024 20:21:32 +0200 > > Carlos López wrote: > > > >> btf_find_struct_member() might return NULL or an error via

Re: [PATCH] tracing/probes: fix error check in parse_btf_field()

2024-05-26 Thread Google
_FIELD); > return -ENOENT; > -- > 2.35.3 > -- Masami Hiramatsu (Google)

[PATCH 3/3] tracing/kprobe: Remove cleanup code unrelated to selftest

2024-05-26 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) This cleanup all kprobe events code is not related to the selftest itself, and it can fail by the reason unrelated to this test. If the test is successful, the generated events are cleaned up. And if not, we cannot guarantee that the kprobe events will work

[PATCH 2/3] tracing/kprobe: Remove unneeded WARN_ON_ONCE() in selftests

2024-05-26 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Since the kprobe-events selftest shows OK or NG with the reason, the WARN_ON_ONCE()s for each place are redundant. Let's remove it. Signed-off-by: Masami Hiramatsu (Google) --- kernel/trace/trace_kprobe.c | 26 +- 1 file changed, 13

[PATCH 1/3] tracing: Build event generation tests only as modules

2024-05-26 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Since the kprobes and synth event generation tests adds and enable generated events in init_module() and delete it in exit_module(), if we make it as built-in, those events are left in kernel and cause kprobe event self-test failure. [ 97.349708

[PATCH 0/3] tracing: Fix some selftest issues

2024-05-26 Thread Masami Hiramatsu (Google)
the main problem is that the test module should not be built-in. But I also think this WARNING message is useless (because there are warning messages already) and the cleanup code is redundant. This series fixes those issues. Thank you, --- Masami Hiramatsu (Google) (3): tracing: Build event

Re: [PATCH v10 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph

2024-05-25 Thread Google
On Fri, 24 May 2024 18:41:56 -0400 Steven Rostedt wrote: > On Tue, 7 May 2024 23:08:00 +0900 > "Masami Hiramatsu (Google)" wrote: > > > Steven Rostedt (VMware) (15): > > function_graph: Convert ret_stack to a series of longs > > fgraph: Us

Re: [PATCH v10 07/36] function_graph: Allow multiple users to attach to function graph

2024-05-25 Thread Google
On Fri, 24 May 2024 21:32:08 -0400 Steven Rostedt wrote: > On Tue, 7 May 2024 23:09:22 +0900 > "Masami Hiramatsu (Google)" wrote: > > > @@ -109,6 +244,21 @@ ftrace_push_return_trace(unsigned long ret, unsigned > > long func, > > if (!current->

Re: [PATCH v10 03/36] x86: tracing: Add ftrace_regs definition in the header

2024-05-23 Thread Google
On Thu, 23 May 2024 19:14:59 -0400 Steven Rostedt wrote: > On Tue, 7 May 2024 23:08:35 +0900 > "Masami Hiramatsu (Google)" wrote: > > > From: Masami Hiramatsu (Google) > > > > Add ftrace_regs definition for x86_64 in the ftrace header to > > cl

Re: [PATCH v10 01/36] tracing: Add a comment about ftrace_regs definition

2024-05-23 Thread Google
On Thu, 23 May 2024 19:10:31 -0400 Steven Rostedt wrote: > On Tue, 7 May 2024 23:08:12 +0900 > "Masami Hiramatsu (Google)" wrote: > > > From: Masami Hiramatsu (Google) > > > > To clarify what will be expected on ftrace_regs, add a comment to the &g

Re: [PATCH] uprobes: prevent mutex_lock() under rcu_read_lock()

2024-05-23 Thread Google
ong func, >struct uprobe_cpu_buffer **ucbp) > { > struct event_file_link *link; > + struct uprobe_cpu_buffer *ucb; > + > + ucb = prepare_uprobe_buffer(tu, regs, ucbp); > > rcu_read_lock(); > trace_probe_for_each_link_rcu(link, >tp) > - __uprobe_trace_func(tu, func, regs, ucbp, link->file); > + __uprobe_trace_func(tu, func, regs, ucb, link->file); > rcu_read_unlock(); > } > > -- > 2.43.0 > -- Masami Hiramatsu (Google)

Re: [PATCH] kernel: trace: preemptirq_delay_test: add MODULE_DESCRIPTION()

2024-05-18 Thread Google
On Sat, 18 May 2024 15:54:49 -0700 Jeff Johnson wrote: > Fix the 'make W=1' warning: > > WARNING: modpost: missing MODULE_DESCRIPTION() in > kernel/trace/preemptirq_delay_test.o > Looks good to me. Acked-by: Masami Hiramatsu (Google) Fixes: f96e8577da10 ("lib: Ad

Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

2024-05-13 Thread Google
r debugging. But I think that needs another series of patches. We also need to discuss when it should be prohibited and how (e.g. audit interface? SELinux?). But I think this series is just optimizing currently available uprobes with a new syscall. I don't think it changes such security concerning. Thank you, > > jirka -- Masami Hiramatsu (Google)

Re: [PATCHv5 bpf-next 7/8] selftests/x86: Add return uprobe shadow stack test

2024-05-13 Thread Google
> + uretprobe_trigger(); > + > + printf("[OK]\tUretprobe test\n"); > + err = 0; > + > +out: > + ARCH_PRCTL(ARCH_SHSTK_DISABLE, ARCH_SHSTK_SHSTK); > + signal(SIGSEGV, SIG_DFL); > + if (fd) > + close(fd); > + return err; > +} > + > void segv_handler_ptrace(int signum, siginfo_t *si, void *uc) > { > /* The SSP adjustment caused a segfault. */ > @@ -867,6 +1003,12 @@ int main(int argc, char *argv[]) > goto out; > } > > + if (test_uretprobe()) { > + ret = 1; > + printf("[FAIL]\turetprobe test\n"); > + goto out; > + } > + > return ret; > > out: > -- > 2.44.0 > -- Masami Hiramatsu (Google)

Re: [PATCH RESEND v8 07/16] mm/execmem, arch: convert simple overrides of module_alloc to execmem

2024-05-07 Thread Google
e does > not implement execmem_arch_setup(), execmem_alloc() will fall back to > module_alloc(). > Looks good to me. Reviewed-by: Masami Hiramatsu (Google) Thanks, > Signed-off-by: Mike Rapoport (IBM) > Acked-by: Song Liu > --- > arch/loongarch/kernel/module.c | 19 ++

Re: [PATCH RESEND v8 05/16] module: make module_memory_{alloc,free} more self-contained

2024-05-07 Thread Google
On Sun, 5 May 2024 19:06:17 +0300 Mike Rapoport wrote: > From: "Mike Rapoport (IBM)" > > Move the logic related to the memory allocation and freeing into > module_memory_alloc() and module_memory_free(). > Looks good to me. Reviewed-by: Masami Hiramatsu (Google

[PATCH v10 36/36] fgraph: Skip recording calltime/rettime if it is not nneeded

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Skip recording calltime and rettime if the fgraph_ops does not need it. This is a kind of performance optimization for fprobe. Since the fprobe user does not use these entries, recording timestamp in fgraph is just a overhead (e.g. eBPF, ftrace). So introduce

[PATCH v10 35/36] Documentation: probes: Update fprobe on function-graph tracer

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Update fprobe documentation for the new fprobe on function-graph tracer. This includes some bahvior changes and pt_regs to ftrace_regs interface change. Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Update @fregs parameter explanation

[PATCH v10 17/36] function_graph: Move graph notrace bit to shadow stack global var

2024-05-07 Thread Masami Hiramatsu (Google)
are) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Make description lines shorter than 76 chars. --- include/linux/trace_recursion.h |7 --- kernel/trace/trace.h |9 + kernel/trace/trace_functions_graph.c | 10 ++ 3 files chan

[PATCH v10 34/36] selftests/ftrace: Add a test case for repeating register/unregister fprobe

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) This test case repeats define and undefine the fprobe dynamic event to ensure that the fprobe does not cause any issue with such operations. Signed-off-by: Masami Hiramatsu (Google) --- .../test.d/dynevent/add_remove_fprobe_repeat.tc| 19

[PATCH v10 33/36] selftests: ftrace: Remove obsolate maxactive syntax check

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Since the fprobe event does not support maxactive anymore, stop testing the maxactive syntax error checking. Signed-off-by: Masami Hiramatsu (Google) --- .../ftrace/test.d/dynevent/fprobe_syntax_errors.tc |4 +--- 1 file changed, 1 insertion(+), 3 deletions

[PATCH v10 32/36] tracing/fprobe: Remove nr_maxactive from fprobe

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Remove depercated fprobe::nr_maxactive. This involves fprobe events to rejects the maxactive number. Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Newly added. --- include/linux/fprobe.h |2 -- kernel/trace/trace_fprobe.c | 44

[PATCH v10 31/36] fprobe: Rewrite fprobe on function-graph tracer

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Rewrite fprobe implementation on function-graph tracer. Major API changes are: - 'nr_maxactive' field is deprecated. - This depends on CONFIG_DYNAMIC_FTRACE_WITH_ARGS or !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS, and CONFIG_HAVE_FUNCTION_GRAPH_FREGS. So

[PATCH v10 30/36] ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Add CONFIG_HAVE_FTRACE_GRAPH_FUNC kconfig in addition to ftrace_graph_func macro check. This is for the other feature (e.g. FPROBE) which requires to access ftrace_regs from fgraph_ops::entryfunc() can avoid compiling if the fgraph can not pass the valid

[PATCH v10 29/36] bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Enable kprobe_multi feature if CONFIG_FPROBE is enabled. The pt_regs is converted from ftrace_regs by ftrace_partial_regs(), thus some registers may always returns 0. But it should be enough for function entry (access arguments) and exit (access return value

[PATCH v10 28/36] tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Allow fprobe events to be enabled with CONFIG_DYNAMIC_FTRACE_WITH_ARGS. With this change, fprobe events mostly use ftrace_regs instead of pt_regs. Note that if the arch doesn't enable HAVE_PT_REGS_COMPAT_FTRACE_REGS, fprobe events will not be able to be used from

[PATCH v10 27/36] tracing: Add ftrace_fill_perf_regs() for perf event

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Add ftrace_fill_perf_regs() which should be compatible with the perf_fetch_caller_regs(). In other words, the pt_regs returned from the ftrace_fill_perf_regs() must satisfy 'user_mode(regs) == false' and can be used for stack tracing. Signed-off-by: Masami

[PATCH v10 26/36] tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Add ftrace_partial_regs() which converts the ftrace_regs to pt_regs. This is for the eBPF which needs this to keep the same pt_regs interface to access registers. Thus when replacing the pt_regs with ftrace_regs in fprobes (which is used by kprobe_multi eBPF event

[PATCH v10 25/36] fprobe: Use ftrace_regs in fprobe exit handler

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Change the fprobe exit handler to use ftrace_regs structure instead of pt_regs. This also introduce HAVE_PT_REGS_TO_FTRACE_REGS_CAST which means the ftrace_regs's memory layout is equal to the pt_regs so that those are able to cast. Fprobe introduces a new

[PATCH v10 24/36] fprobe: Use ftrace_regs in fprobe entry handler

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) This allows fprobes to be available with CONFIG_DYNAMIC_FTRACE_WITH_ARGS instead of CONFIG_DYNAMIC_FTRACE_WITH_REGS, then we can enable fprobe on arm64. Signed-off-by: Masami Hiramatsu (Google) Acked-by: Florent Revest --- Changes in v6: - Keep using

[PATCH v10 23/36] function_graph: Pass ftrace_regs to retfunc

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Pass ftrace_regs to the fgraph_ops::retfunc(). If ftrace_regs is not available, it passes a NULL instead. User callback function can access some registers (including return address) via this ftrace_regs. Signed-off-by: Masami Hiramatsu (Google) --- Changes

[PATCH v10 22/36] function_graph: Replace fgraph_ret_regs with ftrace_regs

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Use ftrace_regs instead of fgraph_ret_regs for tracing return value on function_graph tracer because of simplifying the callback interface. The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by CONFIG_HAVE_FUNCTION_GRAPH_FREGS. Signed-off-by: Masami

[PATCH v10 21/36] function_graph: Pass ftrace_regs to entryfunc

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Pass ftrace_regs to the fgraph_ops::entryfunc(). If ftrace_regs is not available, it passes a NULL instead. User callback function can access some registers (including return address) via this ftrace_regs. Signed-off-by: Masami Hiramatsu (Google) --- Changes

[PATCH v10 20/36] ftrace: Add multiple fgraph storage selftest

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Add a selftest for multiple function graph tracer with storage on a same function. In this case, the shadow stack entry will be shared among those fgraph with different data storage. So this will ensure the fgraph will not mixed those storage data. Signed-off

[PATCH v10 19/36] function_graph: Add selftest for passing local variables

2024-05-07 Thread Masami Hiramatsu (Google)
From: Steven Rostedt (VMware) Add boot up selftest that passes variables from a function entry to a function exit, and make sure that they do get passed around. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Add reserved size test

[PATCH v10 04/36] function_graph: Convert ret_stack to a series of longs

2024-05-07 Thread Masami Hiramatsu (Google)
for the return side of the functions. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/sched.h |2 - kernel/trace/fgraph.c | 124 - 2 files changed, 71 insertions(+), 55 deletions(-) diff --git

[PATCH v10 18/36] function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data()

2024-05-07 Thread Masami Hiramatsu (Google)
shadow ret_stack and this then can be retrieved by fgraph_retrieve_data() called by the corresponding retfunc(). Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v10: - Fix to support data size up to 32 words (previously it only support up

[PATCH v10 16/36] function_graph: Move graph depth stored data to shadow stack global var

2024-05-07 Thread Masami Hiramatsu (Google)
are) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/trace_recursion.h | 29 - kernel/trace/trace.h| 34 -- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/include/linux/trace_recursion.h b/incl

[PATCH v10 15/36] function_graph: Move set_graph_function tests to shadow stack global var

2024-05-07 Thread Masami Hiramatsu (Google)
are) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/trace_recursion.h |5 + kernel/trace/trace.h | 32 +--- kernel/trace/trace_functions_graph.c |6 +++--- kernel/trace/trace_irqsoff.c |4 ++-- kernel/tr

[PATCH v10 07/36] function_graph: Allow multiple users to attach to function graph

2024-05-07 Thread Masami Hiramatsu (Google)
are called is not completely handled yet, but that shouldn't be too hard to manage. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v10: - Rephrase all "index" on shadow stack to "offset" and some "ret_stack" to &

[PATCH v10 14/36] function_graph: Add "task variables" per task for fgraph_ops

2024-05-07 Thread Masami Hiramatsu (Google)
s on a per task basis having a way to maintain state for each task. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v10: - Explain where the task vars is placed in shadow stack. Changes in v3: - Move fgraph_ops::idx to previous patch in

[PATCH v10 13/36] function_graph: Use a simple LRU for fgraph_array index number

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Since the fgraph_array index is used for the bitmap on the shadow stack, it may leave some entries after a function_graph instance is removed. Thus if another instance reuses the fgraph_array index soon after releasing it, the fgraph may confuse to call the newer

[PATCH v10 12/36] function_graph: Have the instances use their own ftrace_ops for filtering

2024-05-07 Thread Masami Hiramatsu (Google)
function_graph_enter_ops() instead of function_graph_enter() so that it avoid pushing on shadow stack multiple times on the same function. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v10: - Use "offset" for shadow stack instead of "i

[PATCH v10 11/36] ftrace: Allow ftrace startup flags exist without dynamic ftrace

2024-05-07 Thread Masami Hiramatsu (Google)
to compile because dynamic ftrace is disabled. This change is needed to move some of the logic of what is passed to ftrace_startup() out of the parameters of ftrace_startup(). Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/ftrace.h | 18

[PATCH v10 10/36] ftrace: Allow function_graph tracer to be enabled in instances

2024-05-07 Thread Masami Hiramatsu (Google)
-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Fix to remove set_graph_array() completely. --- include/linux/ftrace.h |1 + kernel/trace/ftrace.c|1 + kernel/trace/trace.h | 13 ++- kernel

[PATCH v10 09/36] ftrace/function_graph: Pass fgraph_ops to function graph callbacks

2024-05-07 Thread Masami Hiramatsu (Google)
to the function graph tracer. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - cleanup to set argument name on function prototype. --- include/linux/ftrace.h | 10 +++--- kernel/trace/fgraph.c| 16

[PATCH v10 08/36] function_graph: Remove logic around ftrace_graph_entry and return

2024-05-07 Thread Masami Hiramatsu (Google)
into the function_graph tracer. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Fix typo and make lines shorter than 76 chars in the description. - Remove unneeded return from return_run() function. --- kernel/trace/fgraph.c | 67

[PATCH v10 06/36] function_graph: Add an array structure that will allow multiple callbacks

2024-05-07 Thread Masami Hiramatsu (Google)
on the shadow stack. We need to only save the index, because this will allow the fgraph_ops to be freed before the function returns (which may happen if the function call schedule for a long time). Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Remove

[PATCH v10 05/36] fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long

2024-05-07 Thread Masami Hiramatsu (Google)
dt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v7: - Use DIV_ROUND_UP() to calculate FGRAPH_RET_INDEX --- kernel/trace/fgraph.c |9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c index 30

[PATCH v10 02/36] tracing: Rename ftrace_regs_return_value to ftrace_regs_get_return_value

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Rename ftrace_regs_return_value to ftrace_regs_get_return_value as same as other ftrace_regs_get/set_* APIs. Signed-off-by: Masami Hiramatsu (Google) Acked-by: Mark Rutland --- Changes in v6: - Moved to top of the series. Changes in v3: - Newly added

[PATCH v10 03/36] x86: tracing: Add ftrace_regs definition in the header

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Add ftrace_regs definition for x86_64 in the ftrace header to clarify what register will be accessible from ftrace_regs. Signed-off-by: Masami Hiramatsu (Google) --- Changes in v3: - Add rip to be saved. Changes in v2: - Newly added. --- arch/x86/include

[PATCH v10 01/36] tracing: Add a comment about ftrace_regs definition

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) To clarify what will be expected on ftrace_regs, add a comment to the architecture independent definition of the ftrace_regs. Signed-off-by: Masami Hiramatsu (Google) Acked-by: Mark Rutland --- Changes in v8: - Update that the saved registers depends

[PATCH v10 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph

2024-05-07 Thread Masami Hiramatsu (Google)
ck. This series can be applied against the probes/for-next branch, which is based on v6.9-rc6. This series can also be found below branch. https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/log/?h=topic/fprobe-on-fgraph Thank you, --- Masami Hiramatsu (Google) (21): tracing:

Re: [PATCH resend ftrace] Asynchronous grace period for register_ftrace_direct()

2024-05-01 Thread Google
hen adding a new direct trampoline. This looks good to me. Reviewed-by: Masami Hiramatsu (Google) Thank you, > [1] https://lore.kernel.org/all/cover.1710877680.git@cloudflare.com/ > > Reported-by: Jakub Kicinski > Reported-by: Alexei Starovoitov > Reported-by: Chris Mason > S

Re: [v3] tracing/probes: Fix memory leak in traceprobe_parse_probe_arg_body()

2024-05-01 Thread Google
ke to read any improved (patch) version descriptions (or > changelogs)? Thanks, but those are nitpicks and I don't mind it. Thank you, > > Regards, > Markus -- Masami Hiramatsu (Google)

Re: [PATCH] eventfs/tracing: Add callback for release of an eventfs_inode

2024-05-01 Thread Google
On Tue, 30 Apr 2024 14:23:27 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > Synthetic events create and destroy tracefs files when they are created > and removed. The tracing subsystem has its own file descriptor > representing the state of the event

  1   2   3   4   5   6   7   8   9   10   >