Re: [for-linus][PATCH 1/9] tracing/remotes: Fix page_va[] access before counter update in trace_remote_alloc_buffer()

[David Laight]

On Fri, 24 Jul 2026 19:18:41 -0400
Steven Rostedt <rostedt@xxxxxxxxxx> wrote:

> From: Fuad Tabba <fuad.tabba@xxxxxxxxx>
> 
> 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.

Uggg...
Shouldn't the __counted_by() field be the size of the array itself, not the
number of valid items?

	David

> 
> Cc: stable@xxxxxxxxxxxxxxx
> Link: https://patch.msgid.link/20260713072823.2668323-1-fuad.tabba@xxxxxxxxx
> Fixes: 96e43537af546 ("tracing: Introduce trace remotes")
> Signed-off-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>
> Reviewed-by: Vincent Donnefort <vdonnefort@xxxxxxxxxx>
> Tested-by: Vincent Donnefort <vdonnefort@xxxxxxxxxx>
> Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
> ---
>  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 0f6ef5c36d84..ef42d9c38b37 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);
>  	}