| Message ID | 20200408232532.2676247-1-yhs@fb.com |
|---|---|
| State | RFC |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: implement bpf based dumping of kernel data structures | expand |
On Wed, Apr 8, 2020 at 4:26 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 | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index d65c6912bdaf..89a0d983b169 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -3837,6 +3837,31 @@ int btf_struct_access(struct bpf_verifier_log *log, > } > > if (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; > + u32 vlen = btf_type_vlen(t); > + u32 last_member_type; > + > + member = btf_type_member(t); > + last_member_type = member[vlen - 1].type; vlen could be zero, and this will be bad > + mtype = btf_type_by_id(btf_vmlinux, last_member_type); might want to strip modifiers here? > + if (!btf_type_is_array(mtype)) > + goto error; > + should probably check that off is >= last_member's offset within a struct? Otherwise access might be spanning previous field and this array? > + array_elem = (struct btf_array *)(mtype + 1); > + if (array_elem->nelems != 0) > + goto error; > + > + elem_type = btf_type_by_id(btf_vmlinux, array_elem->type); strip modifiers > + if (!btf_type_is_struct(elem_type)) > + goto error; > + > + off = (off - t->size) % elem_type->size; I think it will be safer to use field offset, not struct size. Consider example below $ cat test-test.c struct bla { long a; int b; char c[]; }; int main() { static struct bla *x = 0; return 0; } $ pahole -F btf -C bla test-test.o struct bla { long int a; /* 0 8 */ int b; /* 8 4 */ char c[]; /* 12 0 */ /* size: 16, cachelines: 1, members: 3 */ /* padding: 4 */ /* last cacheline: 16 bytes */ }; c is at offset 12, but struct size is 16 due to long alignment. It could be a 4-byte struct instead of char there. > + 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 d65c6912bdaf..89a0d983b169 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -3837,6 +3837,31 @@ int btf_struct_access(struct bpf_verifier_log *log, } if (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; + u32 vlen = btf_type_vlen(t); + u32 last_member_type; + + member = btf_type_member(t); + last_member_type = member[vlen - 1].type; + mtype = btf_type_by_id(btf_vmlinux, last_member_type); + if (!btf_type_is_array(mtype)) + goto error; + + array_elem = (struct btf_array *)(mtype + 1); + if (array_elem->nelems != 0) + goto error; + + elem_type = btf_type_by_id(btf_vmlinux, array_elem->type); + if (!btf_type_is_struct(elem_type)) + goto error; + + off = (off - t->size) % 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 | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)