Re: [PATCH 3/4] tracing/inject: Validate entry allocation size
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2026-07-22 19:44:49
Also in:
lkml, stable
On Wed, 22 Jul 2026 14:10:39 +0800 Li Qiang [off-list ref] 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.orgNo 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.
quoted hunk ↗ jump to hunk
Signed-off-by: Li Qiang <redacted> --- 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?
quoted hunk ↗ jump to hunk
+ 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;