On Tue, 22 Sep 2026 04:43:30 +0000
[email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [Medium] trace: incorrect BTF kflag check in get_offset_of_field() allows
> wild offsets
> --
>
> commit 3bc53bf9a42b7270af7bd91985c43e9cafe48496
> Author: Masami Hiramatsu (Google) <[email protected]>
>
> tracing/wprobe: Support BTF struct offset resolution in set_wprobe trigger
>
> This commit allows the set_wprobe trigger to use BTF struct offset
> resolution to specify the target address field.
>
> Link:
> https://lore.kernel.org/all/59637b96946653393a7ad3c7de094094796b39c2.1785067572.git.wangjinchao...@gmail.com/
>
> > diff --git a/kernel/trace/trace_wprobe.c b/kernel/trace/trace_wprobe.c
> > index f1d6aa320f213..121310b72522f 100644
> > --- a/kernel/trace/trace_wprobe.c
> > +++ b/kernel/trace/trace_wprobe.c
>
> [ ... ]
>
> > @@ -1165,6 +1166,213 @@ 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;
> > + const struct btf_type *mtype;
> > + 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, &mtype);
> > + if (IS_ERR_OR_NULL(field))
> > + return -ENOENT;
> > +
> > + if (btf_type_kflag(mtype)) {
>
> [Severity: Medium]
> Is it intentional to check btf_type_kflag() on the member type (mtype)
> instead of the parent struct type (type)?
Oops, I thought I had fixed this issue...
> If the member's kflag is checked instead of the parent struct's kflag,
> bitfield members can bypass the explicit rejection check and compute wildly
> incorrect memory offsets. This happens because field->offset contains the
> bitfield size in the upper 8 bits when the parent struct's kflag is set.
Yeah, kflag is the attribute of the parept structure's type.
Thanks,
>
> > + /* 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;
> > + }
>
> --
> Sashiko AI review ยท
> https://sashiko.dev/#/patchset/179005108298.388919.4535333252892590932.stgit@devnote2?part=13
--
Masami Hiramatsu (Google) <[email protected]>