linux-kernel - Re: [RFC PATCH] tracing: fix return value in __ftrace_event_enable_disable for TRACE_REG

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: <20250318110700.510f05df7e113180503fc036@kernel.org>
Date: Tue, 18 Mar 2025 11:07:00 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Gabriele Paoloni <gpaoloni@...hat.com>
Cc: rostedt@...dmis.org, mathieu.desnoyers@...icios.com,
 linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] tracing: fix return value in
 __ftrace_event_enable_disable for TRACE_REG_UNREGISTER

On Fri, 14 Mar 2025 13:57:25 +0100
Gabriele Paoloni <gpaoloni@...hat.com> wrote:

> When __ftrace_event_enable_disable invokes the class callback to
> unregister the event, the return value is not reported up to the
> caller, hence leading to event unregister failures being silently
> ignored.
> 
> This patch assigns the ret variable to the invocation of the
> event unregister callback, so that its return value is stored
> and reported to the caller.

Just out of curiosity, have you saw such issue? I think
event unregister should be succeeded or it warns the
fault.

> 
> Signed-off-by: Gabriele Paoloni <gpaoloni@...hat.com>
> ---
> Sending this as RFC since I am not sure if checking the ret
> value is really needed.
> I have been mainly driven by the implementation of
> disable_trace_kprobe, disable_trace_fprobe,
> tracepoint_probe_unregister, disable_trace_eprobe that can
> return an error.
> 
>  kernel/trace/trace_events.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 513de9ceb80e..8d92b271ce0d 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -790,7 +790,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,
>  				clear_bit(EVENT_FILE_FL_RECORDED_TGID_BIT, &file->flags);
>  			}
>  
> -			call->class->reg(call, TRACE_REG_UNREGISTER, file);
> +			ret = call->class->reg(call, TRACE_REG_UNREGISTER, file);

This is not enough. As same as enable failure, this function needs to handle
this error to report it and break.

Thank you,

>  		}
>  		/* If in SOFT_MODE, just set the SOFT_DISABLE_BIT, else clear it */
>  		if (file->flags & EVENT_FILE_FL_SOFT_MODE)
> -- 
> 2.48.1
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>