diff mbox series

[v8,bpf-next,08/13] bpf: Add BTF_SET_START/END macros

Message ID 20200722211223.1055107-9-jolsa@kernel.org
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series bpf: Add d_path helper | expand

Commit Message

Jiri Olsa July 22, 2020, 9:12 p.m. UTC 
Adding support to define sorted set of BTF ID values.

Following defines sorted set of BTF ID values:

  BTF_SET_START(btf_whitelist_d_path)
  BTF_ID(func, vfs_truncate)
  BTF_ID(func, vfs_fallocate)
  BTF_ID(func, dentry_open)
  BTF_ID(func, vfs_getattr)
  BTF_ID(func, filp_close)
  BTF_SET_END(btf_whitelist_d_path)

It defines following 'struct btf_id_set' variable to access
values and count:

  struct btf_id_set btf_whitelist_d_path;

Adding 'allowed' callback to struct bpf_func_proto, to allow
verifier the check on allowed callers.

Adding btf_id_set_contains function, which will be used by
allowed callbacks to verify the caller's BTF ID value is
within allowed set.

Also removing extra '\' in __BTF_ID_LIST macro.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/bpf.h           |  4 ++++
 include/linux/btf_ids.h       | 43 ++++++++++++++++++++++++++++++++++-
 kernel/bpf/btf.c              | 14 ++++++++++++
 kernel/bpf/verifier.c         |  5 ++++
 tools/include/linux/btf_ids.h | 43 ++++++++++++++++++++++++++++++++++-
 5 files changed, 107 insertions(+), 2 deletions(-)

Comments

Andrii Nakryiko July 28, 2020, 7:39 p.m. UTC | #1 
On Wed, Jul 22, 2020 at 2:13 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding support to define sorted set of BTF ID values.
>
> Following defines sorted set of BTF ID values:
>
>   BTF_SET_START(btf_whitelist_d_path)
>   BTF_ID(func, vfs_truncate)
>   BTF_ID(func, vfs_fallocate)
>   BTF_ID(func, dentry_open)
>   BTF_ID(func, vfs_getattr)
>   BTF_ID(func, filp_close)
>   BTF_SET_END(btf_whitelist_d_path)
>
> It defines following 'struct btf_id_set' variable to access
> values and count:
>
>   struct btf_id_set btf_whitelist_d_path;
>
> Adding 'allowed' callback to struct bpf_func_proto, to allow
> verifier the check on allowed callers.
>
> Adding btf_id_set_contains function, which will be used by
> allowed callbacks to verify the caller's BTF ID value is
> within allowed set.
>
> Also removing extra '\' in __BTF_ID_LIST macro.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  include/linux/bpf.h           |  4 ++++
>  include/linux/btf_ids.h       | 43 ++++++++++++++++++++++++++++++++++-
>  kernel/bpf/btf.c              | 14 ++++++++++++
>  kernel/bpf/verifier.c         |  5 ++++
>  tools/include/linux/btf_ids.h | 43 ++++++++++++++++++++++++++++++++++-
>  5 files changed, 107 insertions(+), 2 deletions(-)
>

[...]

> +#define BTF_SET_START(name)                            \
> +__BTF_ID_LIST(name, local)                             \
> +asm(                                                   \
> +".pushsection " BTF_IDS_SECTION ",\"a\";       \n"     \
> +".local __BTF_ID__set__" #name ";              \n"     \
> +"__BTF_ID__set__" #name ":;                    \n"     \
> +".zero 4                                       \n"     \
> +".popsection;                                  \n");
> +
> +#define BTF_SET_END(name)                              \
> +asm(                                                   \
> +".pushsection " BTF_IDS_SECTION ",\"a\";      \n"      \
> +".size __BTF_ID__set__" #name ", .-" #name "  \n"      \
> +".popsection;                                 \n");    \
> +extern struct btf_id_set name;
> +
>  #else

This local symbol assumption will probably at some point bite us.
Yonghong already did global vs static variants for BTF ID list, we'll
end up doing something like that for sets of BTF IDs as well. Let's do
this similarly from the get go.

>
>  #define BTF_ID_LIST(name) static u32 name[5];
>  #define BTF_ID(prefix, name)
>  #define BTF_ID_UNUSED
>  #define BTF_ID_LIST_GLOBAL(name) u32 name[1];
> +#define BTF_SET_START(name) static struct btf_id_set name = { 0 };

nit: this zero is unnecessary and misleading (it's initialized for
only the first member of a struct). Just {} is enough.

> +#define BTF_SET_END(name)
>
>  #endif /* CONFIG_DEBUG_INFO_BTF */
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 562d4453fad3..06714cdda0a9 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -21,6 +21,8 @@
>  #include <linux/btf_ids.h>
>  #include <linux/skmsg.h>
>  #include <linux/perf_event.h>
> +#include <linux/bsearch.h>
> +#include <linux/btf_ids.h>
>  #include <net/sock.h>
>
>  /* BTF (BPF Type Format) is the meta data format which describes
> @@ -4740,3 +4742,15 @@ u32 btf_id(const struct btf *btf)
>  {
>         return btf->id;
>  }
> +
> +static int btf_id_cmp_func(const void *a, const void *b)
> +{
> +       const int *pa = a, *pb = b;
> +
> +       return *pa - *pb;
> +}
> +
> +bool btf_id_set_contains(struct btf_id_set *set, u32 id)
> +{
> +       return bsearch(&id, set->ids, set->cnt, sizeof(int), btf_id_cmp_func) != NULL;

very nit ;) sizeof(__u32)

> +}
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 39922fa07154..49f728c696a9 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4706,6 +4706,11 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>                 return -EINVAL;
>         }
>

[...]
Jiri Olsa July 29, 2020, 11:54 a.m. UTC | #2 
On Tue, Jul 28, 2020 at 12:39:06PM -0700, Andrii Nakryiko wrote:

SNIP

> 
> [...]
> 
> > +#define BTF_SET_START(name)                            \
> > +__BTF_ID_LIST(name, local)                             \
> > +asm(                                                   \
> > +".pushsection " BTF_IDS_SECTION ",\"a\";       \n"     \
> > +".local __BTF_ID__set__" #name ";              \n"     \
> > +"__BTF_ID__set__" #name ":;                    \n"     \
> > +".zero 4                                       \n"     \
> > +".popsection;                                  \n");
> > +
> > +#define BTF_SET_END(name)                              \
> > +asm(                                                   \
> > +".pushsection " BTF_IDS_SECTION ",\"a\";      \n"      \
> > +".size __BTF_ID__set__" #name ", .-" #name "  \n"      \
> > +".popsection;                                 \n");    \
> > +extern struct btf_id_set name;
> > +
> >  #else
> 
> This local symbol assumption will probably at some point bite us.
> Yonghong already did global vs static variants for BTF ID list, we'll
> end up doing something like that for sets of BTF IDs as well. Let's do
> this similarly from the get go.

sure, will add that

> 
> >
> >  #define BTF_ID_LIST(name) static u32 name[5];
> >  #define BTF_ID(prefix, name)
> >  #define BTF_ID_UNUSED
> >  #define BTF_ID_LIST_GLOBAL(name) u32 name[1];
> > +#define BTF_SET_START(name) static struct btf_id_set name = { 0 };
> 
> nit: this zero is unnecessary and misleading (it's initialized for
> only the first member of a struct). Just {} is enough.

ok

> 
> > +#define BTF_SET_END(name)
> >
> >  #endif /* CONFIG_DEBUG_INFO_BTF */
> >
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 562d4453fad3..06714cdda0a9 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -21,6 +21,8 @@
> >  #include <linux/btf_ids.h>
> >  #include <linux/skmsg.h>
> >  #include <linux/perf_event.h>
> > +#include <linux/bsearch.h>
> > +#include <linux/btf_ids.h>
> >  #include <net/sock.h>
> >
> >  /* BTF (BPF Type Format) is the meta data format which describes
> > @@ -4740,3 +4742,15 @@ u32 btf_id(const struct btf *btf)
> >  {
> >         return btf->id;
> >  }
> > +
> > +static int btf_id_cmp_func(const void *a, const void *b)
> > +{
> > +       const int *pa = a, *pb = b;
> > +
> > +       return *pa - *pb;
> > +}
> > +
> > +bool btf_id_set_contains(struct btf_id_set *set, u32 id)
> > +{
> > +       return bsearch(&id, set->ids, set->cnt, sizeof(int), btf_id_cmp_func) != NULL;
> 
> very nit ;) sizeof(__u32)

sure ;-)

thanks,
jirka
diff mbox series

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index c981e258fed3..e5d81c31ed8e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -295,6 +295,7 @@  struct bpf_func_proto {
 						    * for this argument.
 						    */
 	int *ret_btf_id; /* return value btf_id */
+	bool (*allowed)(const struct bpf_prog *prog);
 };
 
 /* bpf_context is intentionally undefined structure. Pointer to bpf_context is
@@ -1787,4 +1788,7 @@  enum bpf_text_poke_type {
 int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
 		       void *addr1, void *addr2);
 
+struct btf_id_set;
+bool btf_id_set_contains(struct btf_id_set *set, u32 id);
+
 #endif /* _LINUX_BPF_H */
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index 4867d549e3c1..840c3e63cc13 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -3,6 +3,11 @@ 
 #ifndef _LINUX_BTF_IDS_H
 #define _LINUX_BTF_IDS_H
 
+struct btf_id_set {
+	u32 cnt;
+	u32 ids[];
+};
+
 #ifdef CONFIG_DEBUG_INFO_BTF
 
 #include <linux/compiler.h> /* for __PASTE */
@@ -62,7 +67,7 @@  asm(							\
 ".pushsection " BTF_IDS_SECTION ",\"a\";       \n"	\
 "." #scope " " #name ";                        \n"	\
 #name ":;                                      \n"	\
-".popsection;                                  \n");	\
+".popsection;                                  \n");
 
 #define BTF_ID_LIST(name)				\
 __BTF_ID_LIST(name, local)				\
@@ -88,12 +93,48 @@  asm(							\
 ".zero 4                                       \n"	\
 ".popsection;                                  \n");
 
+/*
+ * The BTF_SET_START/END macros pair defines sorted list of
+ * BTF IDs plus its members count, with following layout:
+ *
+ * BTF_SET_START(list)
+ * BTF_ID(type1, name1)
+ * BTF_ID(type2, name2)
+ * BTF_SET_END(list)
+ *
+ * __BTF_ID__set__list:
+ * .zero 4
+ * list:
+ * __BTF_ID__type1__name1__3:
+ * .zero 4
+ * __BTF_ID__type2__name2__4:
+ * .zero 4
+ *
+ */
+#define BTF_SET_START(name)				\
+__BTF_ID_LIST(name, local)				\
+asm(							\
+".pushsection " BTF_IDS_SECTION ",\"a\";       \n"	\
+".local __BTF_ID__set__" #name ";              \n"	\
+"__BTF_ID__set__" #name ":;                    \n"	\
+".zero 4                                       \n"	\
+".popsection;                                  \n");
+
+#define BTF_SET_END(name)				\
+asm(							\
+".pushsection " BTF_IDS_SECTION ",\"a\";      \n"	\
+".size __BTF_ID__set__" #name ", .-" #name "  \n"	\
+".popsection;                                 \n");	\
+extern struct btf_id_set name;
+
 #else
 
 #define BTF_ID_LIST(name) static u32 name[5];
 #define BTF_ID(prefix, name)
 #define BTF_ID_UNUSED
 #define BTF_ID_LIST_GLOBAL(name) u32 name[1];
+#define BTF_SET_START(name) static struct btf_id_set name = { 0 };
+#define BTF_SET_END(name)
 
 #endif /* CONFIG_DEBUG_INFO_BTF */
 
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 562d4453fad3..06714cdda0a9 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -21,6 +21,8 @@ 
 #include <linux/btf_ids.h>
 #include <linux/skmsg.h>
 #include <linux/perf_event.h>
+#include <linux/bsearch.h>
+#include <linux/btf_ids.h>
 #include <net/sock.h>
 
 /* BTF (BPF Type Format) is the meta data format which describes
@@ -4740,3 +4742,15 @@  u32 btf_id(const struct btf *btf)
 {
 	return btf->id;
 }
+
+static int btf_id_cmp_func(const void *a, const void *b)
+{
+	const int *pa = a, *pb = b;
+
+	return *pa - *pb;
+}
+
+bool btf_id_set_contains(struct btf_id_set *set, u32 id)
+{
+	return bsearch(&id, set->ids, set->cnt, sizeof(int), btf_id_cmp_func) != NULL;
+}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 39922fa07154..49f728c696a9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4706,6 +4706,11 @@  static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 		return -EINVAL;
 	}
 
+	if (fn->allowed && !fn->allowed(env->prog)) {
+		verbose(env, "helper call is not allowed in probe\n");
+		return -EINVAL;
+	}
+
 	/* With LD_ABS/IND some JITs save/restore skb from r1. */
 	changes_data = bpf_helper_changes_pkt_data(fn->func);
 	if (changes_data && fn->arg1_type != ARG_PTR_TO_CTX) {
diff --git a/tools/include/linux/btf_ids.h b/tools/include/linux/btf_ids.h
index 4867d549e3c1..840c3e63cc13 100644
--- a/tools/include/linux/btf_ids.h
+++ b/tools/include/linux/btf_ids.h
@@ -3,6 +3,11 @@ 
 #ifndef _LINUX_BTF_IDS_H
 #define _LINUX_BTF_IDS_H
 
+struct btf_id_set {
+	u32 cnt;
+	u32 ids[];
+};
+
 #ifdef CONFIG_DEBUG_INFO_BTF
 
 #include <linux/compiler.h> /* for __PASTE */
@@ -62,7 +67,7 @@  asm(							\
 ".pushsection " BTF_IDS_SECTION ",\"a\";       \n"	\
 "." #scope " " #name ";                        \n"	\
 #name ":;                                      \n"	\
-".popsection;                                  \n");	\
+".popsection;                                  \n");
 
 #define BTF_ID_LIST(name)				\
 __BTF_ID_LIST(name, local)				\
@@ -88,12 +93,48 @@  asm(							\
 ".zero 4                                       \n"	\
 ".popsection;                                  \n");
 
+/*
+ * The BTF_SET_START/END macros pair defines sorted list of
+ * BTF IDs plus its members count, with following layout:
+ *
+ * BTF_SET_START(list)
+ * BTF_ID(type1, name1)
+ * BTF_ID(type2, name2)
+ * BTF_SET_END(list)
+ *
+ * __BTF_ID__set__list:
+ * .zero 4
+ * list:
+ * __BTF_ID__type1__name1__3:
+ * .zero 4
+ * __BTF_ID__type2__name2__4:
+ * .zero 4
+ *
+ */
+#define BTF_SET_START(name)				\
+__BTF_ID_LIST(name, local)				\
+asm(							\
+".pushsection " BTF_IDS_SECTION ",\"a\";       \n"	\
+".local __BTF_ID__set__" #name ";              \n"	\
+"__BTF_ID__set__" #name ":;                    \n"	\
+".zero 4                                       \n"	\
+".popsection;                                  \n");
+
+#define BTF_SET_END(name)				\
+asm(							\
+".pushsection " BTF_IDS_SECTION ",\"a\";      \n"	\
+".size __BTF_ID__set__" #name ", .-" #name "  \n"	\
+".popsection;                                 \n");	\
+extern struct btf_id_set name;
+
 #else
 
 #define BTF_ID_LIST(name) static u32 name[5];
 #define BTF_ID(prefix, name)
 #define BTF_ID_UNUSED
 #define BTF_ID_LIST_GLOBAL(name) u32 name[1];
+#define BTF_SET_START(name) static struct btf_id_set name = { 0 };
+#define BTF_SET_END(name)
 
 #endif /* CONFIG_DEBUG_INFO_BTF */