| Message ID | 20200123152440.28956-2-kpsingh@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | MAC and Audit policy using eBPF (KRSI) | expand |
On Thu, Jan 23, 2020 at 7:25 AM KP Singh <kpsingh@chromium.org> wrote: > > From: KP Singh <kpsingh@google.com> > > - The LSM code does the combination of btf_find_by_name_kind and > btf_type_by_id a couple of times to figure out the BTF type for > security_hook_heads and security_list_options. > - Add an extern for btf_vmlinux in btf.h > > Signed-off-by: KP Singh <kpsingh@google.com> > Reviewed-by: Brendan Jackman <jackmanb@google.com> > Reviewed-by: Florent Revest <revest@google.com> > Reviewed-by: Thomas Garnier <thgarnie@google.com> > --- > include/linux/btf.h | 3 +++ > kernel/bpf/btf.c | 12 ++++++++++++ > 2 files changed, 15 insertions(+) > > diff --git a/include/linux/btf.h b/include/linux/btf.h > index 5c1ea99b480f..d4e859f90a39 100644 > --- a/include/linux/btf.h > +++ b/include/linux/btf.h > @@ -15,6 +15,7 @@ struct btf_type; > union bpf_attr; > > extern const struct file_operations btf_fops; > +extern struct btf *btf_vmlinux; > > void btf_put(struct btf *btf); > int btf_new_fd(const union bpf_attr *attr); > @@ -66,6 +67,8 @@ const struct btf_type * > btf_resolve_size(const struct btf *btf, const struct btf_type *type, > u32 *type_size, const struct btf_type **elem_type, > u32 *total_nelems); > +const struct btf_type *btf_type_by_name_kind( > + struct btf *btf, const char *name, u8 kind); > > #define for_each_member(i, struct_type, member) \ > for (i = 0, member = btf_type_member(struct_type); \ > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 32963b6d5a9c..ea53c16802cb 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -441,6 +441,18 @@ const struct btf_type *btf_type_resolve_func_ptr(const struct btf *btf, > return NULL; > } > > +const struct btf_type *btf_type_by_name_kind( > + struct btf *btf, const char *name, u8 kind) > +{ > + s32 type_id; > + > + type_id = btf_find_by_name_kind(btf, name, kind); > + if (type_id < 0) > + return ERR_PTR(type_id); > + > + return btf_type_by_id(btf, type_id); > +} > + is it worth having this as a separate global function? If btf_find_by_name_kind returns valid ID, then you don't really need to check btf_type_by_id result, it is always going to be valid. So the pattern becomes: type_id = btf_find_by_name_kind(btf, name, kind); if (type_id < 0) goto handle_error; t = btf_type_by_id(btf, type_id); /* now just use t */ which is not much more verbose than: t = btf_type_by_name_kind(btf, name, kind); if (IS_ERR(t)) goto handle_error /* now use t */ > /* Types that act only as a source, not sink or intermediate > * type when resolving. > */ > -- > 2.20.1 >
On 23-Jan 12:06, Andrii Nakryiko wrote: > On Thu, Jan 23, 2020 at 7:25 AM KP Singh <kpsingh@chromium.org> wrote: > > > > From: KP Singh <kpsingh@google.com> > > > > - The LSM code does the combination of btf_find_by_name_kind and > > btf_type_by_id a couple of times to figure out the BTF type for > > security_hook_heads and security_list_options. > > - Add an extern for btf_vmlinux in btf.h > > > > Signed-off-by: KP Singh <kpsingh@google.com> > > Reviewed-by: Brendan Jackman <jackmanb@google.com> > > Reviewed-by: Florent Revest <revest@google.com> > > Reviewed-by: Thomas Garnier <thgarnie@google.com> > > --- > > include/linux/btf.h | 3 +++ > > kernel/bpf/btf.c | 12 ++++++++++++ > > 2 files changed, 15 insertions(+) > > > > diff --git a/include/linux/btf.h b/include/linux/btf.h > > index 5c1ea99b480f..d4e859f90a39 100644 > > --- a/include/linux/btf.h > > +++ b/include/linux/btf.h > > @@ -15,6 +15,7 @@ struct btf_type; > > union bpf_attr; > > > > extern const struct file_operations btf_fops; > > +extern struct btf *btf_vmlinux; > > > > void btf_put(struct btf *btf); > > int btf_new_fd(const union bpf_attr *attr); > > @@ -66,6 +67,8 @@ const struct btf_type * > > btf_resolve_size(const struct btf *btf, const struct btf_type *type, > > u32 *type_size, const struct btf_type **elem_type, > > u32 *total_nelems); > > +const struct btf_type *btf_type_by_name_kind( > > + struct btf *btf, const char *name, u8 kind); > > > > #define for_each_member(i, struct_type, member) \ > > for (i = 0, member = btf_type_member(struct_type); \ > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > > index 32963b6d5a9c..ea53c16802cb 100644 > > --- a/kernel/bpf/btf.c > > +++ b/kernel/bpf/btf.c > > @@ -441,6 +441,18 @@ const struct btf_type *btf_type_resolve_func_ptr(const struct btf *btf, > > return NULL; > > } > > > > +const struct btf_type *btf_type_by_name_kind( > > + struct btf *btf, const char *name, u8 kind) > > +{ > > + s32 type_id; > > + > > + type_id = btf_find_by_name_kind(btf, name, kind); > > + if (type_id < 0) > > + return ERR_PTR(type_id); > > + > > + return btf_type_by_id(btf, type_id); > > +} > > + > > is it worth having this as a separate global function? If > btf_find_by_name_kind returns valid ID, then you don't really need to > check btf_type_by_id result, it is always going to be valid. So the > pattern becomes: Yeah you're right. We went from using this a few times in separate functions to 2 times in the same function (based on the changes in v2 -> v3) I will drop this from the next revision. > type_id = btf_find_by_name_kind(btf, name, kind); > if (type_id < 0) > goto handle_error; > t = btf_type_by_id(btf, type_id); > /* now just use t */ > > which is not much more verbose than: > > t = btf_type_by_name_kind(btf, name, kind); > if (IS_ERR(t)) > goto handle_error > /* now use t */ > Agreed. - KP > > > /* Types that act only as a source, not sink or intermediate > > * type when resolving. > > */ > > -- > > 2.20.1 > >
diff --git a/include/linux/btf.h b/include/linux/btf.h index 5c1ea99b480f..d4e859f90a39 100644 --- a/include/linux/btf.h +++ b/include/linux/btf.h @@ -15,6 +15,7 @@ struct btf_type; union bpf_attr; extern const struct file_operations btf_fops; +extern struct btf *btf_vmlinux; void btf_put(struct btf *btf); int btf_new_fd(const union bpf_attr *attr); @@ -66,6 +67,8 @@ const struct btf_type * btf_resolve_size(const struct btf *btf, const struct btf_type *type, u32 *type_size, const struct btf_type **elem_type, u32 *total_nelems); +const struct btf_type *btf_type_by_name_kind( + struct btf *btf, const char *name, u8 kind); #define for_each_member(i, struct_type, member) \ for (i = 0, member = btf_type_member(struct_type); \ diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 32963b6d5a9c..ea53c16802cb 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -441,6 +441,18 @@ const struct btf_type *btf_type_resolve_func_ptr(const struct btf *btf, return NULL; } +const struct btf_type *btf_type_by_name_kind( + struct btf *btf, const char *name, u8 kind) +{ + s32 type_id; + + type_id = btf_find_by_name_kind(btf, name, kind); + if (type_id < 0) + return ERR_PTR(type_id); + + return btf_type_by_id(btf, type_id); +} + /* Types that act only as a source, not sink or intermediate * type when resolving. */