Thank you for your contribution! Sashiko AI review found 2 potential issue(s) 
to consider:
- [High] btf_loc_proto_resolve fails to validate the return type location 
parameter (t->type).
- [Medium] Missing strict validation of unused fields and flags in check_meta 
callbacks for new BTF kinds.
--

commit 32008454060b92e68c858ee6b661e8885cb28034
Author: Alan Maguire <[email protected]>

btf: Extend UAPI to support BTF location (inline site) info

This commit adds BTF_KIND_LOC_PARAM, BTF_KIND_LOC_PROTO, and
BTF_KIND_LOCSEC to help represent location information for functions.
It enables representing fully or partially inlined functions at
potentially multiple inline sites.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 9c2cab08bb794..d74c8668aa3f6 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
[ ... ]
> +static s32 btf_loc_param_check_meta(struct btf_verifier_env *env,
> +                                 const struct btf_type *t,
> +                                 u32 meta_left)
> +{
> +     const struct btf_loc_param *p = btf_loc_param(t);
> +     u32 meta_needed;
> +     u32 size;
> +
> +     meta_needed = sizeof(*p) + sizeof(__u32) * btf_vlen(t);

[Severity: Medium]
Should this function strictly validate that p->flags only contains valid
enum bits, and check that btf_type_kflag(t) is 0? 

The BTF verifier typically rejects non-zero unused fields and unknown flags
to ensure forward compatibility. If malformed BTF objects with arbitrary
garbage in these fields are accepted, they might silently alter behavior
or break if these bits are repurposed in the future.

> +     if (meta_left < meta_needed) {
> +             btf_verifier_log_basic(env, t,
> +                                    "meta_left:%u meta_needed:%u",
> +                                   meta_left, meta_needed);
> +             return -EINVAL;
> +     }
[ ... ]
> +static int btf_loc_proto_resolve(struct btf_verifier_env *env,
> +                              const struct resolve_vertex *v)
> +{
> +     const __u32 *params = btf_loc_params(v->t);
> +     u32 i, nr_params = btf_type_vlen(v->t);
> +     struct btf *btf = env->btf;
> +
> +     for (i = 0; i < nr_params; i++) {

[Severity: High]
Does this function need to validate the return type location parameter
encoded in v->t->type? 

It appears to validate the vlen parameters (params[i]) but entirely skips
verifying v->t->type. If a malformed BTF object specifies an out-of-bounds
type ID or an incorrect BTF kind in the t->type field, could it result in
type confusion or out-of-bounds memory accesses when subsequent code trusts
this field?

> +             const struct btf_type *param_type;
> +             u32 param_type_id = params[i];
> +
> +             if (!param_type_id)
> +                     continue;
[ ... ]
> +static s32 btf_locsec_check_meta(struct btf_verifier_env *env,
> +                              const struct btf_type *t,
> +                              u32 meta_left)
> +{
> +     u32 meta_needed;
> +
> +     meta_needed = sizeof(struct btf_loc) * btf_type_vlen(t);

[Severity: Medium]
Similar to the location parameter check, should this callback enforce that
the unused t->size/t->type union and the kflag are 0? Without these
checks, the verifier might accept garbage data in these unused fields which
violates the strict validation design pattern.

> +
> +     if (meta_left < meta_needed) {
> +             btf_verifier_log_basic(env, t,
> +                                    "meta_left:%u meta_needed:%u",
> +                                    meta_left, meta_needed);
> +             return -EINVAL;
> +     }
> +     return meta_needed;
> +}

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to