Re: [PATCH v17 13/13] tracing/wprobe: Support BTF struct offset resolution in set_wprobe trigger

[sashiko-bot@kernel.org]

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) <mhiramat@kernel.org>

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.wangjinchao600@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)?

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.

> +			/* 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