[PATCH net-next 0/6] bpf: list all loaded ebpf programs in /proc/bpf/programs

[PATCH net-next 0/6] bpf: list all loaded ebpf programs in /proc/bpf/programs

[Hannes Frederic Sowa]

Right now it seems difficult to list all active ebpf programs in a
system. This new /proc/bpf/programs node should help and print
basically essential information about loaded ebpf programs.

This should help an admin to get a quick look of what is going on in
his system.

Feedback welcome!

Hannes Frederic Sowa (6):
  bpf: bpf_lock needs only block bottom half
  bpf: rename bpf_kallsyms to bpf_progs, ksym_lnode to bpf_progs_head
  bpf: bpf_progs stores all loaded programs
  bpf: track if the bpf program was loaded with SYS_ADMIN capabilities
  bpf: add skeleton for procfs printing of bpf_progs
  bpf: show bpf programs

 include/linux/bpf.h      |   2 +-
 include/linux/filter.h   |  10 ++-
 include/uapi/linux/bpf.h |  32 ++++----
 kernel/bpf/core.c        | 195 +++++++++++++++++++++++++++++++++++++----------
 kernel/bpf/syscall.c     |  11 +--
 kernel/bpf/verifier.c    |   4 +-
 net/core/filter.c        |   6 +-
 7 files changed, 190 insertions(+), 70 deletions(-)

-- 
2.9.3

[PATCH net-next 1/6] bpf: bpf_lock needs only block bottom half

[Hannes Frederic Sowa]

We never modify bpf programs from hardirqs ever.

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 kernel/bpf/core.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index b4f1cb0c5ac710..6f81e0f5a0faa2 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -394,27 +394,23 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
 
 void bpf_prog_kallsyms_add(struct bpf_prog *fp)
 {
-	unsigned long flags;
-
 	if (!bpf_prog_kallsyms_candidate(fp) ||
 	    !capable(CAP_SYS_ADMIN))
 		return;
 
-	spin_lock_irqsave(&bpf_lock, flags);
+	spin_lock_bh(&bpf_lock);
 	bpf_prog_ksym_node_add(fp->aux);
-	spin_unlock_irqrestore(&bpf_lock, flags);
+	spin_unlock_bh(&bpf_lock);
 }
 
 void bpf_prog_kallsyms_del(struct bpf_prog *fp)
 {
-	unsigned long flags;
-
 	if (!bpf_prog_kallsyms_candidate(fp))
 		return;
 
-	spin_lock_irqsave(&bpf_lock, flags);
+	spin_lock_bh(&bpf_lock);
 	bpf_prog_ksym_node_del(fp->aux);
-	spin_unlock_irqrestore(&bpf_lock, flags);
+	spin_unlock_bh(&bpf_lock);
 }
 
 static struct bpf_prog *bpf_prog_kallsyms_find(unsigned long addr)
-- 
2.9.3

[PATCH net-next 2/6] bpf: rename bpf_kallsyms to bpf_progs, ksym_lnode to bpf_progs_head

[Hannes Frederic Sowa]

We will soon put all bpf programs on this list, thus use apropriate names.

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/linux/bpf.h |  2 +-
 kernel/bpf/core.c   | 18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 6bb38d76faf42a..0fbf6a76555cc9 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -172,7 +172,7 @@ struct bpf_prog_aux {
 	u32 used_map_cnt;
 	u32 max_ctx_offset;
 	struct latch_tree_node ksym_tnode;
-	struct list_head ksym_lnode;
+	struct list_head bpf_progs_head;
 	const struct bpf_verifier_ops *ops;
 	struct bpf_map **used_maps;
 	struct bpf_prog *prog;
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 6f81e0f5a0faa2..043f634ff58d87 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -98,7 +98,7 @@ struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
 	fp->aux = aux;
 	fp->aux->prog = fp;
 
-	INIT_LIST_HEAD_RCU(&fp->aux->ksym_lnode);
+	INIT_LIST_HEAD_RCU(&fp->aux->bpf_progs_head);
 
 	return fp;
 }
@@ -360,25 +360,25 @@ static const struct latch_tree_ops bpf_tree_ops = {
 };
 
 static DEFINE_SPINLOCK(bpf_lock);
-static LIST_HEAD(bpf_kallsyms);
+static LIST_HEAD(bpf_progs);
 static struct latch_tree_root bpf_tree __cacheline_aligned;
 
 int bpf_jit_kallsyms __read_mostly;
 
 static void bpf_prog_ksym_node_add(struct bpf_prog_aux *aux)
 {
-	WARN_ON_ONCE(!list_empty(&aux->ksym_lnode));
-	list_add_tail_rcu(&aux->ksym_lnode, &bpf_kallsyms);
+	WARN_ON_ONCE(!list_empty(&aux->bpf_progs_head));
+	list_add_tail_rcu(&aux->bpf_progs_head, &bpf_progs);
 	latch_tree_insert(&aux->ksym_tnode, &bpf_tree, &bpf_tree_ops);
 }
 
 static void bpf_prog_ksym_node_del(struct bpf_prog_aux *aux)
 {
-	if (list_empty(&aux->ksym_lnode))
+	if (list_empty(&aux->bpf_progs_head))
 		return;
 
 	latch_tree_erase(&aux->ksym_tnode, &bpf_tree, &bpf_tree_ops);
-	list_del_rcu(&aux->ksym_lnode);
+	list_del_rcu(&aux->bpf_progs_head);
 }
 
 static bool bpf_prog_kallsyms_candidate(const struct bpf_prog *fp)
@@ -388,8 +388,8 @@ static bool bpf_prog_kallsyms_candidate(const struct bpf_prog *fp)
 
 static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
 {
-	return list_empty(&fp->aux->ksym_lnode) ||
-	       fp->aux->ksym_lnode.prev == LIST_POISON2;
+	return list_empty(&fp->aux->bpf_progs_head) ||
+	       fp->aux->bpf_progs_head.prev == LIST_POISON2;
 }
 
 void bpf_prog_kallsyms_add(struct bpf_prog *fp)
@@ -473,7 +473,7 @@ int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 		return ret;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(aux, &bpf_kallsyms, ksym_lnode) {
+	list_for_each_entry_rcu(aux, &bpf_progs, bpf_progs_head) {
 		if (it++ != symnum)
 			continue;
 
-- 
2.9.3

[PATCH net-next 3/6] bpf: bpf_progs stores all loaded programs

[Hannes Frederic Sowa]

We later want to give users a quick dump of what is possible with procfs,
so store a list of all currently loaded bpf programs. Later this list
will be printed in procfs.

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/linux/filter.h |  4 ++--
 kernel/bpf/core.c      | 51 +++++++++++++++++++++++---------------------------
 kernel/bpf/syscall.c   |  4 ++--
 3 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 9a7786db14fa53..63624c619e371b 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -753,8 +753,8 @@ bpf_address_lookup(unsigned long addr, unsigned long *size,
 	return ret;
 }
 
-void bpf_prog_kallsyms_add(struct bpf_prog *fp);
-void bpf_prog_kallsyms_del(struct bpf_prog *fp);
+void bpf_prog_link(struct bpf_prog *fp);
+void bpf_prog_unlink(struct bpf_prog *fp);
 
 #else /* CONFIG_BPF_JIT */
 
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 043f634ff58d87..2139118258cdf8 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -365,22 +365,6 @@ static struct latch_tree_root bpf_tree __cacheline_aligned;
 
 int bpf_jit_kallsyms __read_mostly;
 
-static void bpf_prog_ksym_node_add(struct bpf_prog_aux *aux)
-{
-	WARN_ON_ONCE(!list_empty(&aux->bpf_progs_head));
-	list_add_tail_rcu(&aux->bpf_progs_head, &bpf_progs);
-	latch_tree_insert(&aux->ksym_tnode, &bpf_tree, &bpf_tree_ops);
-}
-
-static void bpf_prog_ksym_node_del(struct bpf_prog_aux *aux)
-{
-	if (list_empty(&aux->bpf_progs_head))
-		return;
-
-	latch_tree_erase(&aux->ksym_tnode, &bpf_tree, &bpf_tree_ops);
-	list_del_rcu(&aux->bpf_progs_head);
-}
-
 static bool bpf_prog_kallsyms_candidate(const struct bpf_prog *fp)
 {
 	return fp->jited && !bpf_prog_was_classic(fp);
@@ -392,38 +376,45 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
 	       fp->aux->bpf_progs_head.prev == LIST_POISON2;
 }
 
-void bpf_prog_kallsyms_add(struct bpf_prog *fp)
+void bpf_prog_link(struct bpf_prog *fp)
 {
-	if (!bpf_prog_kallsyms_candidate(fp) ||
-	    !capable(CAP_SYS_ADMIN))
-		return;
+	struct bpf_prog_aux *aux = fp->aux;
 
 	spin_lock_bh(&bpf_lock);
-	bpf_prog_ksym_node_add(fp->aux);
+	list_add_tail_rcu(&aux->bpf_progs_head, &bpf_progs);
+	if (bpf_prog_kallsyms_candidate(fp))
+		latch_tree_insert(&aux->ksym_tnode, &bpf_tree, &bpf_tree_ops);
 	spin_unlock_bh(&bpf_lock);
 }
 
-void bpf_prog_kallsyms_del(struct bpf_prog *fp)
+void bpf_prog_unlink(struct bpf_prog *fp)
 {
-	if (!bpf_prog_kallsyms_candidate(fp))
-		return;
+	struct bpf_prog_aux *aux = fp->aux;
 
 	spin_lock_bh(&bpf_lock);
-	bpf_prog_ksym_node_del(fp->aux);
+	list_del_rcu(&aux->bpf_progs_head);
+	if (bpf_prog_kallsyms_candidate(fp))
+		latch_tree_erase(&aux->ksym_tnode, &bpf_tree, &bpf_tree_ops);
 	spin_unlock_bh(&bpf_lock);
 }
 
 static struct bpf_prog *bpf_prog_kallsyms_find(unsigned long addr)
 {
 	struct latch_tree_node *n;
+	struct bpf_prog *prog;
 
 	if (!bpf_jit_kallsyms_enabled())
 		return NULL;
 
 	n = latch_tree_find((void *)addr, &bpf_tree, &bpf_tree_ops);
-	return n ?
-	       container_of(n, struct bpf_prog_aux, ksym_tnode)->prog :
-	       NULL;
+	if (!n)
+		return NULL;
+
+	prog = container_of(n, struct bpf_prog_aux, ksym_tnode)->prog;
+	if (!prog->priv_cap_sys_admin)
+		return NULL;
+
+	return prog;
 }
 
 const char *__bpf_address_lookup(unsigned long addr, unsigned long *size,
@@ -474,6 +465,10 @@ int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(aux, &bpf_progs, bpf_progs_head) {
+		if (!bpf_prog_kallsyms_candidate(aux->prog) ||
+		    !aux->prog->priv_cap_sys_admin)
+			continue;
+
 		if (it++ != symnum)
 			continue;
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 13642c73dca0b4..d61d1bd3e6fee6 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -664,7 +664,7 @@ void bpf_prog_put(struct bpf_prog *prog)
 {
 	if (atomic_dec_and_test(&prog->aux->refcnt)) {
 		trace_bpf_prog_put_rcu(prog);
-		bpf_prog_kallsyms_del(prog);
+		bpf_prog_unlink(prog);
 		call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
 	}
 }
@@ -858,7 +858,7 @@ static int bpf_prog_load(union bpf_attr *attr)
 		/* failed to allocate fd */
 		goto free_used_maps;
 
-	bpf_prog_kallsyms_add(prog);
+	bpf_prog_link(prog);
 	trace_bpf_prog_load(prog, err);
 	return err;
 
-- 
2.9.3

Re: [PATCH net-next 3/6] bpf: bpf_progs stores all loaded programs

[Daniel Borkmann]

[ -daniel@iogearbox.com (wrong address) ]

On 04/26/2017 08:24 PM, Hannes Frederic Sowa wrote:
> We later want to give users a quick dump of what is possible with procfs,
> so store a list of all currently loaded bpf programs. Later this list
> will be printed in procfs.
>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>   include/linux/filter.h |  4 ++--
>   kernel/bpf/core.c      | 51 +++++++++++++++++++++++---------------------------
>   kernel/bpf/syscall.c   |  4 ++--
>   3 files changed, 27 insertions(+), 32 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 9a7786db14fa53..63624c619e371b 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -753,8 +753,8 @@ bpf_address_lookup(unsigned long addr, unsigned long *size,
>   	return ret;
>   }
>
> -void bpf_prog_kallsyms_add(struct bpf_prog *fp);
> -void bpf_prog_kallsyms_del(struct bpf_prog *fp);
> +void bpf_prog_link(struct bpf_prog *fp);
> +void bpf_prog_unlink(struct bpf_prog *fp);
>
>   #else /* CONFIG_BPF_JIT */
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 043f634ff58d87..2139118258cdf8 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -365,22 +365,6 @@ static struct latch_tree_root bpf_tree __cacheline_aligned;
>
>   int bpf_jit_kallsyms __read_mostly;
>
> -static void bpf_prog_ksym_node_add(struct bpf_prog_aux *aux)
> -{
> -	WARN_ON_ONCE(!list_empty(&aux->bpf_progs_head));
> -	list_add_tail_rcu(&aux->bpf_progs_head, &bpf_progs);
> -	latch_tree_insert(&aux->ksym_tnode, &bpf_tree, &bpf_tree_ops);
> -}
> -
> -static void bpf_prog_ksym_node_del(struct bpf_prog_aux *aux)
> -{
> -	if (list_empty(&aux->bpf_progs_head))
> -		return;
> -
> -	latch_tree_erase(&aux->ksym_tnode, &bpf_tree, &bpf_tree_ops);
> -	list_del_rcu(&aux->bpf_progs_head);
> -}
> -
>   static bool bpf_prog_kallsyms_candidate(const struct bpf_prog *fp)
>   {
>   	return fp->jited && !bpf_prog_was_classic(fp);
> @@ -392,38 +376,45 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
>   	       fp->aux->bpf_progs_head.prev == LIST_POISON2;
>   }
>
> -void bpf_prog_kallsyms_add(struct bpf_prog *fp)
> +void bpf_prog_link(struct bpf_prog *fp)
>   {
> -	if (!bpf_prog_kallsyms_candidate(fp) ||
> -	    !capable(CAP_SYS_ADMIN))
> -		return;
> +	struct bpf_prog_aux *aux = fp->aux;
>
>   	spin_lock_bh(&bpf_lock);
> -	bpf_prog_ksym_node_add(fp->aux);
> +	list_add_tail_rcu(&aux->bpf_progs_head, &bpf_progs);
> +	if (bpf_prog_kallsyms_candidate(fp))
> +		latch_tree_insert(&aux->ksym_tnode, &bpf_tree, &bpf_tree_ops);

Hmm, this has the side-effect that it will hook up all progs
to kallsyms (I left out !capable(CAP_SYS_ADMIN) intentionally).

>   	spin_unlock_bh(&bpf_lock);
>   }
>
> -void bpf_prog_kallsyms_del(struct bpf_prog *fp)
> +void bpf_prog_unlink(struct bpf_prog *fp)
>   {
> -	if (!bpf_prog_kallsyms_candidate(fp))
> -		return;
> +	struct bpf_prog_aux *aux = fp->aux;
>
>   	spin_lock_bh(&bpf_lock);
> -	bpf_prog_ksym_node_del(fp->aux);
> +	list_del_rcu(&aux->bpf_progs_head);
> +	if (bpf_prog_kallsyms_candidate(fp))
> +		latch_tree_erase(&aux->ksym_tnode, &bpf_tree, &bpf_tree_ops);
>   	spin_unlock_bh(&bpf_lock);
>   }
>
>   static struct bpf_prog *bpf_prog_kallsyms_find(unsigned long addr)
>   {
>   	struct latch_tree_node *n;
> +	struct bpf_prog *prog;
>
>   	if (!bpf_jit_kallsyms_enabled())
>   		return NULL;
>
>   	n = latch_tree_find((void *)addr, &bpf_tree, &bpf_tree_ops);
> -	return n ?
> -	       container_of(n, struct bpf_prog_aux, ksym_tnode)->prog :
> -	       NULL;
> +	if (!n)
> +		return NULL;
> +
> +	prog = container_of(n, struct bpf_prog_aux, ksym_tnode)->prog;
> +	if (!prog->priv_cap_sys_admin)

Where is this bit defined?

If we return NULL on them anyway, why adding them to the tree
in the first place, just wastes resources on the traversal?

> +		return NULL;
> +
> +	return prog;
>   }
>
>   const char *__bpf_address_lookup(unsigned long addr, unsigned long *size,
> @@ -474,6 +465,10 @@ int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
>
>   	rcu_read_lock();
>   	list_for_each_entry_rcu(aux, &bpf_progs, bpf_progs_head) {
> +		if (!bpf_prog_kallsyms_candidate(aux->prog) ||
> +		    !aux->prog->priv_cap_sys_admin)

Same here.

> +			continue;
> +
>   		if (it++ != symnum)
>   			continue;
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 13642c73dca0b4..d61d1bd3e6fee6 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -664,7 +664,7 @@ void bpf_prog_put(struct bpf_prog *prog)
>   {
>   	if (atomic_dec_and_test(&prog->aux->refcnt)) {
>   		trace_bpf_prog_put_rcu(prog);
> -		bpf_prog_kallsyms_del(prog);
> +		bpf_prog_unlink(prog);
>   		call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
>   	}
>   }
> @@ -858,7 +858,7 @@ static int bpf_prog_load(union bpf_attr *attr)
>   		/* failed to allocate fd */
>   		goto free_used_maps;
>
> -	bpf_prog_kallsyms_add(prog);
> +	bpf_prog_link(prog);
>   	trace_bpf_prog_load(prog, err);
>   	return err;
>
>

[PATCH net-next 4/6] bpf: track if the bpf program was loaded with SYS_ADMIN capabilities

[Hannes Frederic Sowa]

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/linux/filter.h | 6 ++++--
 kernel/bpf/core.c      | 4 +++-
 kernel/bpf/syscall.c   | 7 ++++---
 kernel/bpf/verifier.c  | 4 ++--
 net/core/filter.c      | 6 +++---
 5 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 63624c619e371b..635311f57bf24f 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -413,7 +413,8 @@ struct bpf_prog {
 				locked:1,	/* Program image locked? */
 				gpl_compatible:1, /* Is filter GPL compatible? */
 				cb_access:1,	/* Is control block accessed? */
-				dst_needed:1;	/* Do we need dst entry? */
+				dst_needed:1,	/* Do we need dst entry? */
+				priv_cap_sys_admin:1; /* Where we loaded as sys_admin? */
 	kmemcheck_bitfield_end(meta);
 	enum bpf_prog_type	type;		/* Type of BPF program */
 	u32			len;		/* Number of filter blocks */
@@ -615,7 +616,8 @@ static inline int sk_filter(struct sock *sk, struct sk_buff *skb)
 struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err);
 void bpf_prog_free(struct bpf_prog *fp);
 
-struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags);
+struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags,
+				bool cap_sys_admin);
 struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
 				  gfp_t gfp_extra_flags);
 void __bpf_prog_free(struct bpf_prog *fp);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 2139118258cdf8..048e2d79718a16 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -74,7 +74,8 @@ void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, uns
 	return NULL;
 }
 
-struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
+struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags,
+				bool cap_sys_admin)
 {
 	gfp_t gfp_flags = GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO |
 			  gfp_extra_flags;
@@ -94,6 +95,7 @@ struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
 		return NULL;
 	}
 
+	fp->priv_cap_sys_admin = cap_sys_admin;
 	fp->pages = size / PAGE_SIZE;
 	fp->aux = aux;
 	fp->aux->prog = fp;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index d61d1bd3e6fee6..ed698c17578a49 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -786,7 +786,7 @@ EXPORT_SYMBOL_GPL(bpf_prog_get_type);
 /* last field in 'union bpf_attr' used by this command */
 #define	BPF_PROG_LOAD_LAST_FIELD kern_version
 
-static int bpf_prog_load(union bpf_attr *attr)
+static int bpf_prog_load(union bpf_attr *attr, bool cap_sys_admin)
 {
 	enum bpf_prog_type type = attr->prog_type;
 	struct bpf_prog *prog;
@@ -817,7 +817,8 @@ static int bpf_prog_load(union bpf_attr *attr)
 		return -EPERM;
 
 	/* plain bpf_prog allocation */
-	prog = bpf_prog_alloc(bpf_prog_size(attr->insn_cnt), GFP_USER);
+	prog = bpf_prog_alloc(bpf_prog_size(attr->insn_cnt), GFP_USER,
+			      cap_sys_admin);
 	if (!prog)
 		return -ENOMEM;
 
@@ -1053,7 +1054,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
 		err = map_get_next_key(&attr);
 		break;
 	case BPF_PROG_LOAD:
-		err = bpf_prog_load(&attr);
+		err = bpf_prog_load(&attr, capable(CAP_SYS_ADMIN));
 		break;
 	case BPF_OBJ_PIN:
 		err = bpf_obj_pin(&attr);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6f8b6ed690be93..24c9dac374770f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3488,7 +3488,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
 	if (ret < 0)
 		goto skip_full_check;
 
-	env->allow_ptr_leaks = capable(CAP_SYS_ADMIN);
+	env->allow_ptr_leaks = env->prog->priv_cap_sys_admin;
 
 	ret = do_check(env);
 
@@ -3589,7 +3589,7 @@ int bpf_analyzer(struct bpf_prog *prog, const struct bpf_ext_analyzer_ops *ops,
 	if (ret < 0)
 		goto skip_full_check;
 
-	env->allow_ptr_leaks = capable(CAP_SYS_ADMIN);
+	env->allow_ptr_leaks = prog->priv_cap_sys_admin;
 
 	ret = do_check(env);
 
diff --git a/net/core/filter.c b/net/core/filter.c
index 9a37860a80fc78..dc020d40bb770a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1100,7 +1100,7 @@ int bpf_prog_create(struct bpf_prog **pfp, struct sock_fprog_kern *fprog)
 	if (!bpf_check_basics_ok(fprog->filter, fprog->len))
 		return -EINVAL;
 
-	fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0);
+	fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0, false);
 	if (!fp)
 		return -ENOMEM;
 
@@ -1147,7 +1147,7 @@ int bpf_prog_create_from_user(struct bpf_prog **pfp, struct sock_fprog *fprog,
 	if (!bpf_check_basics_ok(fprog->filter, fprog->len))
 		return -EINVAL;
 
-	fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0);
+	fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0, false);
 	if (!fp)
 		return -ENOMEM;
 
@@ -1249,7 +1249,7 @@ struct bpf_prog *__get_filter(struct sock_fprog *fprog, struct sock *sk)
 	if (!bpf_check_basics_ok(fprog->filter, fprog->len))
 		return ERR_PTR(-EINVAL);
 
-	prog = bpf_prog_alloc(bpf_prog_size(fprog->len), 0);
+	prog = bpf_prog_alloc(bpf_prog_size(fprog->len), 0, false);
 	if (!prog)
 		return ERR_PTR(-ENOMEM);
 
-- 
2.9.3

Re: [PATCH net-next 4/6] bpf: track if the bpf program was loaded with SYS_ADMIN capabilities

[Daniel Borkmann]

On 04/26/2017 08:24 PM, Hannes Frederic Sowa wrote:
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Ahh, looks this got swapped with 3/6.

> ---
>   include/linux/filter.h | 6 ++++--
>   kernel/bpf/core.c      | 4 +++-
>   kernel/bpf/syscall.c   | 7 ++++---
>   kernel/bpf/verifier.c  | 4 ++--
>   net/core/filter.c      | 6 +++---
>   5 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 63624c619e371b..635311f57bf24f 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -413,7 +413,8 @@ struct bpf_prog {
>   				locked:1,	/* Program image locked? */
>   				gpl_compatible:1, /* Is filter GPL compatible? */
>   				cb_access:1,	/* Is control block accessed? */
> -				dst_needed:1;	/* Do we need dst entry? */
> +				dst_needed:1,	/* Do we need dst entry? */
> +				priv_cap_sys_admin:1; /* Where we loaded as sys_admin? */
>   	kmemcheck_bitfield_end(meta);
>   	enum bpf_prog_type	type;		/* Type of BPF program */
[...]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 6f8b6ed690be93..24c9dac374770f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3488,7 +3488,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
>   	if (ret < 0)
>   		goto skip_full_check;
>
> -	env->allow_ptr_leaks = capable(CAP_SYS_ADMIN);
> +	env->allow_ptr_leaks = env->prog->priv_cap_sys_admin;
>
>   	ret = do_check(env);
>
> @@ -3589,7 +3589,7 @@ int bpf_analyzer(struct bpf_prog *prog, const struct bpf_ext_analyzer_ops *ops,
>   	if (ret < 0)
>   		goto skip_full_check;
>
> -	env->allow_ptr_leaks = capable(CAP_SYS_ADMIN);
> +	env->allow_ptr_leaks = prog->priv_cap_sys_admin;
>
>   	ret = do_check(env);
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9a37860a80fc78..dc020d40bb770a 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1100,7 +1100,7 @@ int bpf_prog_create(struct bpf_prog **pfp, struct sock_fprog_kern *fprog)
>   	if (!bpf_check_basics_ok(fprog->filter, fprog->len))
>   		return -EINVAL;
>
> -	fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0);
> +	fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0, false);
>   	if (!fp)
>   		return -ENOMEM;
>

Did you check that transferring allow_ptr_leaks doesn't have a side
effect on the nfp JIT? I believe it can also do cbpf migrations to
a certain extend.

Re: [PATCH net-next 4/6] bpf: track if the bpf program was loaded with SYS_ADMIN capabilities

[Hannes Frederic Sowa]

On 26.04.2017 23:04, Daniel Borkmann wrote:
> On 04/26/2017 08:24 PM, Hannes Frederic Sowa wrote:
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 9a37860a80fc78..dc020d40bb770a 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -1100,7 +1100,7 @@ int bpf_prog_create(struct bpf_prog **pfp,
>> struct sock_fprog_kern *fprog)
>>       if (!bpf_check_basics_ok(fprog->filter, fprog->len))
>>           return -EINVAL;
>>
>> -    fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0);
>> +    fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0, false);
>>       if (!fp)
>>           return -ENOMEM;
>>
> 
> Did you check that transferring allow_ptr_leaks doesn't have a side
> effect on the nfp JIT? I believe it can also do cbpf migrations to
> a certain extend.

Initially I grepped allow_ptr_leaks usages and didn't see it. I just
looked through the code path and didn't see how it could have an impact.
Also, cbpf programs shouldn't depend on allow_ptr_leak to the best of my
knowledge, no?

Thanks,
Hannes

Re: [PATCH net-next 4/6] bpf: track if the bpf program was loaded with SYS_ADMIN capabilities

[Alexei Starovoitov]

On Wed, Apr 26, 2017 at 08:24:17PM +0200, Hannes Frederic Sowa wrote:
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  include/linux/filter.h | 6 ++++--
>  kernel/bpf/core.c      | 4 +++-
>  kernel/bpf/syscall.c   | 7 ++++---
>  kernel/bpf/verifier.c  | 4 ++--
>  net/core/filter.c      | 6 +++---
>  5 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 63624c619e371b..635311f57bf24f 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -413,7 +413,8 @@ struct bpf_prog {
>  				locked:1,	/* Program image locked? */
>  				gpl_compatible:1, /* Is filter GPL compatible? */
>  				cb_access:1,	/* Is control block accessed? */
> -				dst_needed:1;	/* Do we need dst entry? */
> +				dst_needed:1,	/* Do we need dst entry? */
> +				priv_cap_sys_admin:1; /* Where we loaded as sys_admin? */

This is no go.
You didn't provide any explanation whatsoever why you want to see this boolean value.

Re: [PATCH net-next 4/6] bpf: track if the bpf program was loaded with SYS_ADMIN capabilities

[Hannes Frederic Sowa]

Hi,

On 26.04.2017 23:08, Alexei Starovoitov wrote:
> On Wed, Apr 26, 2017 at 08:24:17PM +0200, Hannes Frederic Sowa wrote:
>> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> ---
>>  include/linux/filter.h | 6 ++++--
>>  kernel/bpf/core.c      | 4 +++-
>>  kernel/bpf/syscall.c   | 7 ++++---
>>  kernel/bpf/verifier.c  | 4 ++--
>>  net/core/filter.c      | 6 +++---
>>  5 files changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index 63624c619e371b..635311f57bf24f 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> @@ -413,7 +413,8 @@ struct bpf_prog {
>>  				locked:1,	/* Program image locked? */
>>  				gpl_compatible:1, /* Is filter GPL compatible? */
>>  				cb_access:1,	/* Is control block accessed? */
>> -				dst_needed:1;	/* Do we need dst entry? */
>> +				dst_needed:1,	/* Do we need dst entry? */
>> +				priv_cap_sys_admin:1; /* Where we loaded as sys_admin? */
> 
> This is no go.
> You didn't provide any explanation whatsoever why you want to see this boolean value.

Sorry, should be in the description and will be added if this patch
series is considered to be worthwhile to pursue.

cap_sys_admin influences the verifier a lot in terms which programs are
accepted and which are not. So during investigations it might be even
interesting if the bpf program required those special flags or if the
same program could be loaded just as underprivileged.

I also have to review if we can/should attach cap_sys_admin verified
programs as unprivileged user. It should stop to install a ptr leaking
bpf program as ordinary user, even if one got the file descriptor, no?

Bye,
Hannes

Re: [PATCH net-next 4/6] bpf: track if the bpf program was loaded with SYS_ADMIN capabilities

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]

Hi Hannes,

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Hannes-Frederic-Sowa/bpf-list-all-loaded-ebpf-programs-in-proc-bpf-programs/20170427-090839
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   lib/test_bpf.c: In function 'generate_filter':
>> lib/test_bpf.c:5613:8: error: too few arguments to function 'bpf_prog_alloc'
      fp = bpf_prog_alloc(bpf_prog_size(flen), 0);
           ^~~~~~~~~~~~~~
   In file included from lib/test_bpf.c:20:0:
   include/linux/filter.h:619:18: note: declared here
    struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags,
                     ^~~~~~~~~~~~~~

vim +/bpf_prog_alloc +5613 lib/test_bpf.c

10f18e0b Daniel Borkmann    2014-05-23  5607  				*err, fprog.len);
10f18e0b Daniel Borkmann    2014-05-23  5608  			return NULL;
64a8946b Alexei Starovoitov 2014-05-08  5609  		}
10f18e0b Daniel Borkmann    2014-05-23  5610  		break;
10f18e0b Daniel Borkmann    2014-05-23  5611  
10f18e0b Daniel Borkmann    2014-05-23  5612  	case INTERNAL:
60a3b225 Daniel Borkmann    2014-09-02 @5613  		fp = bpf_prog_alloc(bpf_prog_size(flen), 0);
10f18e0b Daniel Borkmann    2014-05-23  5614  		if (fp == NULL) {
10f18e0b Daniel Borkmann    2014-05-23  5615  			pr_cont("UNEXPECTED_FAIL no memory left\n");
10f18e0b Daniel Borkmann    2014-05-23  5616  			*err = -ENOMEM;

:::::: The code at line 5613 was first introduced by commit
:::::: 60a3b2253c413cf601783b070507d7dd6620c954 net: bpf: make eBPF interpreter images read-only

:::::: TO: Daniel Borkmann <dborkman@redhat.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38891 bytes --]

Re: [PATCH net-next 4/6] bpf: track if the bpf program was loaded with SYS_ADMIN capabilities

[kbuild test robot]

Hi Hannes,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Hannes-Frederic-Sowa/bpf-list-all-loaded-ebpf-programs-in-proc-bpf-programs/20170427-090839
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   include/linux/compiler.h:264:8: sparse: attribute 'no_sanitize_address': unknown attribute
   lib/test_bpf.c:407:29: sparse: subtraction of functions? Share your drugs
   lib/test_bpf.c:418:29: sparse: subtraction of functions? Share your drugs
>> lib/test_bpf.c:5613:36: sparse: not enough arguments for function bpf_prog_alloc
   lib/test_bpf.c: In function 'generate_filter':
   lib/test_bpf.c:5613:8: error: too few arguments to function 'bpf_prog_alloc'
      fp = bpf_prog_alloc(bpf_prog_size(flen), 0);
           ^~~~~~~~~~~~~~
   In file included from lib/test_bpf.c:20:0:
   include/linux/filter.h:619:18: note: declared here
    struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags,
                     ^~~~~~~~~~~~~~

vim +5613 lib/test_bpf.c

10f18e0ba Daniel Borkmann    2014-05-23  5597  				/* Verifier didn't reject the test that's
10f18e0ba Daniel Borkmann    2014-05-23  5598  				 * bad enough, just return!
10f18e0ba Daniel Borkmann    2014-05-23  5599  				 */
10f18e0ba Daniel Borkmann    2014-05-23  5600  				*err = -EINVAL;
10f18e0ba Daniel Borkmann    2014-05-23  5601  				return NULL;
10f18e0ba Daniel Borkmann    2014-05-23  5602  			}
10f18e0ba Daniel Borkmann    2014-05-23  5603  		}
10f18e0ba Daniel Borkmann    2014-05-23  5604  		/* We don't expect to fail. */
10f18e0ba Daniel Borkmann    2014-05-23  5605  		if (*err) {
10f18e0ba Daniel Borkmann    2014-05-23  5606  			pr_cont("FAIL to attach err=%d len=%d\n",
10f18e0ba Daniel Borkmann    2014-05-23  5607  				*err, fprog.len);
10f18e0ba Daniel Borkmann    2014-05-23  5608  			return NULL;
64a8946b4 Alexei Starovoitov 2014-05-08  5609  		}
10f18e0ba Daniel Borkmann    2014-05-23  5610  		break;
10f18e0ba Daniel Borkmann    2014-05-23  5611  
10f18e0ba Daniel Borkmann    2014-05-23  5612  	case INTERNAL:
60a3b2253 Daniel Borkmann    2014-09-02 @5613  		fp = bpf_prog_alloc(bpf_prog_size(flen), 0);
10f18e0ba Daniel Borkmann    2014-05-23  5614  		if (fp == NULL) {
10f18e0ba Daniel Borkmann    2014-05-23  5615  			pr_cont("UNEXPECTED_FAIL no memory left\n");
10f18e0ba Daniel Borkmann    2014-05-23  5616  			*err = -ENOMEM;
10f18e0ba Daniel Borkmann    2014-05-23  5617  			return NULL;
10f18e0ba Daniel Borkmann    2014-05-23  5618  		}
10f18e0ba Daniel Borkmann    2014-05-23  5619  
10f18e0ba Daniel Borkmann    2014-05-23  5620  		fp->len = flen;
4962fa10f Daniel Borkmann    2015-07-30  5621  		/* Type doesn't really matter here as long as it's not unspec. */

:::::: The code at line 5613 was first introduced by commit
:::::: 60a3b2253c413cf601783b070507d7dd6620c954 net: bpf: make eBPF interpreter images read-only

:::::: TO: Daniel Borkmann <dborkman@redhat.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH net-next 5/6] bpf: add skeleton for procfs printing of bpf_progs

[Hannes Frederic Sowa]

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 kernel/bpf/core.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 048e2d79718a16..3ba175a24e971a 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -32,6 +32,9 @@
 #include <linux/kallsyms.h>
 #include <linux/rcupdate.h>
 
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+
 #include <asm/unaligned.h>
 
 /* Registers */
@@ -488,6 +491,93 @@ int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 	return ret;
 }
 
+/* ebpf procfs implementation */
+
+static struct proc_dir_entry *ebpf_proc_dir;
+
+static void *ebpf_proc_start(struct seq_file *s, loff_t *pos)
+	__acquires(RCU_BH)
+{
+	struct bpf_prog_aux *aux;
+	loff_t off = 0;
+
+	rcu_read_lock();
+
+	if (*pos == 0)
+		return SEQ_START_TOKEN;
+
+	list_for_each_entry_rcu(aux, &bpf_progs, bpf_progs_head)
+		if (++off == *pos)
+			return aux;
+
+	return NULL;
+}
+
+static void *ebpf_proc_next(struct seq_file *s, void *v, loff_t *pos)
+{
+	struct bpf_prog_aux *aux;
+
+	++*pos;
+
+	if (v == SEQ_START_TOKEN)
+		return list_first_or_null_rcu(&bpf_progs, struct bpf_prog_aux,
+					      bpf_progs_head);
+
+	aux = v;
+	return list_next_or_null_rcu(&bpf_progs,
+				     &aux->bpf_progs_head,
+				     struct bpf_prog_aux,
+				     bpf_progs_head);
+}
+
+static void ebpf_proc_stop(struct seq_file *s, void *v)
+	__releases(RCU_BH)
+{
+	rcu_read_unlock();
+}
+
+static int ebpf_proc_show(struct seq_file *s, void *v)
+{
+	if (v == SEQ_START_TOKEN) {
+		seq_printf(s, "# tag\n");
+		return 0;
+	}
+
+	return 0;
+}
+
+static const struct seq_operations ebpf_seq_ops = {
+	.start = ebpf_proc_start,
+	.next = ebpf_proc_next,
+	.stop = ebpf_proc_stop,
+	.show = ebpf_proc_show,
+};
+
+static int ebpf_proc_open(struct inode *inode, struct file *file)
+{
+	return seq_open(file, &ebpf_seq_ops);
+}
+
+static const struct file_operations ebpf_proc_operations = {
+	.open = ebpf_proc_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = seq_release,
+};
+
+static int __init ebpf_proc_init(void)
+{
+	ebpf_proc_dir = proc_mkdir("bpf", NULL);
+	if (!ebpf_proc_dir)
+		return 0;
+	proc_create("programs", 0400, ebpf_proc_dir, &ebpf_proc_operations);
+	return 0;
+}
+
+device_initcall(ebpf_proc_init);
+
+/* end of bpf proc inmplementation */
+
 struct bpf_binary_header *
 bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
 		     unsigned int alignment,
-- 
2.9.3

[PATCH net-next 6/6] bpf: show bpf programs

[Hannes Frederic Sowa]

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/uapi/linux/bpf.h | 32 +++++++++++++++++++-------------
 kernel/bpf/core.c        | 30 +++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e553529929f683..d6506e320953d5 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -101,20 +101,26 @@ enum bpf_map_type {
 	BPF_MAP_TYPE_HASH_OF_MAPS,
 };
 
+#define BPF_PROG_TYPES			\
+	X(BPF_PROG_TYPE_UNSPEC),	\
+	X(BPF_PROG_TYPE_SOCKET_FILTER),	\
+	X(BPF_PROG_TYPE_KPROBE),	\
+	X(BPF_PROG_TYPE_SCHED_CLS),	\
+	X(BPF_PROG_TYPE_SCHED_ACT),	\
+	X(BPF_PROG_TYPE_TRACEPOINT),	\
+	X(BPF_PROG_TYPE_XDP),		\
+	X(BPF_PROG_TYPE_PERF_EVENT),	\
+	X(BPF_PROG_TYPE_CGROUP_SKB),	\
+	X(BPF_PROG_TYPE_CGROUP_SOCK),	\
+	X(BPF_PROG_TYPE_LWT_IN),	\
+	X(BPF_PROG_TYPE_LWT_OUT),	\
+	X(BPF_PROG_TYPE_LWT_XMIT),
+
+
 enum bpf_prog_type {
-	BPF_PROG_TYPE_UNSPEC,
-	BPF_PROG_TYPE_SOCKET_FILTER,
-	BPF_PROG_TYPE_KPROBE,
-	BPF_PROG_TYPE_SCHED_CLS,
-	BPF_PROG_TYPE_SCHED_ACT,
-	BPF_PROG_TYPE_TRACEPOINT,
-	BPF_PROG_TYPE_XDP,
-	BPF_PROG_TYPE_PERF_EVENT,
-	BPF_PROG_TYPE_CGROUP_SKB,
-	BPF_PROG_TYPE_CGROUP_SOCK,
-	BPF_PROG_TYPE_LWT_IN,
-	BPF_PROG_TYPE_LWT_OUT,
-	BPF_PROG_TYPE_LWT_XMIT,
+#define X(type) type
+	BPF_PROG_TYPES
+#undef X
 };
 
 enum bpf_attach_type {
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 3ba175a24e971a..685c1d0f31e029 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -536,13 +536,41 @@ static void ebpf_proc_stop(struct seq_file *s, void *v)
 	rcu_read_unlock();
 }
 
+static const char *bpf_type_string(enum bpf_prog_type type)
+{
+	static const char *bpf_type_names[] = {
+#define X(type) #type
+		BPF_PROG_TYPES
+#undef X
+	};
+
+	if (type >= ARRAY_SIZE(bpf_type_names))
+		return "<unknown>";
+
+	return bpf_type_names[type];
+}
+
 static int ebpf_proc_show(struct seq_file *s, void *v)
 {
+	struct bpf_prog *prog;
+	struct bpf_prog_aux *aux;
+	char prog_tag[sizeof(prog->tag) * 2 + 1] = { };
+
 	if (v == SEQ_START_TOKEN) {
-		seq_printf(s, "# tag\n");
+		seq_printf(s, "# tag\t\t\ttype\t\t\truntime\tcap\tmemlock\n");
 		return 0;
 	}
 
+	aux = v;
+	prog = aux->prog;
+
+	bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
+	seq_printf(s, "%s\t%s\t%s\t%s\t%llu\n", prog_tag,
+		   bpf_type_string(prog->type),
+		   prog->jited ? "jit" : "int",
+		   prog->priv_cap_sys_admin ? "priv" : "unpriv",
+		   prog->pages * 1ULL << PAGE_SHIFT);
+
 	return 0;
 }
 
-- 
2.9.3

Re: [PATCH net-next 6/6] bpf: show bpf programs

[Alexei Starovoitov]

On Wed, Apr 26, 2017 at 08:24:19PM +0200, Hannes Frederic Sowa wrote:
>  
> +static const char *bpf_type_string(enum bpf_prog_type type)
> +{
> +	static const char *bpf_type_names[] = {
> +#define X(type) #type
> +		BPF_PROG_TYPES
> +#undef X
> +	};
> +
> +	if (type >= ARRAY_SIZE(bpf_type_names))
> +		return "<unknown>";
> +
> +	return bpf_type_names[type];
> +}
> +
>  static int ebpf_proc_show(struct seq_file *s, void *v)
>  {
> +	struct bpf_prog *prog;
> +	struct bpf_prog_aux *aux;
> +	char prog_tag[sizeof(prog->tag) * 2 + 1] = { };
> +
>  	if (v == SEQ_START_TOKEN) {
> -		seq_printf(s, "# tag\n");
> +		seq_printf(s, "# tag\t\t\ttype\t\t\truntime\tcap\tmemlock\n");
>  		return 0;
>  	}
>  
> +	aux = v;
> +	prog = aux->prog;
> +
> +	bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
> +	seq_printf(s, "%s\t%s\t%s\t%s\t%llu\n", prog_tag,
> +		   bpf_type_string(prog->type),
> +		   prog->jited ? "jit" : "int",
> +		   prog->priv_cap_sys_admin ? "priv" : "unpriv",
> +		   prog->pages * 1ULL << PAGE_SHIFT);

As I said several times already I'm strongly against procfs
style of exposing information about the programs.
I don't want this to become debugfs for bpf.
Maintaining the list of all loaded programs is fine
and we need a way to iterate through them, but procfs
is obviously not the interface to do that.
Programs/maps are binary whereas any fs interface is text.
It also doesn't scale with large number of programs/maps.
I prefer Daniel's suggestion on adding 'get_next' like API.
Also would be good if you can wait for Martin to finish his
prog->handle/id patches. Then user space will be able
to iterate through all the progs/maps and fetch all info about
them through syscall in extensible way.
And you wouldn't need to abuse kallsyms list for different purpose.

Re: [PATCH net-next 6/6] bpf: show bpf programs

[Hannes Frederic Sowa]

On 26.04.2017 23:25, Alexei Starovoitov wrote:
> On Wed, Apr 26, 2017 at 08:24:19PM +0200, Hannes Frederic Sowa wrote:
>>  
>> +static const char *bpf_type_string(enum bpf_prog_type type)
>> +{
>> +	static const char *bpf_type_names[] = {
>> +#define X(type) #type
>> +		BPF_PROG_TYPES
>> +#undef X
>> +	};
>> +
>> +	if (type >= ARRAY_SIZE(bpf_type_names))
>> +		return "<unknown>";
>> +
>> +	return bpf_type_names[type];
>> +}
>> +
>>  static int ebpf_proc_show(struct seq_file *s, void *v)
>>  {
>> +	struct bpf_prog *prog;
>> +	struct bpf_prog_aux *aux;
>> +	char prog_tag[sizeof(prog->tag) * 2 + 1] = { };
>> +
>>  	if (v == SEQ_START_TOKEN) {
>> -		seq_printf(s, "# tag\n");
>> +		seq_printf(s, "# tag\t\t\ttype\t\t\truntime\tcap\tmemlock\n");
>>  		return 0;
>>  	}
>>  
>> +	aux = v;
>> +	prog = aux->prog;
>> +
>> +	bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
>> +	seq_printf(s, "%s\t%s\t%s\t%s\t%llu\n", prog_tag,
>> +		   bpf_type_string(prog->type),
>> +		   prog->jited ? "jit" : "int",
>> +		   prog->priv_cap_sys_admin ? "priv" : "unpriv",
>> +		   prog->pages * 1ULL << PAGE_SHIFT);
> 
> As I said several times already I'm strongly against procfs
> style of exposing information about the programs.

I would not expand procfs interface to dump ebpf bytecode or jitcode,
albeit I would prefer a file system abstraction for that.

> I don't want this to become debugfs for bpf.

Right now it just prints a list of ebpf programs. You reject of where
things are going or do you already reject this particular patch?

> Maintaining the list of all loaded programs is fine
> and we need a way to iterate through them, but procfs
> is obviously not the interface to do that.
> Programs/maps are binary whereas any fs interface is text.

It should give admins only a high level overview of what is going on in
there system. I don't want o print any kind of opcodes there, just
giving people the possibility to see what is going on.

> It also doesn't scale with large number of programs/maps.
> I prefer Daniel's suggestion on adding 'get_next' like API.
> Also would be good if you can wait for Martin to finish his
> prog->handle/id patches. Then user space will be able
> to iterate through all the progs/maps and fetch all info about
> them through syscall in extensible way.
> And you wouldn't need to abuse kallsyms list for different purpose.

I don't buy the scalability argument:

What is the scalability issue with this O(1) approach on loading and
O(n) (but also without any locks held) on dumping? You can't do any
better. You can even dump several programs with one syscall instead of
slowly iterating over them.

Certainly I can wait with those patches until Martin presented his patches.

And regarding abusing the list, I just renamed it. If you think I abuse
something I can also add another list like bpf_prog_id patches do?

Bye bye,
Hannes

Re: [PATCH net-next 6/6] bpf: show bpf programs

[Daniel Borkmann]

On 04/26/2017 08:24 PM, Hannes Frederic Sowa wrote:
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>   include/uapi/linux/bpf.h | 32 +++++++++++++++++++-------------
>   kernel/bpf/core.c        | 30 +++++++++++++++++++++++++++++-
>   2 files changed, 48 insertions(+), 14 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index e553529929f683..d6506e320953d5 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -101,20 +101,26 @@ enum bpf_map_type {
>   	BPF_MAP_TYPE_HASH_OF_MAPS,
>   };
>
> +#define BPF_PROG_TYPES			\
> +	X(BPF_PROG_TYPE_UNSPEC),	\
> +	X(BPF_PROG_TYPE_SOCKET_FILTER),	\
> +	X(BPF_PROG_TYPE_KPROBE),	\
> +	X(BPF_PROG_TYPE_SCHED_CLS),	\
> +	X(BPF_PROG_TYPE_SCHED_ACT),	\
> +	X(BPF_PROG_TYPE_TRACEPOINT),	\
> +	X(BPF_PROG_TYPE_XDP),		\
> +	X(BPF_PROG_TYPE_PERF_EVENT),	\
> +	X(BPF_PROG_TYPE_CGROUP_SKB),	\
> +	X(BPF_PROG_TYPE_CGROUP_SOCK),	\
> +	X(BPF_PROG_TYPE_LWT_IN),	\
> +	X(BPF_PROG_TYPE_LWT_OUT),	\
> +	X(BPF_PROG_TYPE_LWT_XMIT),
> +
> +
>   enum bpf_prog_type {
> -	BPF_PROG_TYPE_UNSPEC,
> -	BPF_PROG_TYPE_SOCKET_FILTER,
> -	BPF_PROG_TYPE_KPROBE,
> -	BPF_PROG_TYPE_SCHED_CLS,
> -	BPF_PROG_TYPE_SCHED_ACT,
> -	BPF_PROG_TYPE_TRACEPOINT,
> -	BPF_PROG_TYPE_XDP,
> -	BPF_PROG_TYPE_PERF_EVENT,
> -	BPF_PROG_TYPE_CGROUP_SKB,
> -	BPF_PROG_TYPE_CGROUP_SOCK,
> -	BPF_PROG_TYPE_LWT_IN,
> -	BPF_PROG_TYPE_LWT_OUT,
> -	BPF_PROG_TYPE_LWT_XMIT,
> +#define X(type) type

Defining X in uapi could clash easily with other headers e.g.
from application side.

> +	BPF_PROG_TYPES
> +#undef X
>   };
>
>   enum bpf_attach_type {
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 3ba175a24e971a..685c1d0f31e029 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -536,13 +536,41 @@ static void ebpf_proc_stop(struct seq_file *s, void *v)
>   	rcu_read_unlock();
>   }
>
> +static const char *bpf_type_string(enum bpf_prog_type type)
> +{
> +	static const char *bpf_type_names[] = {
> +#define X(type) #type
> +		BPF_PROG_TYPES
> +#undef X
> +	};
> +
> +	if (type >= ARRAY_SIZE(bpf_type_names))
> +		return "<unknown>";
> +
> +	return bpf_type_names[type];
> +}
> +
>   static int ebpf_proc_show(struct seq_file *s, void *v)
>   {
> +	struct bpf_prog *prog;
> +	struct bpf_prog_aux *aux;
> +	char prog_tag[sizeof(prog->tag) * 2 + 1] = { };
> +
>   	if (v == SEQ_START_TOKEN) {
> -		seq_printf(s, "# tag\n");
> +		seq_printf(s, "# tag\t\t\ttype\t\t\truntime\tcap\tmemlock\n");
>   		return 0;
>   	}
>
> +	aux = v;
> +	prog = aux->prog;
> +
> +	bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
> +	seq_printf(s, "%s\t%s\t%s\t%s\t%llu\n", prog_tag,
> +		   bpf_type_string(prog->type),
> +		   prog->jited ? "jit" : "int",
> +		   prog->priv_cap_sys_admin ? "priv" : "unpriv",
> +		   prog->pages * 1ULL << PAGE_SHIFT);

Yeah, so that would be quite similar to what we dump in
bpf_prog_show_fdinfo() modulo the priv bit.

I generally agree that a facility for dumping all progs is needed
and it was also on the TODO list after the bpf(2) cmd for dumping
program insns back to user space.

I think the procfs interface has pro and cons: the upside is that
you can use it with tools like cat to inspect it, but what you still
cannot do is to say that you want to see the prog insns for, say,
prog #4 from that list. If we could iterate over that list through fds
via bpf(2) syscall, you could i) present the same info you have above
via fdinfo already and ii) also dump the BPF insns from that specific
program through a BPF_PROG_DUMP bpf(2) command. Once that dump also
supports maps in progs, you could go further and fetch related map
fds for inspection, etc.

Such option of iterating through that would need a new BPF syscall
cmd aka BPF_PROG_GET_NEXT which returns the first prog from the list
and you would walk the next one by passing the current fd, which can
later also be closed as not needed anymore. We could restrict that
dump to capable(CAP_SYS_ADMIN), and the kernel tree would need to
ship a tool e.g. under tools/bpf/ that can be used for inspection.

> +
>   	return 0;
>   }
>
>

Re: [PATCH net-next 6/6] bpf: show bpf programs

[Hannes Frederic Sowa]

On 26.04.2017 23:35, Daniel Borkmann wrote:
> On 04/26/2017 08:24 PM, Hannes Frederic Sowa wrote:
>> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> ---
>>   include/uapi/linux/bpf.h | 32 +++++++++++++++++++-------------
>>   kernel/bpf/core.c        | 30 +++++++++++++++++++++++++++++-
>>   2 files changed, 48 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index e553529929f683..d6506e320953d5 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -101,20 +101,26 @@ enum bpf_map_type {
>>       BPF_MAP_TYPE_HASH_OF_MAPS,
>>   };
>>
>> +#define BPF_PROG_TYPES            \
>> +    X(BPF_PROG_TYPE_UNSPEC),    \
>> +    X(BPF_PROG_TYPE_SOCKET_FILTER),    \
>> +    X(BPF_PROG_TYPE_KPROBE),    \
>> +    X(BPF_PROG_TYPE_SCHED_CLS),    \
>> +    X(BPF_PROG_TYPE_SCHED_ACT),    \
>> +    X(BPF_PROG_TYPE_TRACEPOINT),    \
>> +    X(BPF_PROG_TYPE_XDP),        \
>> +    X(BPF_PROG_TYPE_PERF_EVENT),    \
>> +    X(BPF_PROG_TYPE_CGROUP_SKB),    \
>> +    X(BPF_PROG_TYPE_CGROUP_SOCK),    \
>> +    X(BPF_PROG_TYPE_LWT_IN),    \
>> +    X(BPF_PROG_TYPE_LWT_OUT),    \
>> +    X(BPF_PROG_TYPE_LWT_XMIT),
>> +
>> +
>>   enum bpf_prog_type {
>> -    BPF_PROG_TYPE_UNSPEC,
>> -    BPF_PROG_TYPE_SOCKET_FILTER,
>> -    BPF_PROG_TYPE_KPROBE,
>> -    BPF_PROG_TYPE_SCHED_CLS,
>> -    BPF_PROG_TYPE_SCHED_ACT,
>> -    BPF_PROG_TYPE_TRACEPOINT,
>> -    BPF_PROG_TYPE_XDP,
>> -    BPF_PROG_TYPE_PERF_EVENT,
>> -    BPF_PROG_TYPE_CGROUP_SKB,
>> -    BPF_PROG_TYPE_CGROUP_SOCK,
>> -    BPF_PROG_TYPE_LWT_IN,
>> -    BPF_PROG_TYPE_LWT_OUT,
>> -    BPF_PROG_TYPE_LWT_XMIT,
>> +#define X(type) type
> 
> Defining X in uapi could clash easily with other headers e.g.
> from application side.

I think that X-macros are so much used that code review should catch
that no X macro is leaked beyond its scope.

I certainly can use some other macro names, maybe something along the
line from bpf_types.h.

> 
>> +    BPF_PROG_TYPES
>> +#undef X
>>   };
>>
>>   enum bpf_attach_type {
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index 3ba175a24e971a..685c1d0f31e029 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -536,13 +536,41 @@ static void ebpf_proc_stop(struct seq_file *s,
>> void *v)
>>       rcu_read_unlock();
>>   }
>>
>> +static const char *bpf_type_string(enum bpf_prog_type type)
>> +{
>> +    static const char *bpf_type_names[] = {
>> +#define X(type) #type
>> +        BPF_PROG_TYPES
>> +#undef X
>> +    };
>> +
>> +    if (type >= ARRAY_SIZE(bpf_type_names))
>> +        return "<unknown>";
>> +
>> +    return bpf_type_names[type];
>> +}
>> +
>>   static int ebpf_proc_show(struct seq_file *s, void *v)
>>   {
>> +    struct bpf_prog *prog;
>> +    struct bpf_prog_aux *aux;
>> +    char prog_tag[sizeof(prog->tag) * 2 + 1] = { };
>> +
>>       if (v == SEQ_START_TOKEN) {
>> -        seq_printf(s, "# tag\n");
>> +        seq_printf(s, "# tag\t\t\ttype\t\t\truntime\tcap\tmemlock\n");
>>           return 0;
>>       }
>>
>> +    aux = v;
>> +    prog = aux->prog;
>> +
>> +    bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
>> +    seq_printf(s, "%s\t%s\t%s\t%s\t%llu\n", prog_tag,
>> +           bpf_type_string(prog->type),
>> +           prog->jited ? "jit" : "int",
>> +           prog->priv_cap_sys_admin ? "priv" : "unpriv",
>> +           prog->pages * 1ULL << PAGE_SHIFT);
> 
> Yeah, so that would be quite similar to what we dump in
> bpf_prog_show_fdinfo() modulo the priv bit.

And, additionally, that you don't need to have a file descriptor handy
to do so, which is my main point.

> I generally agree that a facility for dumping all progs is needed
> and it was also on the TODO list after the bpf(2) cmd for dumping
> program insns back to user space.

I think this is orthogonal.

> I think the procfs interface has pro and cons: the upside is that
> you can use it with tools like cat to inspect it, but what you still
> cannot do is to say that you want to see the prog insns for, say,
> prog #4 from that list. If we could iterate over that list through fds
> via bpf(2) syscall, you could i) present the same info you have above
> via fdinfo already and ii) also dump the BPF insns from that specific
> program through a BPF_PROG_DUMP bpf(2) command. Once that dump also
> supports maps in progs, you could go further and fetch related map
> fds for inspection, etc.

Sure, that sounds super. But so far Linux and most (maybe I should write
all) subsystems always provided some easy way to get the insights of the
kernel without having to code or rely on special tools so far. And I
very much enjoyed that part on Linux. FreeBSD did very often abstract
some details behind shared libraries and commands that has no easy
pedant to use for with cat and grep.

> Such option of iterating through that would need a new BPF syscall
> cmd aka BPF_PROG_GET_NEXT which returns the first prog from the list
> and you would walk the next one by passing the current fd, which can
> later also be closed as not needed anymore. We could restrict that
> dump to capable(CAP_SYS_ADMIN), and the kernel tree would need to
> ship a tool e.g. under tools/bpf/ that can be used for inspection.
> 
Martin already posted his patches which add a bpf_prog_id to ebpf
programs. I will have alook at those and will comment over there.

Thansk for the review,
Hannes

Re: [PATCH net-next 6/6] bpf: show bpf programs

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 27 Apr 2017 15:22:49 +0200

> Sure, that sounds super. But so far Linux and most (maybe I should write
> all) subsystems always provided some easy way to get the insights of the
> kernel without having to code or rely on special tools so far.

Not true.

You cannot fully dump socket TCP internal state without netlink based
tools.  It is just one of many examples.

Can you dump all nftables rules without a special tool?

I don't think this is a legitimate line of argument, and I want
this to be done via the bpf() system call which is what people
are working on.

Thanks.

Re: [PATCH net-next 6/6] bpf: show bpf programs

[Hannes Frederic Sowa]

On 27.04.2017 18:00, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Thu, 27 Apr 2017 15:22:49 +0200
> 
>> Sure, that sounds super. But so far Linux and most (maybe I should write
>> all) subsystems always provided some easy way to get the insights of the
>> kernel without having to code or rely on special tools so far.
> 
> Not true.

Yes, I should not have written it that generally. ;)

> You cannot fully dump socket TCP internal state without netlink based
> tools.  It is just one of many examples.
>
> Can you dump all nftables rules without a special tool?

You got me here, I agree that not all state is discoverable via procfs.
But to some degree even netfilter and tcp do expose some considerable
amount of data via procfs. In the case of netfilter it might be less
valuable, though, I have to agree.

> I don't think this is a legitimate line of argument, and I want
> this to be done via the bpf() system call which is what people
> are working on.

I hope you saw that I was absolutely not against dumping or enumeration
with the bpf syscall. It will be the primary interface to debug ebpf and
I completely agree.

Merely I tried to establish the procfs interface as quick look interface
if some type of bpf program is loaded which could start any further
diagnosis. This interface should not have any dependencies and should
even work on embedded devices, where sometimes it might be difficult to
get a binary for the correct architecture installed ad-hoc (I am
thinking about openwrt). But this is definitely also solvable.

I do think if a common utility in util-linux, like lsbpf, is available I
will be fine.

Anyway, I will take this argument back.

Thanks,
Hannes

Re: [PATCH net-next 6/6] bpf: show bpf programs

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 27 Apr 2017 18:28:17 +0200

> Merely I tried to establish the procfs interface as quick look
> interface

Show me that "quick look" nftables dumping facility via procfs
and I'll start to listen to you.

What you are proposing has no real value once we have bpf() system
call based traversal and has no strict precedence across the
networking subsystem.

Thank you.

Re: [PATCH net-next 4/6] bpf: track if the bpf program was loaded with SYS_ADMIN capabilities

[Alexei Starovoitov]

On Thu, Apr 27, 2017 at 6:17 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>
> cap_sys_admin influences the verifier a lot in terms which programs are
> accepted and which are not. So during investigations it might be even
> interesting if the bpf program required those special flags or if the
> same program could be loaded just as underprivileged.

there are bunch of other things that influence verifier.
should we add all of them ?
there is prog->aux->user which is more than enough for
debugging.

Re: [PATCH net-next 4/6] bpf: track if the bpf program was loaded with SYS_ADMIN capabilities

[Jiri Benc]

On Thu, 27 Apr 2017 07:49:29 -0700, Alexei Starovoitov wrote:
> there are bunch of other things that influence verifier.
> should we add all of them ?

Sure, why not? It's always good to be able to see the complete input
conditions while debugging.

 Jiri