[bug report] tracing: Have persistent trace instances save module addresses

[bug report] tracing: Have persistent trace instances save module addresses

[Dan Carpenter]

Hello Steven Rostedt,

Commit dca91c1c5468 ("tracing: Have persistent trace instances save
module addresses") from Mar 5, 2025 (linux-next), leads to the
following Smatch static checker warning:

	kernel/trace/trace.c:9422 allocate_trace_buffer()
	error: uninitialized symbol 'scratch_size'.

kernel/trace/trace.c
    9403 static int
    9404 allocate_trace_buffer(struct trace_array *tr, struct array_buffer *buf, int size)
    9405 {
    9406         enum ring_buffer_flags rb_flags;
    9407         struct trace_scratch *tscratch;
    9408         unsigned int scratch_size;
    9409 
    9410         rb_flags = tr->trace_flags & TRACE_ITER_OVERWRITE ? RB_FL_OVERWRITE : 0;
    9411 
    9412         buf->tr = tr;
    9413 
    9414         if (tr->range_addr_start && tr->range_addr_size) {
    9415                 /* Add scratch buffer to handle 128 modules */
    9416                 buf->buffer = ring_buffer_alloc_range(size, rb_flags, 0,
    9417                                                       tr->range_addr_start,
    9418                                                       tr->range_addr_size,
    9419                                                       struct_size(tscratch, entries, 128));
    9420 
    9421                 tscratch = ring_buffer_meta_scratch(buf->buffer, &scratch_size);
--> 9422                 setup_trace_scratch(tr, tscratch, scratch_size);

If ring_buffer_alloc_range() fails then scratch_size is uninitialized.
This is "harmless".  According to the C standard, it's undefined but in
the kernel we would allow it if setup_trace_scratch() is inlined.  Which
it probably will be.  The other thing is that if it's not inline the
UBSan detector will complain about these.

The compiler basically always initializes stack variables to zero these
days so adding an "= 0;" doesn't change runtime at all.

    9423 
    9424                 /*
    9425                  * This is basically the same as a mapped buffer,
    9426                  * with the same restrictions.
    9427                  */
    9428                 tr->mapped++;
    9429         } else {
    9430                 buf->buffer = ring_buffer_alloc(size, rb_flags);
    9431         }
    9432         if (!buf->buffer)
    9433                 return -ENOMEM;
    9434 
    9435         buf->data = alloc_percpu(struct trace_array_cpu);
    9436         if (!buf->data) {
    9437                 ring_buffer_free(buf->buffer);
    9438                 buf->buffer = NULL;
    9439                 return -ENOMEM;
    9440         }
    9441 
    9442         /* Allocate the first page for all buffers */
    9443         set_buffer_entries(&tr->array_buffer,
    9444                            ring_buffer_size(tr->array_buffer.buffer, 0));
    9445 
    9446         return 0;
    9447 }

regards,
dan carpenter

Re: [bug report] tracing: Have persistent trace instances save module addresses

[Steven Rostedt]

On Thu, 13 Mar 2025 11:50:12 +0300
Dan Carpenter <dan.carpenter@linaro.org> wrote:

>     9421                 tscratch = ring_buffer_meta_scratch(buf->buffer, &scratch_size);
> --> 9422                 setup_trace_scratch(tr, tscratch, scratch_size);  
> 
> If ring_buffer_alloc_range() fails then scratch_size is uninitialized.
> This is "harmless".  According to the C standard, it's undefined but in
> the kernel we would allow it if setup_trace_scratch() is inlined.  Which
> it probably will be.  The other thing is that if it's not inline the
> UBSan detector will complain about these.
> 
> The compiler basically always initializes stack variables to zero these
> days so adding an "= 0;" doesn't change runtime at all.

Sure.

-- Steve