linux-kernel - Re: [PATCH 1/2] tracing: Make the backup instance readonly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20260108115125.b832464d94ebcfd0ea3b5af4@kernel.org>
Date: Thu, 8 Jan 2026 11:51:25 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] tracing: Make the backup instance readonly

On Wed, 7 Jan 2026 11:41:33 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:

> On Wed,  7 Jan 2026 23:45:59 +0900
> "Masami Hiramatsu (Google)" <mhiramat@...nel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> > 
> > Since there is no reason to reuse the backup instance, make it
> > readonly. Note that only backup instances are readonly, because
> > other trace instances will be empty unless it is writable.
> > Only backup instances have copy entries from the original.
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> > ---
> >  kernel/trace/trace.c        |   91 ++++++++++++++++++++++++++++++++-----------
> >  kernel/trace/trace.h        |    6 +++
> >  kernel/trace/trace_events.c |   14 +++++--
> >  3 files changed, 84 insertions(+), 27 deletions(-)
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 38f7a7a55c23..725930f5980e 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -4888,6 +4888,9 @@ static int tracing_open(struct inode *inode, struct file *file)
> >  		int cpu = tracing_get_cpu(inode);
> >  		struct array_buffer *trace_buf = &tr->array_buffer;
> >  
> > +		if (trace_array_is_readonly(tr))
> > +			return -EPERM;
> 
> So this fails if someone opens a file in RDONLY mode?
> 
> Why?

Ah, that's a bug. Let me fix it.

> 
> 
> > +
> >  #ifdef CONFIG_TRACER_MAX_TRACE
> >  		if (tr->current_trace->print_max)
> >  			trace_buf = &tr->max_buffer;
> > @@ -6077,6 +6080,9 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr,
> >  ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
> >  				  unsigned long size, int cpu_id)
> >  {
> > +	if (trace_array_is_readonly(tr))
> > +		return -EPERM;
> 
> In fact, I don't think we need any of these.

Would you mean we should check readonly 

> 
> > +
> >  	guard(mutex)(&trace_types_lock);
> >  
> >  	if (cpu_id != RING_BUFFER_ALL_CPUS) {
> 
> 
> 
> > @@ -9353,12 +9381,16 @@ static void
> >  tracing_init_tracefs_percpu(struct trace_array *tr, long cpu)
> >  {
> >  	struct dentry *d_percpu = tracing_dentry_percpu(tr, cpu);
> > +	umode_t writable_mode = TRACE_MODE_WRITE;
> >  	struct dentry *d_cpu;
> >  	char cpu_dir[30]; /* 30 characters should be more than enough */
> >  
> >  	if (!d_percpu)
> >  		return;
> >  
> > +	if (trace_array_is_readonly(tr))
> > +		writable_mode = TRACE_MODE_READ;
> 
> This is more like what we should do with all the files in a read-only
> instance. Just make all files not allow writes.

Actually, that's my first prototype but it did not work (at least on
tracefs).
Superuser can write anything unless the file does not have .write
operation.

> 
> We may need to make sure they can't be changed to write as well. But that
> will require a change to tracefs (and eventfs).

Ah, you mean the permission check is not correctly done in tracefs/eventfs yet?

Thank you,

> 
> -- Steve
> 
> 
> > +
> >  	snprintf(cpu_dir, 30, "cpu%ld", cpu);
> >  	d_cpu = tracefs_create_dir(cpu_dir, d_percpu);
> >  	if (!d_cpu) {
> > @@ -9371,7 +9403,7 @@ tracing_init_tracefs_percpu(struct trace_array *tr, long cpu)
> >  				tr, cpu, &tracing_pipe_fops);
> >  
> >  	/* per cpu trace */
> > -	trace_create_cpu_file("trace", TRACE_MODE_WRITE, d_cpu,
> > +	trace_create_cpu_file("trace", writable_mode, d_cpu,
> >  				tr, cpu, &tracing_fops);
> >  
> >  	trace_create_cpu_file("trace_pipe_raw", TRACE_MODE_READ, d_cpu,
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>