Re: [PATCH 2/2] tracing/probes: Fix BTF kflag check for anonymous struct member access

[Google]

On Mon, 31 Aug 2026 13:14:29 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> On Tue,  1 Sep 2026 00:06:16 +0900
> "Masami Hiramatsu (Google)" <mhiramat@xxxxxxxxxx> wrote:
> > diff --git a/kernel/trace/trace_btf.h b/kernel/trace/trace_btf.h
> > index 4bc44bc261e6..4bd26bceae23 100644
> > --- a/kernel/trace/trace_btf.h
> > +++ b/kernel/trace/trace_btf.h
> > @@ -8,4 +8,5 @@ const struct btf_param *btf_get_func_param(const struct btf_type *func_proto,
> >  const struct btf_member *btf_find_struct_member(struct btf *btf,
> >  						const struct btf_type *type,
> >  						const char *member_name,
> > -						u32 *anon_offset);
> > +						u32 *anon_offset,
> > +						const struct btf_type **member_type);
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index c4163904ba74..ce2a7bb00d28 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -625,6 +625,7 @@ static int get_bitoffset_of_field(char **pfieldname, const struct btf_type **pty
> >  {
> >  	const struct btf_type *type = *ptype;
> >  	const struct btf_member *field;
> > +	const struct btf_type *mtype = NULL;
> 
> Why initialize mtype to NULL?
> 
> >  	struct btf *btf = ctx_btf(ctx);
> >  	char *fieldname = *pfieldname;
> >  	int bitoffs = 0;
> > @@ -640,7 +641,7 @@ static int get_bitoffset_of_field(char **pfieldname, const struct btf_type **pty
> >  
> >  		anon_offs = 0;
> >  		field = btf_find_struct_member(btf, type, fieldname,
> > -						&anon_offs);
> > +						&anon_offs, &mtype);
> 
> If mtype is not set here, then field would be either an error or NULL.
> 
> >  		if (IS_ERR(field)) {
> >  			trace_probe_log_err(ctx->offset, BAD_BTF_TID);
> >  			return PTR_ERR(field);
> 
> If field is an error or NULL, it exits out early.
> 
> > @@ -653,7 +654,7 @@ static int get_bitoffset_of_field(char **pfieldname, const struct btf_type **pty
> >  		bitoffs += anon_offs;
> >  
> >  		/* Accumulate the bit-offsets of the dot-connected fields */
> > -		if (btf_type_kflag(type)) {
> > +		if (btf_type_kflag(mtype)) {
> 
> mtype should be guaranteed as set here (maybe set to NULL, but set regardless)
> 
> Now, if btf_find_struct_member() did not set mtype, because this is in a
> loop, in a second iteration, mtype would be stale.

Ah, indeed!

> 
> So either set it to NULL at the top of the loop, or don't set it at all.
> 
> Or am I missing something?

OK, let me just drop NULL initialization because it is a kind of hardening.
(Since anon_offs is left uninitialized, mtype = NULL seems somewhat unbalanced.)

Thanks!

> 
> -- Steve
> 
> 
> 
> >  			bitoffs += BTF_MEMBER_BIT_OFFSET(field->offset);
> >  			ctx->last_bitsize = BTF_MEMBER_BITFIELD_SIZE(field->offset);
> >  		} else {
> > @@ -661,11 +662,11 @@ static int get_bitoffset_of_field(char **pfieldname, const struct btf_type **pty
> >  			ctx->last_bitsize = 0;
> >  		}
> >  
> > -			type = btf_type_skip_modifiers(btf, field->type, NULL);
> > -			if (!type) {
> > -				trace_probe_log_err(ctx->offset, BAD_BTF_TID);
> > -				return -EINVAL;
> > -			}
> > +		type = btf_type_skip_modifiers(btf, field->type, NULL);
> > +		if (!type) {
> > +			trace_probe_log_err(ctx->offset, BAD_BTF_TID);
> > +			return -EINVAL;
> > +		}
> >  
> >  		if (next)
> >  			ctx->offset += next - fieldname;
> 


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>