Re: [PATCH 3/5] tracing: Check that the synthetic event and field names are legal

[Masami Hiramatsu]

On Fri,  9 Oct 2020 10:17:09 -0500
Tom Zanussi <zanussi@xxxxxxxxxx> wrote:

> Call the is_good_name() function used by probe events to make sure
> synthetic event and field names don't contain illegal characters and
> cause unexpected parsing of synthetic event commands.
> 

I've tested it. This looks good to me.

/sys/kernel/debug/tracing # echo "myevent char str]" >> synthetic_events 
sh: write error: Invalid argument
/sys/kernel/debug/tracing # echo "myevent char str;[]" >> synthetic_events 
sh: write error: Invalid argument

It works correctly now :)

Reviewed-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
Tested-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>

Thanks!

> Fixes: 4b147936fa50 (tracing: Add support for 'synthetic' events)
> Reported-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> Signed-off-by: Tom Zanussi <zanussi@xxxxxxxxxx>
> ---
>  kernel/trace/trace_events_synth.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
> index b19e2f4159ab..8c9d6e464da0 100644
> --- a/kernel/trace/trace_events_synth.c
> +++ b/kernel/trace/trace_events_synth.c
> @@ -572,6 +572,10 @@ static struct synth_field *parse_synth_field(int argc, const char **argv,
>  		ret = -ENOMEM;
>  		goto free;
>  	}
> +	if (!is_good_name(field->name)) {
> +		ret = -EINVAL;
> +		goto free;
> +	}
>  
>  	if (field_type[0] == ';')
>  		field_type++;
> @@ -1112,6 +1116,11 @@ static int __create_synth_event(int argc, const char *name, const char **argv)
>  
>  	mutex_lock(&event_mutex);
>  
> +	if (!is_good_name(name)) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
>  	event = find_synth_event(name);
>  	if (event) {
>  		ret = -EEXIST;
> -- 
> 2.17.1
> 


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>