From: Vincent Donnefort <vdonnefort@google.com>
To: Fuad Tabba <fuad.tabba@linux.dev>
Cc: rostedt@goodmis.org, mhiramat@kernel.org,
linux-trace-kernel@vger.kernel.org,
mathieu.desnoyers@efficios.com, will@kernel.org,
tabba@google.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] tracing/remotes: Fix page_va[] access before counter update in trace_remote_alloc_buffer()
Date: Mon, 13 Jul 2026 09:21:18 +0100 [thread overview]
Message-ID: <alSf_uhS2Gjt14ZO@google.com> (raw)
In-Reply-To: <20260713072823.2668323-1-fuad.tabba@linux.dev>
On Mon, Jul 13, 2026 at 08:28:23AM +0100, Fuad Tabba wrote:
> page_va[] is annotated __counted_by(nr_page_va), so nr_page_va must
> cover an index before that element is accessed. The allocation loop
> writes page_va[id] while nr_page_va is still id and increments it only
> afterwards, so every write is one element past the declared count.
>
> The store is out of bounds with respect to the annotation: a build with
> CONFIG_UBSAN_BOUNDS on a toolchain that honours __counted_by
> (clang >= 20.1, gcc >= 15.1) flags it as an array-index overflow.
>
> Increment nr_page_va before writing the element it now covers. A failed
> allocation then leaves the slot counted but NULL; the error path frees
> it with free_page(0), which is a no-op.
>
> Fixes: 96e43537af546 ("tracing: Introduce trace remotes")
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
Reviewed-by: Vincent Donnefort <vdonnefort@google.com>
Tested-by: Vincent Donnefort <vdonnefort@google.com>
> ---
> I found this while reviewing Vincent's series [1].
>
> Applies on top of Vincent's "[PATCH v2 0/3] tracing/remotes: Fix leak in
> trace_remote_alloc_buffer() error path" [2], which fixes the other
> issues in this function.
>
> [1] https://lore.kernel.org/all/20260710114819.2689386-1-vdonnefort@google.com/
> [2] https://lore.kernel.org/all/20260709160017.1729517-1-vdonnefort@google.com/
>
> kernel/trace/trace_remote.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> index 0f6ef5c36d84e..ef42d9c38b374 100644
> --- a/kernel/trace/trace_remote.c
> +++ b/kernel/trace/trace_remote.c
> @@ -1004,11 +1004,10 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size,
> desc->nr_cpus++;
>
> for (id = 0; id < nr_pages; id++) {
> + rb_desc->nr_page_va++;
> rb_desc->page_va[id] = (unsigned long)__get_free_page(GFP_KERNEL);
> if (!rb_desc->page_va[id])
> goto err;
> -
> - rb_desc->nr_page_va++;
> }
> rb_desc = __next_ring_buffer_desc(rb_desc);
> }
>
> base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
> prerequisite-patch-id: 8bdd7b28f09e97cff4f5995196b83b377c961d8b
> prerequisite-patch-id: 6781b7ce5156d229fd21a757ef0cd66dd76130cc
> prerequisite-patch-id: 56bcbbdceddf4533770d48d1835d5b7e11f47d56
> --
> 2.39.5
>
prev parent reply other threads:[~2026-07-13 8:21 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-13 7:28 [PATCH v1] tracing/remotes: Fix page_va[] access before counter update in trace_remote_alloc_buffer() Fuad Tabba
2026-07-13 8:21 ` Vincent Donnefort [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alSf_uhS2Gjt14ZO@google.com \
--to=vdonnefort@google.com \
--cc=fuad.tabba@linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=rostedt@goodmis.org \
--cc=tabba@google.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox