Re: [PATCH 3/4] tracing/inject: Validate entry allocation size

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 22 Jul 2026 14:10:39 +0800
Li Qiang <liqiang01@kylinos.cn> wrote:

> trace_get_entry_size() calculated the allocation from signed field offsets
> and sizes without validating their sum. A malformed event could use a

What type of malformed event are you talking about?

> negative range or overflow the calculation, allocate too little memory, and
> then write past it while initializing or populating the entry. Events with
> no fields also allocated less than a trace_entry.

What event has no fields?

Note, trace event injection is only for debugging tools that read trace
events. It is in no-way something that should *ever* be used by a
production machine.

Is this being found by AI? If it is, you must specify that.

> 
> Start at sizeof(struct trace_entry), validate each field range, and reserve
> room for the trailing NUL. Propagate sizing errors to parse_entry() before
> it initializes the allocation.

Was an AI agent used to create this?

> 
> Fixes: 6c3edaf9fd6a ("tracing: Introduce trace event injection")
> Cc: stable@vger.kernel.org

No need for the stable Cc. This is not something anyone should care about.
If you want to inject a bad event, by all means, crash your machine!
As I stated, it would be a bug to have this enabled on a production machine.

That said, I'm all for making this more robust.

> Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
> ---
>  kernel/trace/trace_events_inject.c | 31 ++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_inject.c b/kernel/trace/trace_events_inject.c
> index a8f076809db4..b8b141c00d5c 100644
> --- a/kernel/trace/trace_events_inject.c
> +++ b/kernel/trace/trace_events_inject.c
> @@ -135,27 +135,43 @@ parse_field(char *str, struct trace_event_call *call,
>  	return -EINVAL;
>  }
>  
> -static int trace_get_entry_size(struct trace_event_call *call)
> +static int trace_get_entry_size(struct trace_event_call *call, int *entry_size)

Why does entry_size need to be a separate parameter? Just return it, and
have a negative be an error.

>  {
>  	struct ftrace_event_field *field;
>  	struct list_head *head;
> -	int size = 0;
> +	int field_size;
> +	int size = sizeof(struct trace_entry);

Keep the upside-down declarations please.

>  
>  	head = trace_get_fields(call);
>  	list_for_each_entry(field, head, link) {
> -		if (field->size + field->offset > size)
> -			size = field->size + field->offset;
> +		if (field->offset < 0 || field->size < 0 ||

Explain to me how the field->offset or field->size can be less than zero?

> +		    field->size > INT_MAX - field->offset)
> +			return -E2BIG;
> +
> +		field_size = field->size + field->offset;
> +		if (field_size > size)
> +			size = field_size;
>  	}
>  
> -	return size;
> +	/* trace_alloc_entry() reserves an extra NUL byte. */
> +	if (size == INT_MAX)
> +		return -E2BIG;
> +
> +	*entry_size = size;
> +	return 0;
>  }
>  
>  static void *trace_alloc_entry(struct trace_event_call *call, int *size)
>  {
> -	int entry_size = trace_get_entry_size(call);
> +	int entry_size;
>  	struct ftrace_event_field *field;
>  	struct list_head *head;
>  	void *entry = NULL;
> +	int ret;
> +
> +	ret = trace_get_entry_size(call, &entry_size);
> +	if (ret)
> +		return ERR_PTR(ret);
>  
>  	/* We need an extra '\0' at the end. */
>  	entry = kzalloc(entry_size + 1, GFP_KERNEL);
> @@ -202,6 +218,9 @@ static int parse_entry(char *str, struct trace_event_call *call, void **pentry)
>  	int len;
>  
>  	entry = trace_alloc_entry(call, &entry_size);
> +	if (IS_ERR(entry))
> +		return PTR_ERR(entry);

Why this change? trace_alloc_entry() returns NULL on error. The above chunk
broken.

-- Steve


> +
>  	*pentry = entry;
>  	if (!entry)
>  		return -ENOMEM;