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

[Beau Belgrave <beaub@linux.microsoft.com>]

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