* [PATCH v2 0/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
@ 2026-07-09 16:00 Vincent Donnefort
2026-07-09 16:00 ` [PATCH v2 1/3] " Vincent Donnefort
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Vincent Donnefort @ 2026-07-09 16:00 UTC (permalink / raw)
To: rostedt, mhiramat, linux-trace-kernel
Cc: mathieu.desnoyers, kernel-team, linux-kernel, Vincent Donnefort
This series addresses a few issues in the trace remote buffers allocation
and descriptor handling, mostly reported by Sashiko bot on top of v1.
Changelog:
v1 -> v2:
- Leak fix: increment nr_cpus early (after meta_va allocation) instead of
in the error path.
- Added struct_len double-counting fix.
- Added sparse CPU masks support in ring_buffer_desc().
v1: https://lore.kernel.org/all/20260708133201.295072-1-vdonnefort@google.com/
Vincent Donnefort (3):
tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
tracing/remotes: Fix struct_len in trace_remote_alloc_buffer()
ring-buffer: Allow sparse CPU masks in ring_buffer_desc()
kernel/trace/ring_buffer.c | 5 +----
kernel/trace/trace_remote.c | 18 ++++++------------
2 files changed, 7 insertions(+), 16 deletions(-)
base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path 2026-07-09 16:00 [PATCH v2 0/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path Vincent Donnefort @ 2026-07-09 16:00 ` Vincent Donnefort 2026-07-09 23:15 ` Steven Rostedt 2026-07-09 16:00 ` [PATCH v2 2/3] tracing/remotes: Fix struct_len in trace_remote_alloc_buffer() Vincent Donnefort 2026-07-09 16:00 ` [PATCH v2 3/3] ring-buffer: Allow sparse CPU masks in ring_buffer_desc() Vincent Donnefort 2 siblings, 1 reply; 9+ messages in thread From: Vincent Donnefort @ 2026-07-09 16:00 UTC (permalink / raw) To: rostedt, mhiramat, linux-trace-kernel Cc: mathieu.desnoyers, kernel-team, linux-kernel, Vincent Donnefort, Sashiko If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus is not yet incremented for the current CPU. As a consequence, on error, half-allocated rb_desc will not be freed in trace_remote_free_buffer(). Increment desc->nr_cpus as soon as the first allocation for the current CPU has succeeded. Fixes: 96e43537af54 ("tracing: Introduce trace remotes") Reported-by: Sashiko <sashiko-bot@kernel.org> Signed-off-by: Vincent Donnefort <vdonnefort@google.com> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c index 2a6cc000ec98..d48042239d58 100644 --- a/kernel/trace/trace_remote.c +++ b/kernel/trace/trace_remote.c @@ -1006,6 +1006,8 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size, if (!rb_desc->meta_va) goto err; + desc->nr_cpus++; + for (id = 0; id < nr_pages; id++) { rb_desc->page_va[id] = (unsigned long)__get_free_page(GFP_KERNEL); if (!rb_desc->page_va[id]) @@ -1013,7 +1015,6 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size, rb_desc->nr_page_va++; } - desc->nr_cpus++; desc->struct_len += offsetof(struct ring_buffer_desc, page_va); desc->struct_len += struct_size(rb_desc, page_va, rb_desc->nr_page_va); rb_desc = __next_ring_buffer_desc(rb_desc); -- 2.55.0.795.g602f6c329a-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path 2026-07-09 16:00 ` [PATCH v2 1/3] " Vincent Donnefort @ 2026-07-09 23:15 ` Steven Rostedt 2026-07-10 7:58 ` Vincent Donnefort 0 siblings, 1 reply; 9+ messages in thread From: Steven Rostedt @ 2026-07-09 23:15 UTC (permalink / raw) To: Vincent Donnefort Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team, linux-kernel, Sashiko On Thu, 9 Jul 2026 17:00:15 +0100 Vincent Donnefort <vdonnefort@google.com> wrote: > If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus > is not yet incremented for the current CPU. As a consequence, on error, > half-allocated rb_desc will not be freed in trace_remote_free_buffer(). > > Increment desc->nr_cpus as soon as the first allocation for the current > CPU has succeeded. > > Fixes: 96e43537af54 ("tracing: Introduce trace remotes") > Reported-by: Sashiko <sashiko-bot@kernel.org> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com> This patch makes Sashiko find other possible issues with the code :-p https://sashiko.dev/#/patchset/20260709160017.1729517-2-vdonnefort%40google.com -- Steve ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path 2026-07-09 23:15 ` Steven Rostedt @ 2026-07-10 7:58 ` Vincent Donnefort 2026-07-10 8:01 ` Vincent Donnefort 0 siblings, 1 reply; 9+ messages in thread From: Vincent Donnefort @ 2026-07-10 7:58 UTC (permalink / raw) To: Steven Rostedt Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team, linux-kernel, Sashiko On Thu, Jul 09, 2026 at 07:15:16PM -0400, Steven Rostedt wrote: > On Thu, 9 Jul 2026 17:00:15 +0100 > Vincent Donnefort <vdonnefort@google.com> wrote: > > > If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus > > is not yet incremented for the current CPU. As a consequence, on error, > > half-allocated rb_desc will not be freed in trace_remote_free_buffer(). > > > > Increment desc->nr_cpus as soon as the first allocation for the current > > CPU has succeeded. > > > > Fixes: 96e43537af54 ("tracing: Introduce trace remotes") > > Reported-by: Sashiko <sashiko-bot@kernel.org> > > Signed-off-by: Vincent Donnefort <vdonnefort@google.com> > > This patch makes Sashiko find other possible issues with the code :-p > > https://sashiko.dev/#/patchset/20260709160017.1729517-2-vdonnefort%40google.com So one of them is fixed as part of "tracing/remotes: Add printk, dump_on_panic and boot parameters" [1]. I can pull those fixes into this series instead. I'll look at the rest. [1] https://lore.kernel.org/all/20260605163825.1762953-2-vdonnefort@google.com/ > > -- Steve ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path 2026-07-10 7:58 ` Vincent Donnefort @ 2026-07-10 8:01 ` Vincent Donnefort 2026-07-10 11:56 ` Vincent Donnefort 0 siblings, 1 reply; 9+ messages in thread From: Vincent Donnefort @ 2026-07-10 8:01 UTC (permalink / raw) To: Steven Rostedt Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team, linux-kernel, Sashiko On Fri, Jul 10, 2026 at 08:58:51AM +0100, Vincent Donnefort wrote: > On Thu, Jul 09, 2026 at 07:15:16PM -0400, Steven Rostedt wrote: > > On Thu, 9 Jul 2026 17:00:15 +0100 > > Vincent Donnefort <vdonnefort@google.com> wrote: > > > > > If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus > > > is not yet incremented for the current CPU. As a consequence, on error, > > > half-allocated rb_desc will not be freed in trace_remote_free_buffer(). > > > > > > Increment desc->nr_cpus as soon as the first allocation for the current > > > CPU has succeeded. > > > > > > Fixes: 96e43537af54 ("tracing: Introduce trace remotes") > > > Reported-by: Sashiko <sashiko-bot@kernel.org> > > > Signed-off-by: Vincent Donnefort <vdonnefort@google.com> > > > > This patch makes Sashiko find other possible issues with the code :-p > > > > https://sashiko.dev/#/patchset/20260709160017.1729517-2-vdonnefort%40google.com > > So one of them is fixed as part of "tracing/remotes: Add printk, dump_on_panic > and boot parameters" [1]. I can pull those fixes into this series instead. > > I'll look at the rest. Ha yes this other one is real too. However this is in arch/arm64/kvm/ so I'll post it separately to kvmarm. > > [1] https://lore.kernel.org/all/20260605163825.1762953-2-vdonnefort@google.com/ > > > > > -- Steve ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path 2026-07-10 8:01 ` Vincent Donnefort @ 2026-07-10 11:56 ` Vincent Donnefort 2026-07-10 12:37 ` Steven Rostedt 0 siblings, 1 reply; 9+ messages in thread From: Vincent Donnefort @ 2026-07-10 11:56 UTC (permalink / raw) To: Steven Rostedt Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team, linux-kernel, Sashiko On Fri, Jul 10, 2026 at 09:01:41AM +0100, Vincent Donnefort wrote: > On Fri, Jul 10, 2026 at 08:58:51AM +0100, Vincent Donnefort wrote: > > On Thu, Jul 09, 2026 at 07:15:16PM -0400, Steven Rostedt wrote: > > > On Thu, 9 Jul 2026 17:00:15 +0100 > > > Vincent Donnefort <vdonnefort@google.com> wrote: > > > > > > > If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus > > > > is not yet incremented for the current CPU. As a consequence, on error, > > > > half-allocated rb_desc will not be freed in trace_remote_free_buffer(). > > > > > > > > Increment desc->nr_cpus as soon as the first allocation for the current > > > > CPU has succeeded. > > > > > > > > Fixes: 96e43537af54 ("tracing: Introduce trace remotes") > > > > Reported-by: Sashiko <sashiko-bot@kernel.org> > > > > Signed-off-by: Vincent Donnefort <vdonnefort@google.com> > > > > > > This patch makes Sashiko find other possible issues with the code :-p > > > > > > https://sashiko.dev/#/patchset/20260709160017.1729517-2-vdonnefort%40google.com > > > > So one of them is fixed as part of "tracing/remotes: Add printk, dump_on_panic > > and boot parameters" [1]. I can pull those fixes into this series instead. > > > > I'll look at the rest. > > Ha yes this other one is real too. However this is in arch/arm64/kvm/ so I'll > post it separately to kvmarm. Posted here: https://lore.kernel.org/all/20260710114819.2689386-3-vdonnefort@google.com/ Let me know if I should fold [1] into this series. > > > > > [1] https://lore.kernel.org/all/20260605163825.1762953-2-vdonnefort@google.com/ > > > > > > > > -- Steve ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path 2026-07-10 11:56 ` Vincent Donnefort @ 2026-07-10 12:37 ` Steven Rostedt 0 siblings, 0 replies; 9+ messages in thread From: Steven Rostedt @ 2026-07-10 12:37 UTC (permalink / raw) To: Vincent Donnefort Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team, linux-kernel, Sashiko On Fri, 10 Jul 2026 12:56:32 +0100 Vincent Donnefort <vdonnefort@google.com> wrote: > > Ha yes this other one is real too. However this is in arch/arm64/kvm/ so I'll > > post it separately to kvmarm. > > Posted here: https://lore.kernel.org/all/20260710114819.2689386-3-vdonnefort@google.com/ > > Let me know if I should fold [1] into this series. If they are separate, I'll pull this series in now and start testing it. I just wanted to know if these would cause you to rethink this series. -- Steve ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] tracing/remotes: Fix struct_len in trace_remote_alloc_buffer() 2026-07-09 16:00 [PATCH v2 0/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path Vincent Donnefort 2026-07-09 16:00 ` [PATCH v2 1/3] " Vincent Donnefort @ 2026-07-09 16:00 ` Vincent Donnefort 2026-07-09 16:00 ` [PATCH v2 3/3] ring-buffer: Allow sparse CPU masks in ring_buffer_desc() Vincent Donnefort 2 siblings, 0 replies; 9+ messages in thread From: Vincent Donnefort @ 2026-07-09 16:00 UTC (permalink / raw) To: rostedt, mhiramat, linux-trace-kernel Cc: mathieu.desnoyers, kernel-team, linux-kernel, Vincent Donnefort, Sashiko Pre-calculate desc->struct_len up-front in trace_remote_alloc_buffer() with trace_buffer_desc_size() to fix double-counting. While at it, use the accessor __first_ring_buffer_desc(). Fixes: 96e43537af54 ("tracing: Introduce trace remotes") Reported-by: Sashiko <sashiko-bot@kernel.org> Signed-off-by: Vincent Donnefort <vdonnefort@google.com> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c index d48042239d58..0f6ef5c36d84 100644 --- a/kernel/trace/trace_remote.c +++ b/kernel/trace/trace_remote.c @@ -979,27 +979,22 @@ EXPORT_SYMBOL_GPL(trace_remote_free_buffer); int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size, size_t buffer_size, const struct cpumask *cpumask) { + size_t min_desc_size = trace_buffer_desc_size(buffer_size, cpumask_weight(cpumask)); unsigned int nr_pages = max(DIV_ROUND_UP(buffer_size, PAGE_SIZE), 2UL) + 1; - void *desc_end = desc + desc_size; struct ring_buffer_desc *rb_desc; int cpu, ret = -ENOMEM; - if (desc_size < struct_size(desc, __data, 0)) + if (desc_size < min_desc_size) return -EINVAL; desc->nr_cpus = 0; - desc->struct_len = struct_size(desc, __data, 0); + desc->struct_len = min_desc_size; - rb_desc = (struct ring_buffer_desc *)&desc->__data[0]; + rb_desc = __first_ring_buffer_desc(desc); for_each_cpu(cpu, cpumask) { unsigned int id; - if ((void *)rb_desc + struct_size(rb_desc, page_va, nr_pages) > desc_end) { - ret = -EINVAL; - goto err; - } - rb_desc->cpu = cpu; rb_desc->nr_page_va = 0; rb_desc->meta_va = (unsigned long)__get_free_page(GFP_KERNEL); @@ -1015,8 +1010,6 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size, rb_desc->nr_page_va++; } - desc->struct_len += offsetof(struct ring_buffer_desc, page_va); - desc->struct_len += struct_size(rb_desc, page_va, rb_desc->nr_page_va); rb_desc = __next_ring_buffer_desc(rb_desc); } -- 2.55.0.795.g602f6c329a-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] ring-buffer: Allow sparse CPU masks in ring_buffer_desc() 2026-07-09 16:00 [PATCH v2 0/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path Vincent Donnefort 2026-07-09 16:00 ` [PATCH v2 1/3] " Vincent Donnefort 2026-07-09 16:00 ` [PATCH v2 2/3] tracing/remotes: Fix struct_len in trace_remote_alloc_buffer() Vincent Donnefort @ 2026-07-09 16:00 ` Vincent Donnefort 2 siblings, 0 replies; 9+ messages in thread From: Vincent Donnefort @ 2026-07-09 16:00 UTC (permalink / raw) To: rostedt, mhiramat, linux-trace-kernel Cc: mathieu.desnoyers, kernel-team, linux-kernel, Vincent Donnefort, Sashiko No user currently relies on sparse CPU masks, but the descriptor logic already supports them via linear fallback. Remove the arbitrary limitation. Fixes: 2e67fabd8b77 ("ring-buffer: Introduce ring-buffer remotes") Reported-by: Sashiko <sashiko-bot@kernel.org> Signed-off-by: Vincent Donnefort <vdonnefort@google.com> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index 56a328e94395..4e7b4ce7b8a7 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c @@ -2329,10 +2329,7 @@ static struct ring_buffer_desc *ring_buffer_desc(struct trace_buffer_desc *trace size_t len; int i; - if (!trace_desc) - return NULL; - - if (cpu >= trace_desc->nr_cpus) + if (!trace_desc || !trace_desc->nr_cpus) return NULL; end = (struct ring_buffer_desc *)((void *)trace_desc + trace_desc->struct_len); -- 2.55.0.795.g602f6c329a-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-07-10 12:37 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-09 16:00 [PATCH v2 0/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path Vincent Donnefort 2026-07-09 16:00 ` [PATCH v2 1/3] " Vincent Donnefort 2026-07-09 23:15 ` Steven Rostedt 2026-07-10 7:58 ` Vincent Donnefort 2026-07-10 8:01 ` Vincent Donnefort 2026-07-10 11:56 ` Vincent Donnefort 2026-07-10 12:37 ` Steven Rostedt 2026-07-09 16:00 ` [PATCH v2 2/3] tracing/remotes: Fix struct_len in trace_remote_alloc_buffer() Vincent Donnefort 2026-07-09 16:00 ` [PATCH v2 3/3] ring-buffer: Allow sparse CPU masks in ring_buffer_desc() Vincent Donnefort
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox