On Sat, 22 Aug 2026 09:51:09 +0000
[email protected] wrote:

> > @@ -990,6 +992,181 @@ static void wprobe_trigger_free(struct 
> > event_trigger_data *data)
> >     }
> >  }
> >  
> > +#ifdef CONFIG_PROBE_EVENTS_BTF_ARGS
> > +
> > +static int get_offset_of_field(struct btf *btf, const struct btf_type 
> > *type, char *field_name)
> > +{
> > +   const struct btf_member *field;
> > +   int bitoffs = 0;
> > +   u32 anon_offs;
> > +   char *next;
> > +
> > +   do {
> > +           next = strchr(field_name, '.');
> > +           if (next)
> > +                   *next++ = '\0';
> > +
> > +           field = btf_find_struct_member(btf, type, field_name, 
> > &anon_offs);
> > +           if (IS_ERR_OR_NULL(field))
> > +                   return -ENOENT;
> > +
> > +           if (btf_type_kflag(type)) {
> 
> [Severity: High]
> Does this code check the kflag of the original parent type instead of the
> anonymous struct that actually contains the field?
> 
> btf_find_struct_member() can return a field from a nested anonymous struct,
> but if the parent struct and the anonymous struct have mismatched BTF kflags
> (for example, the parent has kflag=0 while the anonymous struct has kflag=1
> because it contains bitfields), using the parent's kflag here will cause
> the bitfield size encoded in the upper 8 bits of the offset to be
> misinterpreted as part of the byte offset.

Oops, that's right! btf_find_struct_member() skips anonymous struct and
get the field directly. Hmm, it seems the same issue in the trace_probe.c.
Let me make a fix patch.

> 
> This can add a massively inflated value to bitoffs and result in a wild
> pointer dereference when the wprobe triggers.
> 
> > +                   /* Reject bitfield member access */
> > +                   if (BTF_MEMBER_BITFIELD_SIZE(field->offset))
> > +                           return -EINVAL;
> > +                   bitoffs += anon_offs + 
> > BTF_MEMBER_BIT_OFFSET(field->offset);
> > +           } else {
> > +                   bitoffs += anon_offs + field->offset;
> > +           }
> > +
> > +           field_name = next;
> > +           if (next) {
> > +                   type = btf_type_skip_modifiers(btf, field->type, NULL);
> > +                   if (!type)
> > +                           return -ENOENT;
> > +           }
> > +   } while (next);
> > +   return bitoffs / BITS_PER_BYTE;
> > +}
> 
> [ ... ]
> 
> > +static int wprobe_trigger_typecast_parse(char **field_str_ptr,
> > +                                    struct trace_event_file *file,
> > +                                    struct wprobe_trigger_data 
> > *wprobe_data,
> > +                                    const char *glob)
> > +{
> > +   struct btf *btf __free(btf_put) = NULL;
> > +   const struct btf_type *type;
> > +   char *assign_field;
> > +   char *event_field;
> > +   char *type_field;
> > +   char *type_name;
> > +   char *offs;
> > +   long val = 0;
> > +   int id;
> > +   int adjust;
> > +
> > +   type_name = *field_str_ptr + 1;
> > +   event_field = strchr(type_name, ')');
> > +   if (!event_field) {
> > +           wprobe_trigger_log_err(file, glob, type_name - glob, 
> > DEREF_OPEN_BRACE);
> 
> [Severity: Low]
> Will this log message place the caret out of bounds and hide the user's
> input?
> 
> The glob string passed into wprobe_trigger_log_err() seems to be truncated
> by an earlier strsep() call replacing the delimiter with a null terminator:
> 
> kernel/trace/trace_events_trigger.c:trigger_process_regex() {
>     ...
>     command = strsep(&next, ": \t");
>     ...
> }

Oops, strsep modifies the buffer. OK. let me fix it.

Thanks!


> 
> Since glob is truncated to just the command name, calculating the offset as
> type_name - glob can point far past the end of the printed text, making the
> syntax error logs illegible.
> 
> > +           return -EINVAL;
> > +   }
> 
> -- 
> Sashiko AI review ยท 
> https://sashiko.dev/#/patchset/178739053919.1520941.17662338993878200834.stgit@devnote2?part=12


-- 
Masami Hiramatsu (Google) <[email protected]>

Reply via email to