On Mon, 31 Aug 2026 13:14:29 -0400
Steven Rostedt <[email protected]> wrote:

> On Tue,  1 Sep 2026 00:06:16 +0900
> "Masami Hiramatsu (Google)" <[email protected]> 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) <[email protected]>

Reply via email to