linux-kernel - Re: [PATCH] tracing: Fix using ret variable in tracing_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20250107073558.7dd7753a0973afb2962ae775@kernel.org>
Date: Tue, 7 Jan 2025 07:35:58 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Linux Trace Kernel
 <linux-trace-kernel@...r.kernel.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Dan Carpenter <dan.carpenter@...aro.org>
Subject: Re: [PATCH] tracing: Fix using ret variable in tracing_set_tracer()

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

> From: Steven Rostedt <rostedt@...dmis.org>
> 
> 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@...nel.org>

Thanks,

> Reported-by: kernel test robot <lkp@...el.com>
> Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> Closes: https://lore.kernel.org/r/202412271654.nJVBuwmF-lkp@intel.com/
> Fixes: d33b10c0c73ad ("tracing: Switch trace.c code over to use guard()")
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
>  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@...nel.org>