mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order
@ 2026-09-12 10:39 David Carlier
  2026-09-13 20:38 ` Vincent Donnefort
  0 siblings, 1 reply; 4+ messages in thread
From: David Carlier @ 2026-09-12 10:39 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Vincent Donnefort, Mathieu Desnoyers, linux-trace-kernel,
	linux-kernel, David Carlier, stable

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order
  2026-09-12 10:39 [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order David Carlier
@ 2026-09-13 20:38 ` Vincent Donnefort
  2026-09-13 20:41   ` Vincent Donnefort
  0 siblings, 1 reply; 4+ messages in thread
From: Vincent Donnefort @ 2026-09-13 20:38 UTC (permalink / raw)
  To: David Carlier
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, linux-kernel, stable

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order
  2026-09-13 20:38 ` Vincent Donnefort
@ 2026-09-13 20:41   ` Vincent Donnefort
  2026-09-14 13:27     ` Steven Rostedt
  0 siblings, 1 reply; 4+ messages in thread
From: Vincent Donnefort @ 2026-09-13 20:41 UTC (permalink / raw)
  To: David Carlier
  Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, linux-kernel, stable

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order
  2026-09-13 20:41   ` Vincent Donnefort
@ 2026-09-14 13:27     ` Steven Rostedt
  0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2026-09-14 13:27 UTC (permalink / raw)
  To: Vincent Donnefort
  Cc: David Carlier, Masami Hiramatsu, Mathieu Desnoyers,
	linux-trace-kernel, linux-kernel, stable

On Sun, 13 Sep 2026 21:41:14 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> 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?

FYI, I already pulled this patch and it's in 7.3-rc3. It looked fine to
me as I was just kicking off one of my long test sessions, so I threw
it in. Perhaps I should have waited to this week. :-/

Any update needs to be done on top of this patch.

-- Steve

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-14 13:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-12 10:39 [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order David Carlier
2026-09-13 20:38 ` Vincent Donnefort
2026-09-13 20:41   ` Vincent Donnefort
2026-09-14 13:27     ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®