linux-kernel - Re: [PATCH] tracing: Reset the function filter after completing trampoline/graph selftest

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Apr 2022 18:58:59 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Li Huafei <lihuafei1@...wei.com>
Cc:     <jolsa@...hat.com>, <mingo@...hat.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tracing: Reset the function filter after completing
 trampoline/graph selftest

On Mon, 18 Apr 2022 15:39:58 +0800
Li Huafei <lihuafei1@...wei.com> wrote:

> The direct trampoline and graph coexistence test sets global_ops to
> trace only 'trace_selftest_dynamic_test_func', but does not reset it
> after the test is completed, resulting in the function filter being set
> already after the system starts. Although it can be reset through the
> tracefs interface, it is more or less confusing to the user, and we
> should reset it to trace all functions after the trampoline/graph test
> completes.
> 
> Fixes: 130c08065848 ("tracing: Add trampoline/graph selftest")
> Signed-off-by: Li Huafei <lihuafei1@...wei.com>
> ---
>  kernel/trace/trace_selftest.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
> index abcadbe933bb..e89f72571f8e 100644
> --- a/kernel/trace/trace_selftest.c
> +++ b/kernel/trace/trace_selftest.c
> @@ -866,12 +866,12 @@ trace_selftest_startup_function_graph(struct tracer *trace,
>  	ret = register_ftrace_direct((unsigned long) DYN_FTRACE_TEST_NAME,
>  				     (unsigned long) trace_direct_tramp);
>  	if (ret)
> -		goto out;
> +		goto reset_filter;
>  
>  	ret = register_ftrace_graph(&fgraph_ops);
>  	if (ret) {
>  		warn_failed_init_tracer(trace, ret);
> -		goto out;
> +		goto reset_filter;
>  	}
>  
>  	DYN_FTRACE_TEST_NAME();
> @@ -887,14 +887,18 @@ trace_selftest_startup_function_graph(struct tracer *trace,
>  	ret = unregister_ftrace_direct((unsigned long) DYN_FTRACE_TEST_NAME,
>  				       (unsigned long) trace_direct_tramp);
>  	if (ret)
> -		goto out;
> +		goto reset_filter;
>  
>  	tracing_start();
>  
>  	if (!ret && !count) {
>  		ret = -1;
> -		goto out;
> +		goto reset_filter;

No need for all the 'goto reset_filter', if this function fails, then the
tracer is disabled, and there's no reason to clear the filter. In fact, it
may cause a crash (because something bad happened).

-- Steve


>  	}
> +
> +reset_filter:
> +	/* Enable tracing on all functions again */
> +	ftrace_set_global_filter(NULL, 0, 1);
>  #endif
>  
>  	/* Don't test dynamic tracing, the function tracer already did */