On Mon, Apr 27, 2020 at 1:17 PM Yonghong Song <yhs@xxxxxx> wrote:
>
> In /proc/net/ipv6_route, we have
> struct fib6_info {
> struct fib6_table *fib6_table;
> ...
> struct fib6_nh fib6_nh[0];
> }
> struct fib6_nh {
> struct fib_nh_common nh_common;
> struct rt6_info **rt6i_pcpu;
> struct rt6_exception_bucket *rt6i_exception_bucket;
> };
> struct fib_nh_common {
> ...
> u8 nhc_gw_family;
> ...
> }
>
> The access:
> struct fib6_nh *fib6_nh = &rt->fib6_nh;
> ... fib6_nh->nh_common.nhc_gw_family ...
>
> This patch ensures such an access is handled properly.
>
> Signed-off-by: Yonghong Song <yhs@xxxxxx>
> ---
> kernel/bpf/btf.c | 33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 2c098e6b1acc..22c69e1d5a56 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -3831,6 +3831,7 @@ int btf_struct_access(struct bpf_verifier_log *log,
> const struct btf_type *mtype, *elem_type = NULL;
> const struct btf_member *member;
> const char *tname, *mname;
> + u32 vlen;
>
> again:
> tname = __btf_name_by_offset(btf_vmlinux, t->name_off);
> @@ -3839,7 +3840,37 @@ int btf_struct_access(struct bpf_verifier_log *log,
> return -EINVAL;
> }
>
> - if (off + size > t->size) {
> + vlen = btf_type_vlen(t);
> + if (vlen > 0 && off + size > t->size) {
if vlen == 0, it will skip this entire check and will eventually go to:
bpf_log(log, "struct %s doesn't have field at offset %d\n", tname, off);
return -EINVAL;
That's probably not right and we are better still reporting:
bpf_log(log, "access beyond struct %s at off %u size %u\n"
tname, off, size);
return -EACCESS;
So this if (vlen > 0) check should be nested in this if?
> + /* If the last element is a variable size array, we may
> + * need to relax the rule.
> + */
> + struct btf_array *array_elem;
> +
> + member = btf_type_member(t) + vlen - 1;
> + mtype = btf_type_skip_modifiers(btf_vmlinux, member->type,
> + NULL);
> + if (!btf_type_is_array(mtype))
> + goto error;
> +
> + array_elem = (struct btf_array *)(mtype + 1);
> + if (array_elem->nelems != 0)
> + goto error;
> +
> + moff = btf_member_bit_offset(t, member) / 8;
> + if (off < moff)
> + goto error;
> +
> + elem_type = btf_type_skip_modifiers(btf_vmlinux,
> + array_elem->type, NULL);
> + if (!btf_type_is_struct(elem_type))
> + goto error;
What about array of primitive types or pointers? Do we want to
explicitly disable such use cases?
> +
> + off = (off - moff) % elem_type->size;
> + return btf_struct_access(log, elem_type, off, size, atype,
> + next_btf_id);
> +
> +error:
> bpf_log(log, "access beyond struct %s at off %u size %u\n",
> tname, off, size);
> return -EACCES;
> --
> 2.24.1
>
