Message ID | 20200801170322.75218-7-jolsa@kernel.org |
---|---|
State | Changes Requested |
Delegated to: | BPF Maintainers |
Headers | show |
Series | bpf: Add d_path helper | expand |
On Sat, Aug 1, 2020 at 10:04 AM Jiri Olsa <jolsa@kernel.org> wrote: > > Andrii suggested we can simply jump to again label > instead of making recursion call. > > Suggested-by: Andrii Nakryiko <andriin@fb.com> > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > kernel/bpf/btf.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index bc05a24f7361..0f995038b589 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -3931,14 +3931,13 @@ int btf_struct_access(struct bpf_verifier_log *log, > /* Only allow structure for now, can be relaxed for > * other types later. > */ > - elem_type = btf_type_skip_modifiers(btf_vmlinux, > - array_elem->type, NULL); > - if (!btf_type_is_struct(elem_type)) > + t = btf_type_skip_modifiers(btf_vmlinux, array_elem->type, > + NULL); > + if (!btf_type_is_struct(t)) > goto error; > > - off = (off - moff) % elem_type->size; > - return btf_struct_access(log, elem_type, off, size, atype, > - next_btf_id); > + off = (off - moff) % t->size; > + goto again; Transformation looks good, thanks. So: Acked-by: Andrii Nakryiko <andriin@fb.com> But this '% t->size' makes me wonder what will happen when we have an array of zero-sized structs or multi-dimensional arrays with dimensions of size 0... I.e.: struct {} arr[123]; or int arr[0][0]0]; We should probably be more careful with division here. > > error: > bpf_log(log, "access beyond struct %s at off %u size %u\n", > -- > 2.25.4 >
On Tue, Aug 04, 2020 at 11:12:49PM -0700, Andrii Nakryiko wrote: > On Sat, Aug 1, 2020 at 10:04 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > Andrii suggested we can simply jump to again label > > instead of making recursion call. > > > > Suggested-by: Andrii Nakryiko <andriin@fb.com> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > kernel/bpf/btf.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > > index bc05a24f7361..0f995038b589 100644 > > --- a/kernel/bpf/btf.c > > +++ b/kernel/bpf/btf.c > > @@ -3931,14 +3931,13 @@ int btf_struct_access(struct bpf_verifier_log *log, > > /* Only allow structure for now, can be relaxed for > > * other types later. > > */ > > - elem_type = btf_type_skip_modifiers(btf_vmlinux, > > - array_elem->type, NULL); > > - if (!btf_type_is_struct(elem_type)) > > + t = btf_type_skip_modifiers(btf_vmlinux, array_elem->type, > > + NULL); > > + if (!btf_type_is_struct(t)) > > goto error; > > > > - off = (off - moff) % elem_type->size; > > - return btf_struct_access(log, elem_type, off, size, atype, > > - next_btf_id); > > + off = (off - moff) % t->size; > > + goto again; > > Transformation looks good, thanks. So: > > Acked-by: Andrii Nakryiko <andriin@fb.com> > > But this '% t->size' makes me wonder what will happen when we have an > array of zero-sized structs or multi-dimensional arrays with > dimensions of size 0... I.e.: > > struct {} arr[123]; > > or > > int arr[0][0]0]; > > We should probably be more careful with division here. right, definitely.. I'll send follow up patch for that thanks, jirka
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index bc05a24f7361..0f995038b589 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -3931,14 +3931,13 @@ int btf_struct_access(struct bpf_verifier_log *log, /* Only allow structure for now, can be relaxed for * other types later. */ - elem_type = btf_type_skip_modifiers(btf_vmlinux, - array_elem->type, NULL); - if (!btf_type_is_struct(elem_type)) + t = btf_type_skip_modifiers(btf_vmlinux, array_elem->type, + NULL); + if (!btf_type_is_struct(t)) goto error; - off = (off - moff) % elem_type->size; - return btf_struct_access(log, elem_type, off, size, atype, - next_btf_id); + off = (off - moff) % t->size; + goto again; error: bpf_log(log, "access beyond struct %s at off %u size %u\n",
Andrii suggested we can simply jump to again label instead of making recursion call. Suggested-by: Andrii Nakryiko <andriin@fb.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- kernel/bpf/btf.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)