On Wed, 22 Jul 2026 14:10:39 +0800
Li Qiang <[email protected]> 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: [email protected]
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 <[email protected]>
> ---
> 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;
