Re: [PATCH] tracing: Fix WARN_ON in tracing_buffers_mmap_close

[Vincent Donnefort <vdonnefort@google.com>]

On Fri, Feb 27, 2026 at 10:41:17AM +0000, Vincent Donnefort wrote:
> On Fri, Feb 27, 2026 at 10:02:00AM +0000, Vincent Donnefort wrote:
> > On Fri, Feb 27, 2026 at 10:58:42AM +0800, Qing Wang wrote:
> > > When a process forks, the child process copies the parent's VMAs but the
> > > user_mapped reference count is not incremented. As a result, when both the
> > > parent and child processes exit, tracing_buffers_mmap_close() is called
> > > twice. On the second call, user_mapped is already 0, causing the function to
> > > return -ENODEV and triggering a WARN_ON.
> > > 
> > > Fix it by incrementing the user_mapped reference count without re-mapping
> > > the pages in the VMA's open callback.
> > 
> > Hum, not sure this is entirely correct. We do set VM_DONTCOPY when creating the
> > mapping (see __rb_map_vma). So AFAICT ->open() is not called in this situation (see
> > dup_mmap())
> 
> Ah right, Syzkaller is using madvise(MADVISE_DOFORK) which resets VM_DONTCOPY.

As we are applying restrictive rules for this mapping, I believe setting VM_IO
might be a better fix.

> 
> > 
> > > 
> > > Fixes: cf9f0f7c4c5bb ("tracing: Allow user-space mapping of the ring-buffer")
> > > Reported-by: syzbot+3b5dd2030fe08afdf65d@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=3b5dd2030fe08afdf65d
> > > Tested-by: syzbot+3b5dd2030fe08afdf65d@syzkaller.appspotmail.com
> > > Signed-off-by: Qing Wang <wangqing7171@gmail.com>
> > > ---
> > >  include/linux/ring_buffer.h |  1 +
> > >  kernel/trace/ring_buffer.c  | 21 +++++++++++++++++++++
> > >  kernel/trace/trace.c        | 13 +++++++++++++
> > >  3 files changed, 35 insertions(+)
> > > 
> > > diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
> > > index 876358cfe1b1..d862fa610270 100644
> > > --- a/include/linux/ring_buffer.h
> > > +++ b/include/linux/ring_buffer.h
> > > @@ -248,6 +248,7 @@ int trace_rb_cpu_prepare(unsigned int cpu, struct hlist_node *node);
> > >  
> > >  int ring_buffer_map(struct trace_buffer *buffer, int cpu,
> > >  		    struct vm_area_struct *vma);
> > > +void ring_buffer_map_dup(struct trace_buffer *buffer, int cpu);
> > >  int ring_buffer_unmap(struct trace_buffer *buffer, int cpu);
> > >  int ring_buffer_map_get_reader(struct trace_buffer *buffer, int cpu);
> > >  #endif /* _LINUX_RING_BUFFER_H */
> > > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > > index f16f053ef77d..17d0ea0cc3e6 100644
> > > --- a/kernel/trace/ring_buffer.c
> > > +++ b/kernel/trace/ring_buffer.c
> > > @@ -7310,6 +7310,27 @@ int ring_buffer_map(struct trace_buffer *buffer, int cpu,
> > >  	return err;
> > >  }
> > >  
> > > +/*
> > > + * This is called when a VMA is duplicated (e.g., on fork()) to increment
> > > + * the user_mapped counter without remapping pages.
> > > + */
> > > +void ring_buffer_map_dup(struct trace_buffer *buffer, int cpu)
> > > +{
> > > +	struct ring_buffer_per_cpu *cpu_buffer;
> > > +
> > > +	if (WARN_ON(!cpumask_test_cpu(cpu, buffer->cpumask)))
> > > +		return;
> > > +
> > > +	cpu_buffer = buffer->buffers[cpu];
> > > +
> > > +	guard(mutex)(&cpu_buffer->mapping_lock);
> > > +
> > > +	if (cpu_buffer->user_mapped)
> > > +		__rb_inc_dec_mapped(cpu_buffer, true);
> > > +	else
> > > +		WARN(1, "Unexpected buffer stat, it should be mapped");
> > > +}
> > > +
> > >  int ring_buffer_unmap(struct trace_buffer *buffer, int cpu)
> > >  {
> > >  	struct ring_buffer_per_cpu *cpu_buffer;
> > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > > index 23de3719f495..1e7c032a72d2 100644
> > > --- a/kernel/trace/trace.c
> > > +++ b/kernel/trace/trace.c
> > > @@ -8213,6 +8213,18 @@ static inline int get_snapshot_map(struct trace_array *tr) { return 0; }
> > >  static inline void put_snapshot_map(struct trace_array *tr) { }
> > >  #endif
> > >  
> > > +/*
> > > + * This is called when a VMA is duplicated (e.g., on fork()) to increment
> > > + * the user_mapped counter without remapping pages.
> > > + */
> > > +static void tracing_buffers_mmap_open(struct vm_area_struct *vma)
> > > +{
> > > +	struct ftrace_buffer_info *info = vma->vm_file->private_data;
> > > +	struct trace_iterator *iter = &info->iter;
> > > +
> > > +	ring_buffer_map_dup(iter->array_buffer->buffer, iter->cpu_file);
> > > +}
> > > +
> > >  static void tracing_buffers_mmap_close(struct vm_area_struct *vma)
> > >  {
> > >  	struct ftrace_buffer_info *info = vma->vm_file->private_data;
> > > @@ -8232,6 +8244,7 @@ static int tracing_buffers_may_split(struct vm_area_struct *vma, unsigned long a
> > >  }
> > >  
> > >  static const struct vm_operations_struct tracing_buffers_vmops = {
> > > +	.open		= tracing_buffers_mmap_open,
> > >  	.close		= tracing_buffers_mmap_close,
> > >  	.may_split      = tracing_buffers_may_split,
> > >  };
> > > -- 
> > > 2.34.1
> > >