[PATCH 0/4] tracing: fix histogram and injection buffer overflows

[PATCH 0/4] tracing: fix histogram and injection buffer overflows

[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

[PATCH 1/4] tracing/hist: Prevent overflow in histogram expression strings

[Li Qiang]

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

[PATCH 2/4] tracing/user_events: Validate explicit struct field sizes

[Li Qiang]

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

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

[Li Qiang]

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

[PATCH 4/4] tracing/inject: Prevent overflow growing string fields

[Li Qiang]

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

Re: [PATCH 1/4] tracing/hist: Prevent overflow in histogram expression strings

[Steven Rostedt]

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

Re: [PATCH 2/4] tracing/user_events: Validate explicit struct field sizes

[Steven Rostedt]

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)

Re: [PATCH 2/4] tracing/user_events: Validate explicit struct field sizes

[Beau Belgrave]

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