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

2024-05-02 Thread Steven Rostedt
On Thu, 2 May 2024 16:13:59 -0700 "Paul E. McKenney" wrote: > Very good, and thank you! > > I will drop it from RCU as soon as it shows up in either -next or in > mainline. Sounds good. I'm currently working on updates to get into -rc7 and plan to add my next work on top of that (I know, I

Re: [PATCH v2 1/2] tracing/user_events: Fix non-spaced field matching

2024-05-02 Thread Steven Rostedt
On Thu, 2 May 2024 15:58:53 -0700 Beau Belgrave wrote: > It's not an issue on the matching/logic. However, you do get an extra > byte alloc (which doesn't bother me in this edge case). Figured as much, but since there was no mention of it, I decided to bring it up. I'll take this as-is then.

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

2024-05-02 Thread Steven Rostedt
> > 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 > > >

Re: [PATCH v2 1/2] tracing/user_events: Fix non-spaced field matching

2024-05-02 Thread Steven Rostedt
On Tue, 23 Apr 2024 16:23:37 + Beau Belgrave wrote: > When the ABI was updated to prevent same name w/different args, it > missed an important corner case when fields don't end with a space. > Typically, space is used for fields to help separate them, like > "u8 field1; u8 field2". If no

Re: [PATCH v3] ftrace: Fix possible use-after-free issue in ftrace_location()

2024-05-02 Thread Steven Rostedt
On Wed, 17 Apr 2024 11:28:30 +0800 Zheng Yejian wrote: > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index da1710499698..e05d3e3dc06a 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -1581,7 +1581,7 @@ static struct dyn_ftrace *lookup_rec(unsigned long >

[PATCH v3 6/6] eventfs: Have "events" directory get permissions from its parent

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The events directory gets its permissions from the root inode. But this can cause an inconsistency if the instances directory changes its permissions, as the permissions of the created directories under it should inherit the permissions of the instances

[PATCH v3 5/6] eventfs: Do not treat events directory different than other directories

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Treat the events directory the same as other directories when it comes to permissions. The events directory was considered different because it's dentry is persistent, whereas the other directory dentries are created when accessed. But the way tracefs no

[PATCH v3 2/6] tracefs: Reset permissions on remount if permissions are options

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" There's an inconsistency with the way permissions are handled in tracefs. Because the permissions are generated when accessed, they default to the root inode's permission if they were never set by the user. If the user sets the permissions, then a f

[PATCH v3 4/6] eventfs: Do not differentiate the toplevel events directory

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The toplevel events directory is really no different than the events directory of instances. Having the two be different caused inconsistencies and made it harder to fix the permissions bugs. Make all events directories act the same. Cc: sta...@vger.

[PATCH v3 1/6] eventfs: Free all of the eventfs_inode after RCU

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The freeing of eventfs_inode via a kfree_rcu() callback. But the content of the eventfs_inode was being freed after the last kref. This is dangerous, as changes are being made that can access the content of an eventfs_inode from an RCU loop. Instea

[PATCH v3 3/6] tracefs: Still use mount point as default permissions for instances

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" If the instances directory's permissions were never change, then have it and its children use the mount point permissions as the default. Currently, the permissions of instance directories are determined by the instance directory's permissi

[PATCH v3 0/6] tracefs/eventfs: Fix inconsistent permissions

2024-05-02 Thread Steven Rostedt
that the iteration of the list only needs to be protected by rcu_read_lock(). Steven Rostedt (Google) (6): eventfs: Free all of the eventfs_inode after RCU tracefs: Reset permissions on remount if permissions are options tracefs: Still use mount point as default permissions for instances

Re: [PATCH v2 1/5] tracefs: Reset permissions on remount if permissions are options

2024-05-02 Thread Steven Rostedt
On Thu, 02 May 2024 11:15:48 -0400 Steven Rostedt wrote: > +/* > + * On a remount of tracefs, if UID or GID options are set, then > + * the mount point inode permissions should be used. > + * Reset the saved permission flags appropriately. > + */ > +void eventfs_remount(struct

[PATCH v2 5/5] eventfs: Have "events" directory get permissions from its parent

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The events directory gets its permissions from the root inode. But this can cause an inconsistency if the instances directory changes its permissions, as the permissions of the created directories under it should inherit the permissions of the instances

[PATCH v2 4/5] eventfs: Do not treat events directory different than other directories

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Treat the events directory the same as other directories when it comes to permissions. The events directory was considered different because it's dentry is persistent, whereas the other directory dentries are created when accessed. But the way tracefs no

[PATCH v2 0/5] tracefs/eventfs: Fix inconsistent permissions

2024-05-02 Thread Steven Rostedt
the node from the link list, and free the node via call_rcu() so that the iteration of the list only needs to be protected by rcu_read_lock(). Steven Rostedt (Google) (5): tracefs: Reset permissions on remount if permissions are options tracefs: Still use mount point as default

[PATCH v2 2/5] tracefs: Still use mount point as default permissions for instances

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" If the instances directory's permissions were never change, then have it and its children use the mount point permissions as the default. Currently, the permissions of instance directories are determined by the instance directory's permissi

[PATCH v2 3/5] eventfs: Do not differentiate the toplevel events directory

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The toplevel events directory is really no different than the events directory of instances. Having the two be different caused inconsistencies and made it harder to fix the permissions bugs. Make all events directories act the same. Cc: sta...@vger.

[PATCH v2 1/5] tracefs: Reset permissions on remount if permissions are options

2024-05-02 Thread Steven Rostedt
From: "Steven Rostedt (Google)" There's an inconsistency with the way permissions are handled in tracefs. Because the permissions are generated when accessed, they default to the root inode's permission if they were never set by the user. If the user sets the permissions, then a f

Re: [PATCH] tracing: Fix uaf issue in tracing_open_file_tr

2024-05-02 Thread Steven Rostedt
On Thu, 2 May 2024 06:49:18 + Tze-nan Wu (吳澤南) wrote: > Good news, this patch works, the test has passed, no more Kasan report > in my environment. Great to hear! > > my environment: > arm64 + kasan + swtag based kasan + kernel-6.6.18 > > Really appreciate, and learn a lot from the

Re: [PATCH v22 2/5] ring-buffer: Introducing ring-buffer mapping functions

2024-05-02 Thread Steven Rostedt
On Thu, 2 May 2024 14:38:32 +0100 Vincent Donnefort wrote: > > > + while (s < nr_subbufs && p < nr_pages) { > > > + struct page *page = virt_to_page(cpu_buffer->subbuf_ids[s]); > > > + int off = 0; > > > + > > > + for (; off < (1 << (subbuf_order)); off++, page++) { > > >

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

2024-05-02 Thread Steven Rostedt
On Wed, 1 May 2024 23:56:26 +0900 Masami Hiramatsu (Google) wrote: > Looks good to me. > > Reviewed-by: Masami Hiramatsu (Google) Thanks Masami, Although Tze-nan pointed out a issue with this patch. I just published v2, can you review that one too? Thanks, -- Steve

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

2024-05-02 Thread Steven Rostedt
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 events attached to the tracefs files. There's a race between the eventfs files and

Re: [PATCH] tracing: Fix uaf issue in tracing_open_file_tr

2024-05-01 Thread Steven Rostedt
On Thu, 2 May 2024 03:10:24 + Tze-nan Wu (吳澤南) wrote: > > > Sorry for my late reply, I'm testing the patch on my machine now. > Test will be done in four hours. > > There's something I'm worrying about in the patch, > what I'm worrying about is commented in the code below. > >

[PATCH 5/5] eventfs: Have "events" directory get permissions from its parent

2024-05-01 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The events directory gets its permissions from the root inode. But this can cause an inconsistency if the instances directory changes its permissions, as the permissions of the created directories under it should inherit the permissions of the instances

[PATCH 4/5] eventfs: Do not treat events directory different than other directories

2024-05-01 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Treat the events directory the same as other directories when it comes to permissions. The events directory was considered different because it's dentry is persistent, whereas the other directory dentries are created when accessed. But the way tracefs no

[PATCH 3/5] eventfs: Do not differentiate the toplevel events directory

2024-05-01 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The toplevel events directory is really no different than the events directory of instances. Having the two be different caused inconsistencies and made it harder to fix the permissions bugs. Make all events directories act the same. Cc: sta...@vger.

[PATCH 2/5] tracefs: Still use mount point as default permissions for instances

2024-05-01 Thread Steven Rostedt
From: "Steven Rostedt (Google)" If the instances directory's permissions were never change, then have it and its children use the mount point permissions as the default. Currently, the permissions of instance directories are determined by the instance directory's permissi

[PATCH 0/5] tracefs/eventfs: Fix inconsistent permissions

2024-05-01 Thread Steven Rostedt
. Steven Rostedt (Google) (5): tracefs: Reset permissions on remount if permissions are options tracefs: Still use mount point as default permissions for instances eventfs: Do not differentiate the toplevel events directory eventfs: Do not treat events directory different than

[PATCH 1/5] tracefs: Reset permissions on remount if permissions are options

2024-05-01 Thread Steven Rostedt
From: "Steven Rostedt (Google)" There's an inconsistency with the way permissions are handled in tracefs. Because the permissions are generated when accessed, they default to the root inode's permission if they were never set by the user. If the user sets the permissions, then a f

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

2024-04-30 Thread Steven Rostedt
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 events attached to the tracefs files. There's a race between the eventfs files and

Re: [PATCH] tracing: Fix uaf issue in tracing_open_file_tr

2024-04-29 Thread Steven Rostedt
On Sun, 28 Apr 2024 20:28:37 -0400 Steven Rostedt wrote: > > Looking for any suggestion or solution, appreciate. > > Yeah, I do not think eventfs should be involved in this. It needs to be > protected at a higher level (in the synthetic/dynamic event code). > > I'm

Re: [PATCH] tracing: Fix uaf issue in tracing_open_file_tr

2024-04-28 Thread Steven Rostedt
On Fri, 26 Apr 2024 15:34:08 +0800 Tze-nan wu wrote: > "tracing_event_file" is at the risk of use-after-free due to the race of > two functions "tracing_open_file_tr" and "synth_event_release". > Specifically, it could be freed by synth_event_release before > tracing_open_file_tr has the

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

2024-04-28 Thread Steven Rostedt
On Thu, 25 Apr 2024 13:31:53 -0700 Andrii Nakryiko wrote: I'm just coming back from Japan (work and then a vacation), and catching up on my email during the 6 hour layover in Detroit. > Hey Masami, > > I can't really review most of that code as I'm completely unfamiliar > with all those inner

Re: [PATCH v20 2/5] ring-buffer: Introducing ring-buffer mapping functions

2024-04-28 Thread Steven Rostedt
On Tue, 23 Apr 2024 12:04:15 -0400 "Liam R. Howlett" wrote: > > Nit: For all labels, please add a space before them. Otherwise, diffs will > > show "unlock" as the function and not "ring_buffer_map", making it harder > > to find where the change is. > > > > Isn't the inclusion of a space

Re: [PATCH] ftrace: Replace ftrace_disabled variable with ftrace_is_dead function

2024-04-27 Thread Steven Rostedt
On Sat, 20 Apr 2024 11:50:29 +0800 "Bang Li" wrote: > Thank you for your explanation, let me understand this. How about > replacing ftrace_disabled with unlike(ftrace_disabled)? Why? They are slow paths. No need to optimize them. -- Steve

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

2024-04-19 Thread Steven Rostedt
On Mon, 15 Apr 2024 21:50:20 +0900 "Masami Hiramatsu (Google)" wrote: > @@ -27,23 +28,157 @@ > > #define FGRAPH_RET_SIZE sizeof(struct ftrace_ret_stack) > #define FGRAPH_RET_INDEX DIV_ROUND_UP(FGRAPH_RET_SIZE, sizeof(long)) > + > +/* > + * On entry to a function (via function_graph_enter()),

Re: [PATCH net-next v6 0/7] Implement reset reason mechanism to detect

2024-04-19 Thread Steven Rostedt
On Fri, 19 Apr 2024 16:00:20 +0800 Jason Xing wrote: > If other experts see this thread, please help me. I would appreciate > it. I have strong interests and feel strong responsibility to > implement something like this patch series. It can be very useful!! I'm not a networking expert, but as

Re: [PATCH] ftrace: Replace ftrace_disabled variable with ftrace_is_dead function

2024-04-19 Thread Steven Rostedt
On Fri, 19 Apr 2024 22:38:44 +0800 "Bang Li" wrote: > Use the existing function ftrace_is_dead to replace the variable to make > the code clearer. > > Signed-off-by: Bang Li > --- > kernel/trace/ftrace.c | 46 +-- > 1 file changed, 23 insertions(+), 23

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

2024-04-19 Thread Steven Rostedt
d new flag to skip timestamp recording. > > > > Overview > > > > This series does major 2 changes, enable multiple function-graphs on > > the ftrace (e.g. allow function-graph on sub instances) and rewrite the > > fprobe on this function-graph. > >

Re: [PATCH v20 2/5] ring-buffer: Introducing ring-buffer mapping functions

2024-04-18 Thread Steven Rostedt
On Thu, 18 Apr 2024 09:55:55 +0300 Mike Rapoport wrote: Hi Mike, Thanks for doing this review! > > +/** > > + * struct trace_buffer_meta - Ring-buffer Meta-page description > > + * @meta_page_size:Size of this meta-page. > > + * @meta_struct_len: Size of this structure. > > + *

Re: [PATCH for-next v2] tracing/kprobes: Add symbol counting check when module loads

2024-04-15 Thread Steven Rostedt
On Mon, 15 Apr 2024 18:40:23 +0900 "Masami Hiramatsu (Google)" wrote: > Check the number of probe target symbols in the target module when > the module is loaded. If the probe is not on the unique name symbols > in the module, it will be rejected at that point. > > Note that the symbol which

[PATCH] ASoC: tracing: Export SND_SOC_DAPM_DIR_OUT to its value

2024-04-15 Thread Steven Rostedt
() to convert it to its value: (((REC->path_dir) == 1) ? "->" : "<-") So that user space tools, such as perf and trace-cmd, can parse it correctly. Reported-by: Luca Ceresoli Fixes: 6e588a0d839b5 ("ASoC: dapm: Consolidate path trace events") Signed-off-by:

Re: TP_printk() bug with %c, and more?

2024-04-15 Thread Steven Rostedt
On Tue, 16 Apr 2024 04:08:46 +0200 Luca Ceresoli wrote: > Thanks for the insight. I'm definitely trying to fix this based on your > hint as soon as I get my hand on a board. I have a patch I forgot to send out. Let me do that now. -- Steve

Re: TP_printk() bug with %c, and more?

2024-04-15 Thread Steven Rostedt
On Mon, 18 Mar 2024 16:43:07 +0100 Luca Ceresoli wrote: > However the arrows are still reversed. This requires a kernel change. The problem is that the print fmt has: print fmt: "%c%s %s %s %s %s", (int) REC->path_node && (int) REC->path_connect ? '*' : ' ', __get_str(wname),

Re: [RFC PATCH 0/4] perf: Correlating user process data to samples

2024-04-13 Thread Steven Rostedt
On Sat, 13 Apr 2024 12:53:38 +0200 Peter Zijlstra wrote: > On Fri, Apr 12, 2024 at 09:37:24AM -0700, Beau Belgrave wrote: > > > > Anyway, since we typically run stuff from NMI context, accessing user > > > data is 'interesting'. As such I would really like to make this work > > > depend on the

Re: [PATCH v2] tracing: Add sched_prepare_exec tracepoint

2024-04-11 Thread Steven Rostedt
On Thu, 11 Apr 2024 08:15:05 -0700 Kees Cook wrote: > This looks good to me. If tracing wants to take it: > > Acked-by: Kees Cook > > If not, I can take it in my tree if I get a tracing Ack. :) You can take it. Acked-by: Steven Rostedt (Google) -- Steve

[PATCH v2 11/11] tracing: Update function tracing output for previous boot buffer

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" For a persistent ring buffer that is saved across boots, if function tracing was performed in the previous boot, it only saves the address of the functions and uses "%pS" to print their names. But the current boot, those functions may be in

[PATCH v2 10/11] tracing: Handle old buffer mappings for event strings and functions

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Use the saved text_delta and data_delta of a persistent memory mapped ring buffer that was saved from a previous boot, and use the delta in the trace event print output so that strings and functions show up normally. That is, for an event like tra

[PATCH v2 09/11] tracing/ring-buffer: Add last_boot_info file to boot instance

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" If an instance is mapped to memory on boot up, create a new file called "last_boot_info" that will hold information that can be used to properly parse the raw data in the ring buffer. It will export the delta of the addresses for tex

[PATCH v2 08/11] ring-buffer: Save text and data locations in mapped meta data

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" When a ring buffer is mapped to a specific address, save the address of a text function and some data. This will be used to determine the delta between the last boot and the current boot for pointers to functions as well as to data. Signed-off-by: Stev

[PATCH v2 07/11] ring-buffer: Validate boot range memory events

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Make sure all the events in each of the sub-buffers that were mapped in a memory region are valid. This moves the code that walks the buffers for time-stamp validation out of the CONFIG_RING_BUFFER_VALIDATE_TIME_DELTAS ifdef block and is used t

[PATCH v2 06/11] ring-buffer: Add test if range of boot buffer is valid

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Add a test against the ring buffer memory range to see if it has valid data. The ring_buffer_meta structure is given a new field called "first_buffer" which holds the address of the first sub-buffer. This is used to both determine if the ot

[PATCH v2 05/11] ring-buffer: Add output of ring buffer meta page

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Add a buffer_meta per-cpu file for the trace instance that is mapped to boot memory. This shows the current meta-data and can be used by user space tools to record off the current mappings to help reconstruct the ring buffer after a reboot. It does not

[PATCH v2 04/11] tracing: Implement creating an instance based on a given memory region

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Allow for creating a new instance by passing in an address and size to map the ring buffer for the instance to. This will allow features like a pstore memory mapped region to be used for an tracing instance ring buffer that can be retrieved fro

[PATCH v2 03/11] ring-buffer: Add ring_buffer_meta data

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Populate the ring_buffer_meta array. It holds the pointer to the head_buffer (next to read), the commit_buffer (next to write) the size of the sub-buffers, number of sub-buffers and an array that keeps track of the order of the sub-buffers. This i

[PATCH v2 00/11] tracing: Persistent traces across a reboot or crash

2024-04-10 Thread Steven Rostedt
. This is no longer a POC as it seems to be working as expected. This is based on top of Vincent Donnefort's ring buffer user space mapping code: https://lore.kernel.org/linux-trace-kernel/20240406173649.3210836-1-vdonnef...@google.com/ Enjoy... Steven Rostedt (Google) (11): ring-

[PATCH v2 01/11] ring-buffer: Allow mapped field to be set without mapping

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" In preparation for having the ring buffer mapped to a dedicated location, which will have the same restrictions as user space memory mapped buffers, allow it to use the "mapped" field of the ring_buffer_per_cpu structure without having the

[PATCH v2 02/11] ring-buffer: Add ring_buffer_alloc_range()

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" In preparation to allowing the trace ring buffer to be allocated in a range of memory that is persistent across reboots, add ring_buffer_alloc_range(). It takes a contiguous range of memory and will split it up evening for the per CPU ring buffers.

Re: [PATCH] SUNRPC: Fix rpcgss_context trace event acceptor field

2024-04-10 Thread Steven Rostedt
On Wed, 10 Apr 2024 13:07:20 -0400 Chuck Lever wrote: > Well I guess I could test it for you. I'll take it for nfsd v6.9-rc. Thanks! -- Steve

Re: [PATCH v20 0/5] Introducing trace buffer mapping by user-space

2024-04-10 Thread Steven Rostedt
Hi Andrew, et.al. Linus said it's a hard requirement that this code gets an Acked-by (or Reviewed-by) from the memory sub-maintainers before he will accept it. He was upset that we faulted in pages one at a time instead of mapping it in one go:

Re: [PATCH v20 2/5] ring-buffer: Introducing ring-buffer mapping functions

2024-04-10 Thread Steven Rostedt
On Sat, 6 Apr 2024 18:36:46 +0100 Vincent Donnefort wrote: > +int ring_buffer_map(struct trace_buffer *buffer, int cpu, > + struct vm_area_struct *vma) > +{ > + struct ring_buffer_per_cpu *cpu_buffer; > + unsigned long flags, *subbuf_ids; > + int err = 0; > + > +

Re: [PATCH v20 1/5] ring-buffer: allocate sub-buffers with __GFP_COMP

2024-04-10 Thread Steven Rostedt
Hi Vincent, Thanks for sending this. Nit: Subject should start with a capital: ring-buffer: Allocate sub-buffers with __GFP_COMP -- Steve On Sat, 6 Apr 2024 18:36:45 +0100 Vincent Donnefort wrote: > In preparation for the ring-buffer memory mapping, allocate compound > pages for the

Re: [PATCH] SUNRPC: Fix rpcgss_context trace event acceptor field

2024-04-10 Thread Steven Rostedt
On Wed, 10 Apr 2024 12:38:53 -0400 Chuck Lever wrote: > On Wed, Apr 10, 2024 at 12:38:13PM -0400, Steven Rostedt wrote: > > From: "Steven Rostedt (Google)" > > > > The rpcgss_context trace event acceptor field is a dynamically sized > > str

[PATCH] SUNRPC: Fix rpcgss_context trace event acceptor field

2024-04-10 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The rpcgss_context trace event acceptor field is a dynamically sized string that records the "data" parameter. But this parameter is also dependent on the "len" field to determine the size of the data. It needs to use __string_len()

Re: [PATCH] ftrace: Fix use-after-free issue in ftrace_location()

2024-04-10 Thread Steven Rostedt
On Mon, 1 Apr 2024 20:55:43 +0800 Zheng Yejian wrote: > KASAN reports a bug: > > BUG: KASAN: use-after-free in ftrace_location+0x90/0x120 > Read of size 8 at addr 888141d40010 by task insmod/424 > CPU: 8 PID: 424 Comm: insmod Tainted: GW 6.9.0-rc2+ #213 > [...] >

Re: [PATCH] ring-buffer: Only update pages_touched when a new page is touched

2024-04-09 Thread Steven Rostedt
On Wed, 10 Apr 2024 08:44:00 +0900 Masami Hiramatsu (Google) wrote: > Looks good to me. > > Acked-by: Masami Hiramatsu (Google) Thanks. > > BTW, isn't this a real bugfix, because the page_touched can be > bigger than nr_pages without this fix? Yes, I simply forgot to add the Cc stable. --

[PATCH] ring-buffer: Only update pages_touched when a new page is touched

2024-04-09 Thread Steven Rostedt
From: "Steven Rostedt (Google)" The "buffer_percent" logic that is used by the ring buffer splice code to only wake up the tasks when there's no data after the buffer is filled to the percentage of the "buffer_percent" file is dependent on three variables tha

Re: [PATCH net-next v3 6/6] rstreason: make it work in trace world

2024-04-09 Thread Steven Rostedt
> +#define FNe2(reason) { SKB_DROP_REASON_##reason, #reason } Anyway, from a tracing point of view, as it looks like it would work (I haven't tested it). Reviewed-by: Steven Rostedt (Google) -- Steve

Re: [PATCH 1/3] tracing: Remove dependency of saved_cmdlines_buffer on PID_MAX_DEFAULT

2024-04-09 Thread Steven Rostedt
On Mon, 8 Apr 2024 16:58:17 +0200 Michal Koutný wrote: > @@ -294,7 +295,7 @@ static void __trace_find_cmdline(int pid, char comm[]) > return; > } > > - tpid = pid & (PID_MAX_DEFAULT - 1); > + tpid = pid % PID_MAP_SIZE; Does that compile to the same? This is a fast

Re: [PATCH] tracing: Add new_exec tracepoint

2024-04-09 Thread Steven Rostedt
On Mon, 8 Apr 2024 11:01:54 +0200 Marco Elver wrote: > Add "new_exec" tracepoint, which is run right after the point of no > return but before the current task assumes its new exec identity. > > Unlike the tracepoint "sched_process_exec", the "new_exec" tracepoint > runs before flushing the

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

2024-04-03 Thread Steven Rostedt
On Wed, 3 Apr 2024 11:53:14 -0700 "Paul E. McKenney" wrote: > @@ -5366,6 +5366,13 @@ static void remove_direct_functions_hash(struct > ftrace_hash *hash, unsigned long > } > } > > +static void register_ftrace_direct_cb(struct rcu_head *rhp) > +{ > + struct ftrace_hash *fhp =

Re: [PATCH v19 RESEND 3/5] tracing: Allow user-space mapping of the ring-buffer

2024-04-03 Thread Steven Rostedt
On Wed, 3 Apr 2024 15:39:44 +0100 Vincent Donnefort wrote: > > Do you plan on sending out a v20 series? > > Of course, let me spin that this week! Got also few typos to fix in the doc > and > I believe an include missing for riscv. No rush, I'll be on PTO until next Tuesday, and will not

Re: [PATCH v19 RESEND 3/5] tracing: Allow user-space mapping of the ring-buffer

2024-04-03 Thread Steven Rostedt
On Fri, 29 Mar 2024 14:40:55 -0400 Steven Rostedt wrote: > > +static vm_fault_t tracing_buffers_mmap_fault(struct vm_fault *vmf) > > +{ > > + return VM_FAULT_SIGBUS; > > +} > > If this is all it does, I don't believe it's needed. > > > + > > +#

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-03 Thread Steven Rostedt
On Tue, 2 Apr 2024 22:21:00 -0700 Andrii Nakryiko wrote: > > I just checked our fleet-wide production data for the last 24 hours. > > Within the kprobe/kretprobe code path (ftrace_trampoline and > > everything called from it), rcu_is_watching (both calls, see below) > > cause 0.484% CPU cycles

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-03 Thread Steven Rostedt
On Tue, 2 Apr 2024 21:00:19 -0700 Andrii Nakryiko wrote: > I just noticed another rcu_is_watching() call, in rethook_try_get(), > which seems to be a similar and complementary validation check to the > one we are putting under CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING option > in this patch. It

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-02 Thread Steven Rostedt
On Wed, 3 Apr 2024 09:40:48 +0900 Masami Hiramatsu (Google) wrote: > OK, for me, this last sentence is preferred for the help message. That > explains > what this is for. > > All callbacks that attach to the function tracing have some sort > of protection against recursion.

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-01 Thread Steven Rostedt
On Mon, 1 Apr 2024 19:29:46 -0700 Andrii Nakryiko wrote: > On Mon, Apr 1, 2024 at 5:38 PM Masami Hiramatsu wrote: > > > > On Mon, 1 Apr 2024 12:09:18 -0400 > > Steven Rostedt wrote: > > > > > On Mon, 1 Apr 2024 20:25:52 +0900 > > > Masami Hirama

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-04-01 Thread Steven Rostedt
On Mon, 1 Apr 2024 20:25:52 +0900 Masami Hiramatsu (Google) wrote: > > Masami, > > > > Are you OK with just keeping it set to N. > > OK, if it is only for the debugging, I'm OK to set N this. > > > > > We could have other options like PROVE_LOCKING enable it. > > Agreed (but it should

Re: [PATCH v19 RESEND 3/5] tracing: Allow user-space mapping of the ring-buffer

2024-03-29 Thread Steven Rostedt
On Tue, 26 Mar 2024 10:08:28 + Vincent Donnefort wrote: > Currently, user-space extracts data from the ring-buffer via splice, > which is handy for storage or network sharing. However, due to splice > limitations, it is imposible to do real-time analysis without a copy. > > A solution for

Re: [PATCH] trace/sched: add tgid for sched_wakeup_template

2024-03-27 Thread Steven Rostedt
On Wed, 27 Mar 2024 16:50:57 +0800 Tio Zhang wrote: > By doing this, we are able to filter tasks by tgid while we are > tracing wakeup events by ebpf or other methods. > > For example, when we care about tracing a user space process (which has > uncertain number of LWPs, i.e, pids) to monitor

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-26 Thread Steven Rostedt
On Tue, 26 Mar 2024 09:16:33 -0700 Andrii Nakryiko wrote: > > It's no different than lockdep. Test boxes should have it enabled, but > > there's no reason to have this enabled in a production system. > > > > I tend to agree with Steven here (which is why I sent this patch as it > is), but I'm

Re: [PATCH 11/12] [v4] kallsyms: rework symbol lookup return codes

2024-03-26 Thread Steven Rostedt
On Tue, 26 Mar 2024 15:53:38 +0100 Arnd Bergmann wrote: > -const char * > +int > ftrace_mod_address_lookup(unsigned long addr, unsigned long *size, > unsigned long *off, char **modname, char *sym) > { > struct ftrace_mod_map *mod_map; > - const char *ret = NULL; > +

Re: [PATCH] ftrace: make extra rcu_is_watching() validation check optional

2024-03-25 Thread Steven Rostedt
that it *does cause overhead* with function tracing. I believe we found pretty much all locations that were an issue, and we should now just make it an option for developers. It's no different than lockdep. Test boxes should have it enabled, but there's no reason to have this enabled in a production

Re: [RFC PATCH] riscv: Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS

2024-03-21 Thread Steven Rostedt
On Tue, 12 Mar 2024 13:42:28 + Mark Rutland wrote: > There are ways around that, but they're complicated and/or expensive, e.g. > > * Use a sequence of multiple patches, starting with replacing the JALR with an > exception-generating instruction with a fixup handler, which is sort-of what

Re: [PATCH v7 1/5] tracing/probes: support '%pd' type for print struct dentry's name

2024-03-21 Thread Steven Rostedt
On Fri, 22 Mar 2024 00:28:05 +0900 Masami Hiramatsu (Google) wrote: > On Fri, 22 Mar 2024 00:07:59 +0900 > Masami Hiramatsu (Google) wrote: > > > > What would be really useful is if we had a way to expose BTF here. > > > Something like: > > > > > > "%pB::" > > > > > > The "%pB" would mean

Re: [PATCH v7 1/5] tracing/probes: support '%pd' type for print struct dentry's name

2024-03-21 Thread Steven Rostedt
On Fri, 22 Mar 2024 00:07:59 +0900 Masami Hiramatsu (Google) wrote: > > What would be really useful is if we had a way to expose BTF here. > > Something like: > > > > "%pB::" > > > > The "%pB" would mean to look up the struct/field offsets and types via BTF, > > and create the appropriate

Re: [RFC PATCH] riscv: Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS

2024-03-21 Thread Steven Rostedt
On Tue, 12 Mar 2024 13:42:28 + Mark Rutland wrote: > > It would be interesting to see how the per-call performance would > > improve on x86 with CALL_OPS! ;-) > > Heh. ;) But this would require adding -fpatchable-function-entry on x86, which would increase the size of text, which could

Re: [PATCH v7 1/5] tracing/probes: support '%pd' type for print struct dentry's name

2024-03-21 Thread Steven Rostedt
On Wed, 20 Mar 2024 21:29:20 +0800 Ye Bin wrote: > Support print type '%pd' for print dentry's name. > The above is not a very detailed change log. A change log should state not only what the change is doing, but also why. Having examples of before and after would be useful in the change

Re: [PATCH v3] net/ipv4: add tracepoint for icmp_send

2024-03-21 Thread Steven Rostedt
On Thu, 21 Mar 2024 10:45:00 +0800 Jason Xing wrote: > The format of the whole patch looks strange... Did you send this patch > by using 'git send-email' instead of pasting the text and sending? Yeah, it's uuencoded. Subject:

Re: [RFC][PATCH] tracing: Introduce restart_critical_timings()

2024-03-20 Thread Steven Rostedt
On Wed, 20 Mar 2024 20:46:11 -0400 Waiman Long wrote: > I have no objection to that. However, there are now 2 function call > overhead in each iteration if either CONFIG_IRQSOFF_TRACER or > CONFIG_PREEMPT_TRACER is on. Is it possible to do it with just one > function call? IOW, make

Re: [RFC][PATCH] tracing: Introduce restart_critical_timings()

2024-03-20 Thread Steven Rostedt
On Wed, 20 Mar 2024 13:15:39 -0400 Mathieu Desnoyers wrote: > > I would like to introduce restart_critical_timings() and place it in > > locations that have this behavior. > > Is there any way you could move this to need_resched() rather than > sprinkle those everywhere ? Because

[RFC][PATCH] tracing: Introduce restart_critical_timings()

2024-03-20 Thread Steven Rostedt
From: Steven Rostedt (Google) I'm debugging some latency issues on a Chromebook and the preemptirqsoff tracer hit this: # tracer: preemptirqsoff # # preemptirqsoff latency trace v1.1.5 on 5.15.148-21853-g165fd2387469-dirty

Re: [GIT PULL] tracing/tools: Updates for 6.9

2024-03-20 Thread Steven Rostedt
On Wed, 20 Mar 2024 13:41:12 +0100 Daniel Bristot de Oliveira wrote: > On 3/20/24 00:02, Steven Rostedt wrote: > > On Mon, 18 Mar 2024 18:41:13 +0100 > > Daniel Bristot de Oliveira wrote: > > > >> Steven, > >> > >> Tracing tooling updates

Re: [PATCH] tracing: probes: Fix to zero initialize a local variable

2024-03-20 Thread Steven Rostedt
On Wed, 20 Mar 2024 17:10:38 +0900 "Masami Hiramatsu (Google)" wrote: > From: Masami Hiramatsu (Google) > > Fix to initialize 'val' local variable with zero. > Dan reported that Smatch static code checker reports an error that a local > 'val' variable needs to be initialized. Actually, the

Re: [PATCH] tracing: probes: Fix to zero initialize a local variable

2024-03-20 Thread Steven Rostedt
Fixes: 25f00e40ce79 ("tracing/probes: Support $argN in return probe (kprobe > and fprobe)") > Signed-off-by: Masami Hiramatsu (Google) > --- Reviewed-by: Steven Rostedt (Google) -- Steve > kernel/trace/trace_probe.c |2 +- > 1 file changed, 1 insertion(+), 1

Re: [bug report] tracing/probes: Support $argN in return probe (kprobe and fprobe)

2024-03-20 Thread Steven Rostedt
On Wed, 20 Mar 2024 12:44:23 +0900 Masami Hiramatsu (Google) wrote: > > > kernel/trace/trace_probe.c > > > 846 return; > > > 847 > > > 848 for (i = 0; i < earg->size; i++) { > > > 849 struct fetch_insn *code = >code[i]; > > > 850 > >

Re: [PATCH 0/8] tracing: Persistent traces across a reboot or crash

2024-03-19 Thread Steven Rostedt
On Sat, 9 Mar 2024 12:40:51 -0800 Kees Cook wrote: > The part I'd like to get wired up sanely is having pstore find the > nvdimm area automatically, but it never quite happened: > https://lore.kernel.org/lkml/CAGXu5jLtmb3qinZnX3rScUJLUFdf+pRDVPjy=cs4kutw9tl...@mail.gmail.com/ The automatic

Re: [PATCH 2/2] tracing: Ignore -Wstring-compare with diagnostic macros

2024-03-19 Thread Steven Rostedt
On Tue, 19 Mar 2024 17:30:41 -0700 Justin Stitt wrote: > > diff --git a/include/trace/stages/stage6_event_callback.h > > b/include/trace/stages/stage6_event_callback.h > > index 83da83a0c14f..56a4eea5a48e 100644 > > --- a/include/trace/stages/stage6_event_callback.h > > +++

Re: [GIT PULL] tracing/tools: Updates for 6.9

2024-03-19 Thread Steven Rostedt
On Mon, 18 Mar 2024 18:41:13 +0100 Daniel Bristot de Oliveira wrote: > Steven, > > Tracing tooling updates for 6.9 > > Tracing: > - Update makefiles for latency-collector and RTLA, > using tools/build/ makefiles like perf does, inheriting > its benefits. For

  1   2   3   4   5   6   7   8   9   10   >