[PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order

[PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order

[David Carlier]

ring_buffer_subbuf_order_set() stores the new order and only then walks
the CPUs, returning -EBUSY if any of them has resizing disabled. A user
mapped buffer has resizing disabled, and __rb_map_vma() reads
buffer->subbuf_order without buffer->mutex, so an mmap of an already
mapped CPU racing the failing order change sizes the mapping with the
new order and inserts pages past the sub-buffer into the VMA.

Check the CPUs before storing the new order.

Fixes: 117c39200d9d ("ring-buffer: Introducing ring-buffer mapping functions")
Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 kernel/trace/ring_buffer.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 9c03a555a6ba..d7e5e4620d09 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -7474,6 +7474,14 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 
 	old_capacity = rb_subbuf_capacity(buffer);
 
+	/* The mmap fast path reads subbuf_order without buffer->mutex. */
+	for_each_buffer_cpu(buffer, cpu) {
+		if (!cpumask_test_cpu(cpu, buffer->cpumask))
+			continue;
+		if (atomic_read(&buffer->buffers[cpu]->resize_disabled))
+			return -EBUSY;
+	}
+
 	atomic_inc(&buffer->record_disabled);
 
 	/* Make sure all commits have finished */
-- 
2.55.0

Re: [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing cpus_read_lock() allows concurrent CPU hotplug events to crash ring_buffer_subbuf_order_set() via uninitialized list dereference or NULL pointer dereference.
--

commit d554518f86e022de0069fc76033b9d71e9046148
Author: David Carlier <devnexen@gmail.com>

ring-buffer: Check resize_disabled before publishing the new subbuf order

This commit checks if resizing is disabled on any CPU before applying
the new sub-buffer order. This aims to prevent a race condition with
mmap fast paths mapping already-mapped CPUs with the new order.

Cc: stable@vger.kernel.org

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 9c03a555a6ba2..d7e5e4620d096 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -7474,6 +7474,14 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
>  
>  	old_capacity = rb_subbuf_capacity(buffer);
>  
> +	/* The mmap fast path reads subbuf_order without buffer->mutex. */
> +	for_each_buffer_cpu(buffer, cpu) {
> +		if (!cpumask_test_cpu(cpu, buffer->cpumask))
> +			continue;
> +		if (atomic_read(&buffer->buffers[cpu]->resize_disabled))
> +			return -EBUSY;
> +	}
> +

[Severity: High]
Can this new loop race with concurrent CPU hotplug events and trigger a NULL
pointer dereference?

Unlike ring_buffer_resize(), ring_buffer_subbuf_order_set() doesn't appear
to take cpus_read_lock(). Meanwhile, trace_rb_cpu_prepare() can update
buffer->cpumask without holding buffer->mutex.

On weakly ordered architectures, could the lack of an smp_rmb() here cause
the reader to observe a newly added CPU in buffer->cpumask but load a NULL
buffer->buffers[cpu]?

Additionally, without cpus_read_lock(), if a CPU is added to the cpumask
between the allocation loop and the final page update loop later in this
function, new_pages will be an empty list head:

ring_buffer_subbuf_order_set() {
    ...
    /* One page was allocated for the reader page */
    cpu_buffer->reader_page = list_entry(cpu_buffer->new_pages.next,
                                         struct buffer_page, list);
    list_del_init(&cpu_buffer->reader_page->list);
    ...
}

Could this calculate a fake pointer overlapping the cpu_buffer struct and
corrupt memory when modified?

>  	atomic_inc(&buffer->record_disabled);
>  
>  	/* Make sure all commits have finished */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912103938.1127021-1-devnexen@gmail.com?part=1

Re: [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order

[David CARLIER]

The first point does not hold. trace_rb_cpu_prepare() allocates
buffer->buffers[cpu] and IPIs via rb_cpu_sync() before setting the cpumask
bit, so observing the bit implies observing the pointer.

owever, The second is real but predates this patch:
ring_buffer_subbuf_order_set()
never took cpus_read_lock(), we can fix as follow :

--- a/kernel/trace/ring_buffer.c
(/home/dcarlier/Contribs/linux-trace/kernel/trace/ring_buffer.c)
+++ b/kernel/trace/ring_buffer.c
@@ -7465,6 +7465,12 @@ int ring_buffer_subbuf_order_set(struct
trace_buffer *buffer, int order)
      if (psize > RB_WRITE_MASK + 1)
              return -EINVAL;

+     /*
+      * Keep CPUs from coming online while changing the order to
+      * synchronize with new per CPU buffers being created.
+      */
+     guard(cpus_read_lock)();
+
      /* prevent another thread from changing buffer sizes */
      guard(mutex)(&buffer->mutex);

Cheers.

On Sat, 12 Sept 2026 at 11:56, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing cpus_read_lock() allows concurrent CPU hotplug events to crash ring_buffer_subbuf_order_set() via uninitialized list dereference or NULL pointer dereference.
> --
>
> commit d554518f86e022de0069fc76033b9d71e9046148
> Author: David Carlier <devnexen@gmail.com>
>
> ring-buffer: Check resize_disabled before publishing the new subbuf order
>
> This commit checks if resizing is disabled on any CPU before applying
> the new sub-buffer order. This aims to prevent a race condition with
> mmap fast paths mapping already-mapped CPUs with the new order.
>
> Cc: stable@vger.kernel.org
>
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 9c03a555a6ba2..d7e5e4620d096 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -7474,6 +7474,14 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
> >
> >       old_capacity = rb_subbuf_capacity(buffer);
> >
> > +     /* The mmap fast path reads subbuf_order without buffer->mutex. */
> > +     for_each_buffer_cpu(buffer, cpu) {
> > +             if (!cpumask_test_cpu(cpu, buffer->cpumask))
> > +                     continue;
> > +             if (atomic_read(&buffer->buffers[cpu]->resize_disabled))
> > +                     return -EBUSY;
> > +     }
> > +
>
> [Severity: High]
> Can this new loop race with concurrent CPU hotplug events and trigger a NULL
> pointer dereference?
>
> Unlike ring_buffer_resize(), ring_buffer_subbuf_order_set() doesn't appear
> to take cpus_read_lock(). Meanwhile, trace_rb_cpu_prepare() can update
> buffer->cpumask without holding buffer->mutex.
>
> On weakly ordered architectures, could the lack of an smp_rmb() here cause
> the reader to observe a newly added CPU in buffer->cpumask but load a NULL
> buffer->buffers[cpu]?
>
> Additionally, without cpus_read_lock(), if a CPU is added to the cpumask
> between the allocation loop and the final page update loop later in this
> function, new_pages will be an empty list head:
>
> ring_buffer_subbuf_order_set() {
>     ...
>     /* One page was allocated for the reader page */
>     cpu_buffer->reader_page = list_entry(cpu_buffer->new_pages.next,
>                                          struct buffer_page, list);
>     list_del_init(&cpu_buffer->reader_page->list);
>     ...
> }
>
> Could this calculate a fake pointer overlapping the cpu_buffer struct and
> corrupt memory when modified?
>
> >       atomic_inc(&buffer->record_disabled);
> >
> >       /* Make sure all commits have finished */
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260912103938.1127021-1-devnexen@gmail.com?part=1

Re: [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order

[Vincent Donnefort]

On Sat, Sep 12, 2026 at 11:39:38AM +0100, David Carlier wrote:
> ring_buffer_subbuf_order_set() stores the new order and only then walks
> the CPUs, returning -EBUSY if any of them has resizing disabled. A user
> mapped buffer has resizing disabled, and __rb_map_vma() reads
> buffer->subbuf_order without buffer->mutex, so an mmap of an already
> mapped CPU racing the failing order change sizes the mapping with the
> new order and inserts pages past the sub-buffer into the VMA.
> 
> Check the CPUs before storing the new order.
> 
> Fixes: 117c39200d9d ("ring-buffer: Introducing ring-buffer mapping functions")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  kernel/trace/ring_buffer.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 9c03a555a6ba..d7e5e4620d09 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -7474,6 +7474,14 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
>  
>  	old_capacity = rb_subbuf_capacity(buffer);
>  
> +	/* The mmap fast path reads subbuf_order without buffer->mutex. */
> +	for_each_buffer_cpu(buffer, cpu) {
> +		if (!cpumask_test_cpu(cpu, buffer->cpumask))
> +			continue;
> +		if (atomic_read(&buffer->buffers[cpu]->resize_disabled))
> +			return -EBUSY;
> +	}
> +
>  	atomic_inc(&buffer->record_disabled);
>  
>  	/* Make sure all commits have finished */
> -- 
> 2.55.0
>

I do not think this is correct.

__rb_map_vma() reads subbuf_order without the the buffer lock __only__ if it is
already ->user_mapped, which means it has already the resized disabled.

During the first setup, it takes buffer->mutex.

-- 
Vincent

Re: [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order

[Vincent Donnefort]

On Sun, Sep 13, 2026 at 09:38:03PM +0100, Vincent Donnefort wrote:
> On Sat, Sep 12, 2026 at 11:39:38AM +0100, David Carlier wrote:
> > ring_buffer_subbuf_order_set() stores the new order and only then walks
> > the CPUs, returning -EBUSY if any of them has resizing disabled. A user
> > mapped buffer has resizing disabled, and __rb_map_vma() reads
> > buffer->subbuf_order without buffer->mutex, so an mmap of an already
> > mapped CPU racing the failing order change sizes the mapping with the
> > new order and inserts pages past the sub-buffer into the VMA.
> > 
> > Check the CPUs before storing the new order.
> > 
> > Fixes: 117c39200d9d ("ring-buffer: Introducing ring-buffer mapping functions")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
> >  kernel/trace/ring_buffer.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 9c03a555a6ba..d7e5e4620d09 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -7474,6 +7474,14 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
> >  
> >  	old_capacity = rb_subbuf_capacity(buffer);
> >  
> > +	/* The mmap fast path reads subbuf_order without buffer->mutex. */
> > +	for_each_buffer_cpu(buffer, cpu) {
> > +		if (!cpumask_test_cpu(cpu, buffer->cpumask))
> > +			continue;
> > +		if (atomic_read(&buffer->buffers[cpu]->resize_disabled))
> > +			return -EBUSY;
> > +	}
> > +
> >  	atomic_inc(&buffer->record_disabled);
> >  
> >  	/* Make sure all commits have finished */
> > -- 
> > 2.55.0
> >
> 
> I do not think this is correct.
> 
> __rb_map_vma() reads subbuf_order without the the buffer lock __only__ if it is
> already ->user_mapped, which means it has already the resized disabled.
> 
> During the first setup, it takes buffer->mutex.
> 
> -- 
> Vincent

Ha my bad, that function also modifies the subbuf_order __before__ checking the
resize_disabled.

Could we also clean the later resize_disabled check in the following loop?

-- 
Vincent