| Message ID | 20200427201251.2995957-1-yhs@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: implement bpf iterator for kernel data | expand |
On Mon, Apr 27, 2020 at 1:17 PM Yonghong Song <yhs@fb.com> 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@fb.com> > --- > 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 >
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 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; + + 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;
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@fb.com> --- kernel/bpf/btf.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)