Re: [RFC PATCH bpf-next 1/7] bpf: Let kfunc sets give kfuncs a BPF body

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpf: integer conversion bypasses kfunc body length check
- [Medium] bpf: type confusion when checking size of pointer arguments
--

commit ab41a09d317c7611ac2e010f734c868245e66554
Author: Yusheng Zheng <yunwei356@gmail.com>

bpf: Let kfunc sets give kfuncs a BPF body

This commit allows a kfunc set to provide a BPF instruction body for its
kfuncs, enabling the verifier to analyze the instructions inline. These
small kfuncs can then be used like instructions while the verifier knows
exactly what they compute.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
[ ... ]
> +static bool btf_check_kfunc_body(const struct btf *btf, const struct btf_type *func,
> +				 const struct bpf_kfunc_body *b)
> +{
> +	const struct btf_type *proto = btf_type_by_id(btf, func->type);
> +	const struct btf_param *args = btf_params(proto);
> +	int i, n = btf_type_vlen(proto), len = b->len;
> +	const struct bpf_insn *insn;
> +	u8 op;
> +
> +	if (n > MAX_BPF_FUNC_REG_ARGS || !b->insns || !len || len > BPF_KFUNC_BODY_MAX_INSNS ||
> +	    (proto->type && !btf_kfunc_reg_type(btf, proto->type)))
> +		return false;

[Severity: Medium]
Does this code inadvertently bypass the bounds check and instruction
validation loop if b->len is very large? 

b->len is an unsigned 32-bit integer. Assigning it to int len
implicitly casts it to a signed integer. If a module registers a kfunc with
b->len >= 0x80000000, len becomes negative. The check
len > BPF_KFUNC_BODY_MAX_INSNS evaluates as false, and the subsequent
for (i = 0; i < len; i++) loop is skipped entirely because
0 < negative_value is false. This could allow invalid or excessively large
BPF instruction sequences to be registered.

> +	for (i = 0; i < n; i++)
> +		if (!btf_kfunc_reg_type(btf, args[i].type) ||
> +		    (btf_param_match_suffix(btf, &args[i], "__k") &&
> +		     btf_type_skip_modifiers(btf, args[i].type, NULL)->size > sizeof(s32)))
> +			return false;

[Severity: Medium]
Does this code check the correct union field for pointer arguments?

When evaluating pointer arguments ending in "__k", this accesses
->size on the returned btf_type. However, for BTF_KIND_PTR, the size
field shares a union with type (the pointee BTF ID). This evaluates the
arbitrary BTF ID of the pointee type instead of its byte size, causing
unpredictable validation results or erroneously rejecting arguments based
on their pointee BTF type ID.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005142219.33451-1-yunwei356@gmail.com?part=1