linux-kernel - Re: [PATCH 2/2] tracing/probes: make match function safe

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]

Message-ID: <Yo1dW2w1UIF+KZFw@home.goodmis.org>
Date:   Tue, 24 May 2022 18:34:03 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Linyu Yuan <quic_linyyuan@...cinc.com>
Cc:     Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Tom Zanussi <zanussi@...nel.org>
Subject: Re: [PATCH 2/2] tracing/probes: make match function safe

Masami and Tom, what are you thoughts on this?

-- Steve

On Sun, May 01, 2022 at 05:34:11PM +0800, Linyu Yuan wrote:
> When delete one kprobe/uprobe/eprobe event entry
> using /sys/kernel/debug/tracing/dynamic_events file,
> it will loop all dynamic envent entries,
> as user will not input dyn_event_operations type,
> when call the match function of kprobe/uprobe/eprobe,
> the dynamic event may have different dyn_event_operations type,
> but currently match function may return a match.
> 
> Fix by check dyn_event_operations type first.
> 
> Signed-off-by: Linyu Yuan <quic_linyyuan@...cinc.com>
> ---
>  kernel/trace/trace_eprobe.c | 31 +++++++++++++++++++++++--------
>  kernel/trace/trace_kprobe.c |  3 +++
>  kernel/trace/trace_uprobe.c |  3 +++
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> index b16e067..0029840 100644
> --- a/kernel/trace/trace_eprobe.c
> +++ b/kernel/trace/trace_eprobe.c
> @@ -19,6 +19,21 @@
>  
>  #define EPROBE_EVENT_SYSTEM "eprobes"
>  
> +static int eprobe_dyn_event_create(const char *raw_command);
> +static int eprobe_dyn_event_show(struct seq_file *m, struct dyn_event *ev);
> +static bool eprobe_dyn_event_is_busy(struct dyn_event *ev);
> +static int eprobe_dyn_event_release(struct dyn_event *ev);
> +static bool eprobe_dyn_event_match(const char *system, const char *event,
> +			int argc, const char **argv, struct dyn_event *ev);
> +
> +static struct dyn_event_operations eprobe_dyn_event_ops = {
> +	.create = eprobe_dyn_event_create,
> +	.show = eprobe_dyn_event_show,
> +	.is_busy = eprobe_dyn_event_is_busy,
> +	.free = eprobe_dyn_event_release,
> +	.match = eprobe_dyn_event_match,
> +};
> +
>  struct trace_eprobe {
>  	/* tracepoint system */
>  	const char *event_system;
> @@ -39,6 +54,11 @@ struct eprobe_data {
>  
>  static int __trace_eprobe_create(int argc, const char *argv[]);
>  
> +static bool is_trace_eprobe(struct dyn_event *ev)
> +{
> +	return ev->ops == &eprobe_dyn_event_ops;
> +}
> +
>  static void trace_event_probe_cleanup(struct trace_eprobe *ep)
>  {
>  	if (!ep)
> @@ -121,6 +141,9 @@ static bool eprobe_dyn_event_match(const char *system, const char *event,
>  	struct trace_eprobe *ep = to_trace_eprobe(ev);
>  	const char *slash;
>  
> +	if (!is_trace_eprobe(ev))
> +		return false;
> +
>  	/*
>  	 * We match the following:
>  	 *  event only			- match all eprobes with event name
> @@ -174,14 +197,6 @@ static bool eprobe_dyn_event_match(const char *system, const char *event,
>  	return trace_probe_match_command_args(&ep->tp, argc, argv);
>  }
>  
> -static struct dyn_event_operations eprobe_dyn_event_ops = {
> -	.create = eprobe_dyn_event_create,
> -	.show = eprobe_dyn_event_show,
> -	.is_busy = eprobe_dyn_event_is_busy,
> -	.free = eprobe_dyn_event_release,
> -	.match = eprobe_dyn_event_match,
> -};
> -
>  static struct trace_eprobe *alloc_event_probe(const char *group,
>  					      const char *this_event,
>  					      struct trace_event_call *event,
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 2cd8ef9..f63abfa 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -163,6 +163,9 @@ static bool trace_kprobe_match(const char *system, const char *event,
>  {
>  	struct trace_kprobe *tk = to_trace_kprobe(ev);
>  
> +	if (!is_trace_kprobe(ev))
> +		return false;
> +
>  	return (event[0] == '\0' ||
>  		strcmp(trace_probe_name(&tk->tp), event) == 0) &&
>  	    (!system || strcmp(trace_probe_group_name(&tk->tp), system) == 0) &&
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index a935c08..ee55ed0 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -312,6 +312,9 @@ static bool trace_uprobe_match(const char *system, const char *event,
>  {
>  	struct trace_uprobe *tu = to_trace_uprobe(ev);
>  
> +	if (!is_trace_uprobe(ev))
> +		return false;
> +
>  	return (event[0] == '\0' ||
>  		strcmp(trace_probe_name(&tu->tp), event) == 0) &&
>  	   (!system || strcmp(trace_probe_group_name(&tu->tp), system) == 0) &&
> -- 
> 2.7.4