Re: [PATCH v2] tracing: Limit access to parser->buffer when trace_get_user failed

[Steven Rostedt <rostedt@goodmis.org>]

On Wed,  6 Aug 2025 07:01:09 +0000
Pu Lehui <pulehui@huaweicloud.com> wrote:

> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 4283ed4e8f59..138212f4ecde 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -1814,9 +1814,11 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
>  	if (!*ppos)
>  		trace_parser_clear(parser);
>  
> +	parser->fail = false;

This should be set when the parser is initialized.

> +
>  	ret = get_user(ch, ubuf++);
>  	if (ret)
> -		return ret;
> +		goto fail;
>  
>  	read++;
>  	cnt--;
> @@ -1830,7 +1832,7 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
>  		while (cnt && isspace(ch)) {
>  			ret = get_user(ch, ubuf++);
>  			if (ret)
> -				return ret;
> +				goto fail;
>  			read++;
>  			cnt--;
>  		}
> @@ -1848,12 +1850,14 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
>  	while (cnt && !isspace(ch) && ch) {
>  		if (parser->idx < parser->size - 1)
>  			parser->buffer[parser->idx++] = ch;
> -		else
> -			return -EINVAL;
> +		else {
> +			ret = -EINVAL;
> +			goto fail;
> +		}
>  
>  		ret = get_user(ch, ubuf++);
>  		if (ret)
> -			return ret;
> +			goto fail;
>  		read++;
>  		cnt--;
>  	}
> @@ -1868,11 +1872,15 @@ int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
>  		/* Make sure the parsed string always terminates with '\0'. */
>  		parser->buffer[parser->idx] = 0;
>  	} else {
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto fail;
>  	}
>  
>  	*ppos += read;
>  	return read;
> +fail:
> +	parser->fail = true;

Should have a helper function called: trace_parser_fail(parser) and use
that.

-- Steve


> +	return ret;
>  }
>  
>  /* TODO add a seq_buf_to_buffer() */
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 1dbf1d3cf2f1..5072bb25a860 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -1292,6 +1292,7 @@ bool ftrace_event_is_function(struct trace_event_call *call);
>   */
>  struct trace_parser {
>  	bool		cont;
> +	bool		fail;
>  	char		*buffer;
>  	unsigned	idx;
>  	unsigned	size;
> @@ -1299,7 +1300,7 @@ struct trace_parser {
>  
>  static inline bool trace_parser_loaded(struct trace_parser *parser)
>  {
> -	return (parser->idx != 0);
> +	return !parser->fail && parser->idx != 0;
>  }
>  
>  static inline bool trace_parser_cont(struct trace_parser *parser)