Re: [PATCH] tracing: Fix using ret variable in tracing_set_tracer()

[Google]

On Mon, 6 Jan 2025 11:11:43 -0500
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> From: Steven Rostedt <rostedt@xxxxxxxxxxx>
> 
> When the function tracing_set_tracer() switched over to using the guard()
> infrastructure, it did not need to save the 'ret' variable and would just
> return the value when an error arised, instead of setting ret and jumping
> to an out label.
> 
> When CONFIG_TRACER_SNAPSHOT is enabled, it had code that expected the
> "ret" variable to be initialized to zero and had set 'ret' while holding
> an arch_spin_lock() (not used by guard), and then upon releasing the lock
> it would check 'ret' and exit if set. But because ret was only set when an
> error occurred while holding the locks, 'ret' would be used uninitialized
> if there was no error. The code in the CONFIG_TRACER_SNAPSHOT block should
> be self contain. Make sure 'ret' is also set when no error occurred.
> 

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

Thanks,

> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Closes: https://lore.kernel.org/r/202412271654.nJVBuwmF-lkp@xxxxxxxxx/
> Fixes: d33b10c0c73ad ("tracing: Switch trace.c code over to use guard()")
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
>  kernel/trace/trace.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 0aaf442271e9..5aeb898054e7 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -6104,8 +6104,7 @@ int tracing_set_tracer(struct trace_array *tr, const char *buf)
>  	if (t->use_max_tr) {
>  		local_irq_disable();
>  		arch_spin_lock(&tr->max_lock);
> -		if (tr->cond_snapshot)
> -			ret = -EBUSY;
> +		ret = tr->cond_snapshot ? -EBUSY : 0;
>  		arch_spin_unlock(&tr->max_lock);
>  		local_irq_enable();
>  		if (ret)
> -- 
> 2.45.2
> 


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>