All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] tracing: fix histogram and injection buffer overflows
@ 2026-07-22  6:10 Li Qiang
  2026-07-22  6:10 ` [PATCH 1/4] tracing/hist: Prevent overflow in histogram expression strings Li Qiang
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Li Qiang @ 2026-07-22  6:10 UTC (permalink / raw)
  To: rostedt, mhiramat; +Cc: linux-trace-kernel, linux-kernel, Li Qiang

The histogram expression and trace injection paths derive allocation sizes
from unbounded strings or field metadata. This series bounds histogram
formatting, validates user-event field declarations and injection allocation
sizes, and protects dynamic string growth.

The first two patches validate metadata at the producer and consumer
boundaries. The last two tighten trace injection allocation checks, including
events with no user fields.

Li Qiang (4):
  tracing/hist: Prevent overflow in histogram expression strings
  tracing/user_events: Validate explicit struct field sizes
  tracing/inject: Validate entry allocation size
  tracing/inject: Prevent overflow growing string fields

 kernel/trace/trace_events_hist.c   | 89 ++++++++++++++++++++----------
 kernel/trace/trace_events_inject.c | 34 ++++++++++--
 kernel/trace/trace_events_user.c   | 16 ++++--
 3 files changed, 99 insertions(+), 40 deletions(-)


base-commit: b95f03f04d475aa6719d15a636ddf32222d55657
-- 
2.43.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] tracing/hist: Prevent overflow in histogram expression strings
  2026-07-22  6:10 [PATCH 0/4] tracing: fix histogram and injection buffer overflows Li Qiang
@ 2026-07-22  6:10 ` Li Qiang
  2026-07-22 16:32   ` Steven Rostedt
  2026-07-22  6:10 ` [PATCH 2/4] tracing/user_events: Validate explicit struct field sizes Li Qiang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Li Qiang @ 2026-07-22  6:10 UTC (permalink / raw)
  To: rostedt, mhiramat; +Cc: linux-trace-kernel, linux-kernel, Li Qiang, stable

expr_str() builds a histogram expression in a fixed-size
MAX_FILTER_STR_VAL allocation with unbounded strcat() calls.
Synthetic events permit field names longer than that buffer. Constructing
a trigger expression can therefore write past the allocation.

Build the expression with seq_buf. Propagate construction errors to the
parser and reject strings that overflow the fixed-size buffer with -E2BIG.

Fixes: 100719dcef44 ("tracing: Add simple expression support to hist triggers")
Cc: stable@vger.kernel.org
Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
---
 kernel/trace/trace_events_hist.c | 89 +++++++++++++++++++++-----------
 1 file changed, 59 insertions(+), 30 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 82ce492ab268..2b2547546360 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1733,86 +1733,99 @@ static const char *get_hist_field_flags(struct hist_field *hist_field)
 	return flags_str;
 }
 
-static void expr_field_str(struct hist_field *field, char *expr)
+static void expr_field_str(struct hist_field *field, struct seq_buf *s)
 {
 	if (field->flags & HIST_FIELD_FL_VAR_REF) {
 		if (!field->system)
-			strcat(expr, "$");
+			seq_buf_putc(s, '$');
 	} else if (field->flags & HIST_FIELD_FL_CONST) {
 		char str[HIST_CONST_DIGITS_MAX];
 
 		snprintf(str, HIST_CONST_DIGITS_MAX, "%llu", field->constant);
-		strcat(expr, str);
+		seq_buf_puts(s, str);
 	}
 
-	strcat(expr, hist_field_name(field, 0));
+	seq_buf_puts(s, hist_field_name(field, 0));
 
 	if (field->flags && !(field->flags & HIST_FIELD_FL_VAR_REF)) {
 		const char *flags_str = get_hist_field_flags(field);
 
 		if (flags_str) {
-			strcat(expr, ".");
-			strcat(expr, flags_str);
+			seq_buf_putc(s, '.');
+			seq_buf_puts(s, flags_str);
 		}
 	}
 }
 
 static char *expr_str(struct hist_field *field, unsigned int level)
 {
+	struct seq_buf s;
 	char *expr;
+	int ret = 0;
 
 	if (level > 1)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	expr = kzalloc(MAX_FILTER_STR_VAL, GFP_KERNEL);
 	if (!expr)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
+
+	seq_buf_init(&s, expr, MAX_FILTER_STR_VAL);
 
 	if (!field->operands[0]) {
-		expr_field_str(field, expr);
-		return expr;
+		expr_field_str(field, &s);
+		goto out;
 	}
 
 	if (field->operator == FIELD_OP_UNARY_MINUS) {
 		char *subexpr;
 
-		strcat(expr, "-(");
+		seq_buf_puts(&s, "-(");
 		subexpr = expr_str(field->operands[0], ++level);
-		if (!subexpr) {
-			kfree(expr);
-			return NULL;
+		if (IS_ERR(subexpr)) {
+			ret = PTR_ERR(subexpr);
+			goto free;
 		}
-		strcat(expr, subexpr);
-		strcat(expr, ")");
+		seq_buf_puts(&s, subexpr);
+		seq_buf_putc(&s, ')');
 
 		kfree(subexpr);
-
-		return expr;
+		goto out;
 	}
 
-	expr_field_str(field->operands[0], expr);
+	expr_field_str(field->operands[0], &s);
 
 	switch (field->operator) {
 	case FIELD_OP_MINUS:
-		strcat(expr, "-");
+		seq_buf_putc(&s, '-');
 		break;
 	case FIELD_OP_PLUS:
-		strcat(expr, "+");
+		seq_buf_putc(&s, '+');
 		break;
 	case FIELD_OP_DIV:
-		strcat(expr, "/");
+		seq_buf_putc(&s, '/');
 		break;
 	case FIELD_OP_MULT:
-		strcat(expr, "*");
+		seq_buf_putc(&s, '*');
 		break;
 	default:
-		kfree(expr);
-		return NULL;
+		ret = -EINVAL;
+		goto free;
 	}
 
-	expr_field_str(field->operands[1], expr);
+	expr_field_str(field->operands[1], &s);
 
+out:
+	seq_buf_str(&s);
+	if (seq_buf_has_overflowed(&s)) {
+		ret = -E2BIG;
+		goto free;
+	}
 	return expr;
+
+free:
+	kfree(expr);
+	return ERR_PTR(ret);
 }
 
 /*
@@ -2556,6 +2569,7 @@ static struct hist_field *parse_unary(struct hist_trigger_data *hist_data,
 				      char *var_name, unsigned int *n_subexprs)
 {
 	struct hist_field *operand1, *expr = NULL;
+	char *expr_name;
 	unsigned long operand_flags;
 	int ret = 0;
 	char *s;
@@ -2625,7 +2639,12 @@ static struct hist_field *parse_unary(struct hist_trigger_data *hist_data,
 	expr->size = operand1->size;
 	expr->is_signed = operand1->is_signed;
 	expr->operator = FIELD_OP_UNARY_MINUS;
-	expr->name = expr_str(expr, 0);
+	expr_name = expr_str(expr, 0);
+	if (IS_ERR(expr_name)) {
+		ret = PTR_ERR(expr_name);
+		goto free;
+	}
+	expr->name = expr_name;
 	expr->type = kstrdup_const(operand1->type, GFP_KERNEL);
 	if (!expr->type) {
 		ret = -ENOMEM;
@@ -2691,7 +2710,7 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
 	struct hist_field *var1 = NULL, *var2 = NULL;
 	unsigned long operand_flags, operand2_flags;
 	int field_op, ret = -EINVAL;
-	char *sep, *operand1_str;
+	char *expr_name, *sep, *operand1_str;
 	enum hist_field_fn op_fn;
 	bool combine_consts;
 
@@ -2837,7 +2856,12 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
 		destroy_hist_field(operand2, 0);
 		destroy_hist_field(operand1, 0);
 
-		expr->name = expr_str(expr, 0);
+		expr_name = expr_str(expr, 0);
+		if (IS_ERR(expr_name)) {
+			ret = PTR_ERR(expr_name);
+			goto free_expr;
+		}
+		expr->name = expr_name;
 	} else {
 		/* The operand sizes should be the same, so just pick one */
 		expr->size = operand1->size;
@@ -2850,7 +2874,12 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
 			goto free_expr;
 		}
 
-		expr->name = expr_str(expr, 0);
+		expr_name = expr_str(expr, 0);
+		if (IS_ERR(expr_name)) {
+			ret = PTR_ERR(expr_name);
+			goto free_expr;
+		}
+		expr->name = expr_name;
 	}
 
 	return expr;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] tracing/user_events: Validate explicit struct field sizes
  2026-07-22  6:10 [PATCH 0/4] tracing: fix histogram and injection buffer overflows Li Qiang
  2026-07-22  6:10 ` [PATCH 1/4] tracing/hist: Prevent overflow in histogram expression strings Li Qiang
@ 2026-07-22  6:10 ` Li Qiang
  2026-07-22 16:33   ` Steven Rostedt
  2026-07-22  6:10 ` [PATCH 3/4] tracing/inject: Validate entry allocation size Li Qiang
  2026-07-22  6:10 ` [PATCH 4/4] tracing/inject: Prevent overflow growing string fields Li Qiang
  3 siblings, 1 reply; 10+ messages in thread
From: Li Qiang @ 2026-07-22  6:10 UTC (permalink / raw)
  To: rostedt, mhiramat; +Cc: linux-trace-kernel, linux-kernel, Li Qiang, stable

User event declarations permit an explicit size for a struct field. The
parser accumulated that size in an unsigned offset, then assigned the
parsed unsigned value directly to signed field metadata. Oversized
declarations or cumulative offsets could wrap or become invalid signed
values.

Validate an explicit size is representable as int before storing it. Keep
the running offset signed and reject additions exceeding INT_MAX, so
invalid field layouts are rejected during declaration parsing.

Fixes: 7f5a08c79df3 ("user_events: Add minimal support for trace_event into ftrace")
Cc: stable@vger.kernel.org
Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
---
 kernel/trace/trace_events_user.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index 8c82ecb735f4..fd5b3946921c 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c
@@ -1197,10 +1197,12 @@ static int user_event_add_field(struct user_event *user, const char *type,
  * Format: type name [size]
  */
 static int user_event_parse_field(char *field, struct user_event *user,
-				  u32 *offset)
+				  int *offset)
 {
 	char *part, *type, *name;
-	u32 depth = 0, saved_offset = *offset;
+	u32 depth = 0;
+	unsigned int field_size;
+	int saved_offset = *offset;
 	int len, size = -EINVAL;
 	bool is_struct = false;
 
@@ -1261,8 +1263,11 @@ static int user_event_parse_field(char *field, struct user_event *user,
 			if (!is_struct)
 				return -EINVAL;
 
-			if (kstrtou32(part, 10, &size))
+			if (kstrtouint(part, 10, &field_size))
 				return -EINVAL;
+			if (field_size > INT_MAX)
+				return -E2BIG;
+			size = field_size;
 			break;
 		default:
 			return -EINVAL;
@@ -1281,6 +1286,9 @@ static int user_event_parse_field(char *field, struct user_event *user,
 	if (size < 0)
 		return size;
 
+	if (size > INT_MAX - saved_offset)
+		return -E2BIG;
+
 	*offset = saved_offset + size;
 
 	return user_event_add_field(user, type, name, saved_offset, size,
@@ -1290,7 +1298,7 @@ static int user_event_parse_field(char *field, struct user_event *user,
 static int user_event_parse_fields(struct user_event *user, char *args)
 {
 	char *field;
-	u32 offset = sizeof(struct trace_entry);
+	int offset = sizeof(struct trace_entry);
 	int ret = -EINVAL;
 
 	if (args == NULL)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] tracing/inject: Validate entry allocation size
  2026-07-22  6:10 [PATCH 0/4] tracing: fix histogram and injection buffer overflows Li Qiang
  2026-07-22  6:10 ` [PATCH 1/4] tracing/hist: Prevent overflow in histogram expression strings Li Qiang
  2026-07-22  6:10 ` [PATCH 2/4] tracing/user_events: Validate explicit struct field sizes Li Qiang
@ 2026-07-22  6:10 ` Li Qiang
  2026-07-22 19:45   ` Steven Rostedt
  2026-07-22  6:10 ` [PATCH 4/4] tracing/inject: Prevent overflow growing string fields Li Qiang
  3 siblings, 1 reply; 10+ messages in thread
From: Li Qiang @ 2026-07-22  6:10 UTC (permalink / raw)
  To: rostedt, mhiramat; +Cc: linux-trace-kernel, linux-kernel, Li Qiang, stable

trace_get_entry_size() calculated the allocation from signed field offsets
and sizes without validating their sum. A malformed event could use a
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.

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.

Fixes: 6c3edaf9fd6a ("tracing: Introduce trace event injection")
Cc: stable@vger.kernel.org
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)
 {
 	struct ftrace_event_field *field;
 	struct list_head *head;
-	int size = 0;
+	int field_size;
+	int size = sizeof(struct trace_entry);
 
 	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 ||
+		    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);
+
 	*pentry = entry;
 	if (!entry)
 		return -ENOMEM;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] tracing/inject: Prevent overflow growing string fields
  2026-07-22  6:10 [PATCH 0/4] tracing: fix histogram and injection buffer overflows Li Qiang
                   ` (2 preceding siblings ...)
  2026-07-22  6:10 ` [PATCH 3/4] tracing/inject: Validate entry allocation size Li Qiang
@ 2026-07-22  6:10 ` Li Qiang
  2026-07-22 19:51   ` Steven Rostedt
  3 siblings, 1 reply; 10+ messages in thread
From: Li Qiang @ 2026-07-22  6:10 UTC (permalink / raw)
  To: rostedt, mhiramat; +Cc: linux-trace-kernel, linux-kernel, Li Qiang, stable

parse_entry() appends dynamic string data by adding its length to the
current entry size. An oversized input can overflow this signed addition,
cause krealloc() to receive too small a length, and then write beyond it.

Reject a string length that cannot be added to entry_size before growing
the allocation.

Fixes: 6c3edaf9fd6a ("tracing: Introduce trace event injection")
Cc: stable@vger.kernel.org
Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
---
 kernel/trace/trace_events_inject.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/trace/trace_events_inject.c b/kernel/trace/trace_events_inject.c
index b8b141c00d5c..5c551def44f6 100644
--- a/kernel/trace/trace_events_inject.c
+++ b/kernel/trace/trace_events_inject.c
@@ -243,6 +243,9 @@ static int parse_entry(char *str, struct trace_event_call *call, void **pentry)
 				int str_loc = entry_size & 0xffff;
 				u32 *str_item;
 
+				if (str_len > INT_MAX - entry_size)
+					return -E2BIG;
+
 				entry_size += str_len;
 				*pentry = krealloc(entry, entry_size, GFP_KERNEL);
 				if (!*pentry) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] tracing/hist: Prevent overflow in histogram expression strings
  2026-07-22  6:10 ` [PATCH 1/4] tracing/hist: Prevent overflow in histogram expression strings Li Qiang
@ 2026-07-22 16:32   ` Steven Rostedt
  0 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2026-07-22 16:32 UTC (permalink / raw)
  To: Li Qiang; +Cc: mhiramat, linux-trace-kernel, linux-kernel, stable

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

> expr_str() builds a histogram expression in a fixed-size
> MAX_FILTER_STR_VAL allocation with unbounded strcat() calls.
> Synthetic events permit field names longer than that buffer. Constructing
> a trigger expression can therefore write past the allocation.
> 
> Build the expression with seq_buf. Propagate construction errors to the
> parser and reject strings that overflow the fixed-size buffer with -E2BIG.
> 
> Fixes: 100719dcef44 ("tracing: Add simple expression support to hist triggers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Li Qiang <liqiang01@kylinos.cn>

This has been fixed by this:

   https://patch.msgid.link/20260611055945.22348-4-pengpeng@iscas.ac.cn

Since it can only be updated by root, I didn't not add a fixes nor a stable
tag and will be sending it in the next merge window (I haven't updated my
linux-next branch, but will soon)

-- Steve

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] tracing/user_events: Validate explicit struct field sizes
  2026-07-22  6:10 ` [PATCH 2/4] tracing/user_events: Validate explicit struct field sizes Li Qiang
@ 2026-07-22 16:33   ` Steven Rostedt
  2026-07-22 17:27     ` Beau Belgrave
  0 siblings, 1 reply; 10+ messages in thread
From: Steven Rostedt @ 2026-07-22 16:33 UTC (permalink / raw)
  To: Li Qiang, Beau Belgrave
  Cc: mhiramat, linux-trace-kernel, linux-kernel, stable

Beau,

Can you review this?

Thanks,

-- Steve


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

> User event declarations permit an explicit size for a struct field. The
> parser accumulated that size in an unsigned offset, then assigned the
> parsed unsigned value directly to signed field metadata. Oversized
> declarations or cumulative offsets could wrap or become invalid signed
> values.
> 
> Validate an explicit size is representable as int before storing it. Keep
> the running offset signed and reject additions exceeding INT_MAX, so
> invalid field layouts are rejected during declaration parsing.
> 
> Fixes: 7f5a08c79df3 ("user_events: Add minimal support for trace_event into ftrace")
> Cc: stable@vger.kernel.org
> Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
> ---
>  kernel/trace/trace_events_user.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> index 8c82ecb735f4..fd5b3946921c 100644
> --- a/kernel/trace/trace_events_user.c
> +++ b/kernel/trace/trace_events_user.c
> @@ -1197,10 +1197,12 @@ static int user_event_add_field(struct user_event *user, const char *type,
>   * Format: type name [size]
>   */
>  static int user_event_parse_field(char *field, struct user_event *user,
> -				  u32 *offset)
> +				  int *offset)
>  {
>  	char *part, *type, *name;
> -	u32 depth = 0, saved_offset = *offset;
> +	u32 depth = 0;
> +	unsigned int field_size;
> +	int saved_offset = *offset;
>  	int len, size = -EINVAL;
>  	bool is_struct = false;
>  
> @@ -1261,8 +1263,11 @@ static int user_event_parse_field(char *field, struct user_event *user,
>  			if (!is_struct)
>  				return -EINVAL;
>  
> -			if (kstrtou32(part, 10, &size))
> +			if (kstrtouint(part, 10, &field_size))
>  				return -EINVAL;
> +			if (field_size > INT_MAX)
> +				return -E2BIG;
> +			size = field_size;
>  			break;
>  		default:
>  			return -EINVAL;
> @@ -1281,6 +1286,9 @@ static int user_event_parse_field(char *field, struct user_event *user,
>  	if (size < 0)
>  		return size;
>  
> +	if (size > INT_MAX - saved_offset)
> +		return -E2BIG;
> +
>  	*offset = saved_offset + size;
>  
>  	return user_event_add_field(user, type, name, saved_offset, size,
> @@ -1290,7 +1298,7 @@ static int user_event_parse_field(char *field, struct user_event *user,
>  static int user_event_parse_fields(struct user_event *user, char *args)
>  {
>  	char *field;
> -	u32 offset = sizeof(struct trace_entry);
> +	int offset = sizeof(struct trace_entry);
>  	int ret = -EINVAL;
>  
>  	if (args == NULL)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] tracing/user_events: Validate explicit struct field sizes
  2026-07-22 16:33   ` Steven Rostedt
@ 2026-07-22 17:27     ` Beau Belgrave
  0 siblings, 0 replies; 10+ messages in thread
From: Beau Belgrave @ 2026-07-22 17:27 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Li Qiang, mhiramat, linux-trace-kernel, linux-kernel, stable

On Wed, Jul 22, 2026 at 12:33:10PM -0400, Steven Rostedt wrote:
> Beau,
> 
> Can you review this?
> 

Sure thing.

> Thanks,
> 
> -- Steve
> 
> 
> On Wed, 22 Jul 2026 14:10:38 +0800
> Li Qiang <liqiang01@kylinos.cn> wrote:
> 
> > User event declarations permit an explicit size for a struct field. The
> > parser accumulated that size in an unsigned offset, then assigned the
> > parsed unsigned value directly to signed field metadata. Oversized
> > declarations or cumulative offsets could wrap or become invalid signed
> > values.
> > 
> > Validate an explicit size is representable as int before storing it. Keep
> > the running offset signed and reject additions exceeding INT_MAX, so
> > invalid field layouts are rejected during declaration parsing.
> > 

Li Qiang, thanks for looking into this!

> > Fixes: 7f5a08c79df3 ("user_events: Add minimal support for trace_event into ftrace")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
> > ---
> >  kernel/trace/trace_events_user.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> > index 8c82ecb735f4..fd5b3946921c 100644
> > --- a/kernel/trace/trace_events_user.c
> > +++ b/kernel/trace/trace_events_user.c
> > @@ -1197,10 +1197,12 @@ static int user_event_add_field(struct user_event *user, const char *type,
> >   * Format: type name [size]
> >   */
> >  static int user_event_parse_field(char *field, struct user_event *user,
> > -				  u32 *offset)
> > +				  int *offset)
> >  {
> >  	char *part, *type, *name;
> > -	u32 depth = 0, saved_offset = *offset;
> > +	u32 depth = 0;
> > +	unsigned int field_size;
> > +	int saved_offset = *offset;
> >  	int len, size = -EINVAL;
> >  	bool is_struct = false;
> >  
> > @@ -1261,8 +1263,11 @@ static int user_event_parse_field(char *field, struct user_event *user,
> >  			if (!is_struct)
> >  				return -EINVAL;
> >  
> > -			if (kstrtou32(part, 10, &size))
> > +			if (kstrtouint(part, 10, &field_size))
> >  				return -EINVAL;
> > +			if (field_size > INT_MAX)
> > +				return -E2BIG;

We have a hard coded limit on arrays already of 1024 (MAX_FIELD_ARRAY_SIZE).
We could have a much smaller max here if we want to. However, since this
will backport, I'm fine keeping it this larger value in-case someone
really needs this much space for a struct.

> > +			size = field_size;
> >  			break;
> >  		default:
> >  			return -EINVAL;
> > @@ -1281,6 +1286,9 @@ static int user_event_parse_field(char *field, struct user_event *user,
> >  	if (size < 0)
> >  		return size;
> >  
> > +	if (size > INT_MAX - saved_offset)
> > +		return -E2BIG;
> > +
> >  	*offset = saved_offset + size;

We should really use check_add_overflow() here and return -E2BIG if it
fails instead of a hand coded check. Please update utilizing that.

> >  
> >  	return user_event_add_field(user, type, name, saved_offset, size,
> > @@ -1290,7 +1298,7 @@ static int user_event_parse_field(char *field, struct user_event *user,
> >  static int user_event_parse_fields(struct user_event *user, char *args)
> >  {
> >  	char *field;
> > -	u32 offset = sizeof(struct trace_entry);
> > +	int offset = sizeof(struct trace_entry);
> >  	int ret = -EINVAL;
> >  
> >  	if (args == NULL)

Thanks,
-Beau

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] tracing/inject: Validate entry allocation size
  2026-07-22  6:10 ` [PATCH 3/4] tracing/inject: Validate entry allocation size Li Qiang
@ 2026-07-22 19:45   ` Steven Rostedt
  0 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2026-07-22 19:45 UTC (permalink / raw)
  To: Li Qiang; +Cc: mhiramat, linux-trace-kernel, linux-kernel, stable

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;


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] tracing/inject: Prevent overflow growing string fields
  2026-07-22  6:10 ` [PATCH 4/4] tracing/inject: Prevent overflow growing string fields Li Qiang
@ 2026-07-22 19:51   ` Steven Rostedt
  0 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2026-07-22 19:51 UTC (permalink / raw)
  To: Li Qiang; +Cc: mhiramat, linux-trace-kernel, linux-kernel, stable

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

> parse_entry() appends dynamic string data by adding its length to the
> current entry size. An oversized input can overflow this signed addition,
> cause krealloc() to receive too small a length, and then write beyond it.
> 
> Reject a string length that cannot be added to entry_size before growing
> the allocation.
> 
> Fixes: 6c3edaf9fd6a ("tracing: Introduce trace event injection")
> Cc: stable@vger.kernel.org
> Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
> ---
>  kernel/trace/trace_events_inject.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/trace/trace_events_inject.c b/kernel/trace/trace_events_inject.c
> index b8b141c00d5c..5c551def44f6 100644
> --- a/kernel/trace/trace_events_inject.c
> +++ b/kernel/trace/trace_events_inject.c
> @@ -243,6 +243,9 @@ static int parse_entry(char *str, struct trace_event_call *call, void **pentry)
>  				int str_loc = entry_size & 0xffff;
>  				u32 *str_item;
>  
> +				if (str_len > INT_MAX - entry_size)
> +					return -E2BIG;

This is just wrong in so many ways that it shows that you don't understand
the code.

-- Steve

> +
>  				entry_size += str_len;
>  				*pentry = krealloc(entry, entry_size, GFP_KERNEL);
>  				if (!*pentry) {


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-07-22 19:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-22  6:10 [PATCH 0/4] tracing: fix histogram and injection buffer overflows Li Qiang
2026-07-22  6:10 ` [PATCH 1/4] tracing/hist: Prevent overflow in histogram expression strings Li Qiang
2026-07-22 16:32   ` Steven Rostedt
2026-07-22  6:10 ` [PATCH 2/4] tracing/user_events: Validate explicit struct field sizes Li Qiang
2026-07-22 16:33   ` Steven Rostedt
2026-07-22 17:27     ` Beau Belgrave
2026-07-22  6:10 ` [PATCH 3/4] tracing/inject: Validate entry allocation size Li Qiang
2026-07-22 19:45   ` Steven Rostedt
2026-07-22  6:10 ` [PATCH 4/4] tracing/inject: Prevent overflow growing string fields Li Qiang
2026-07-22 19:51   ` Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.