[PATCH bpf-next 0/2] bpf: parallel verification

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH bpf-next 0/2] bpf: parallel verification
@ 2019-04-19 14:44 Alexei Starovoitov
  2019-04-19 14:44 ` [PATCH bpf-next 1/2] bpf: remove global variables Alexei Starovoitov
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2019-04-19 14:44 UTC (permalink / raw)
  To: davem; +Cc: daniel, netdev, bpf, kernel-team

Allow the bpf verifier to run in parallel for root.

Alexei Starovoitov (2):
  bpf: remove global variables
  bpf: drop bpf_verifier_lock

 include/linux/bpf_verifier.h |  5 +++++
 kernel/bpf/verifier.c        | 33 ++++++++++++++++++---------------
 2 files changed, 23 insertions(+), 15 deletions(-)

-- 
2.20.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 1/2] bpf: remove global variables
  2019-04-19 14:44 [PATCH bpf-next 0/2] bpf: parallel verification Alexei Starovoitov
@ 2019-04-19 14:44 ` Alexei Starovoitov
  2019-04-19 14:44 ` [PATCH bpf-next 2/2] bpf: drop bpf_verifier_lock Alexei Starovoitov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2019-04-19 14:44 UTC (permalink / raw)
  To: davem; +Cc: daniel, netdev, bpf, kernel-team

Move three global variables protected by bpf_verifier_lock into
'struct bpf_verifier_env' to allow parallel verification.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 include/linux/bpf_verifier.h |  5 +++++
 kernel/bpf/verifier.c        | 25 +++++++++++++------------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index b3ab61fe1932..1305ccbd8fe6 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -295,6 +295,11 @@ struct bpf_verifier_env {
 	const struct bpf_line_info *prev_linfo;
 	struct bpf_verifier_log log;
 	struct bpf_subprog_info subprog_info[BPF_MAX_SUBPROGS + 1];
+	struct {
+		int *insn_state;
+		int *insn_stack;
+		int cur_stack;
+	} cfg;
 	u32 subprog_cnt;
 	/* number of instructions analyzed by the verifier */
 	u32 insn_processed;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index db301e9b5295..5f0eb5bd5589 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5369,10 +5369,6 @@ enum {
 
 #define STATE_LIST_MARK ((struct bpf_verifier_state_list *) -1L)
 
-static int *insn_stack;	/* stack of insns to process */
-static int cur_stack;	/* current stack index */
-static int *insn_state;
-
 /* t, w, e - match pseudo-code above:
  * t - index of current instruction
  * w - next instruction
@@ -5380,6 +5376,9 @@ static int *insn_state;
  */
 static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
 {
+	int *insn_stack = env->cfg.insn_stack;
+	int *insn_state = env->cfg.insn_state;
+
 	if (e == FALLTHROUGH && insn_state[t] >= (DISCOVERED | FALLTHROUGH))
 		return 0;
 
@@ -5400,9 +5399,9 @@ static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
 		/* tree-edge */
 		insn_state[t] = DISCOVERED | e;
 		insn_state[w] = DISCOVERED;
-		if (cur_stack >= env->prog->len)
+		if (env->cfg.cur_stack >= env->prog->len)
 			return -E2BIG;
-		insn_stack[cur_stack++] = w;
+		insn_stack[env->cfg.cur_stack++] = w;
 		return 1;
 	} else if ((insn_state[w] & 0xF0) == DISCOVERED) {
 		verbose_linfo(env, t, "%d: ", t);
@@ -5426,14 +5425,15 @@ static int check_cfg(struct bpf_verifier_env *env)
 {
 	struct bpf_insn *insns = env->prog->insnsi;
 	int insn_cnt = env->prog->len;
+	int *insn_stack, *insn_state;
 	int ret = 0;
 	int i, t;
 
-	insn_state = kvcalloc(insn_cnt, sizeof(int), GFP_KERNEL);
+	insn_state = env->cfg.insn_state = kvcalloc(insn_cnt, sizeof(int), GFP_KERNEL);
 	if (!insn_state)
 		return -ENOMEM;
 
-	insn_stack = kvcalloc(insn_cnt, sizeof(int), GFP_KERNEL);
+	insn_stack = env->cfg.insn_stack = kvcalloc(insn_cnt, sizeof(int), GFP_KERNEL);
 	if (!insn_stack) {
 		kvfree(insn_state);
 		return -ENOMEM;
@@ -5441,12 +5441,12 @@ static int check_cfg(struct bpf_verifier_env *env)
 
 	insn_state[0] = DISCOVERED; /* mark 1st insn as discovered */
 	insn_stack[0] = 0; /* 0 is the first instruction */
-	cur_stack = 1;
+	env->cfg.cur_stack = 1;
 
 peek_stack:
-	if (cur_stack == 0)
+	if (env->cfg.cur_stack == 0)
 		goto check_state;
-	t = insn_stack[cur_stack - 1];
+	t = insn_stack[env->cfg.cur_stack - 1];
 
 	if (BPF_CLASS(insns[t].code) == BPF_JMP ||
 	    BPF_CLASS(insns[t].code) == BPF_JMP32) {
@@ -5515,7 +5515,7 @@ static int check_cfg(struct bpf_verifier_env *env)
 
 mark_explored:
 	insn_state[t] = EXPLORED;
-	if (cur_stack-- <= 0) {
+	if (env->cfg.cur_stack-- <= 0) {
 		verbose(env, "pop stack internal bug\n");
 		ret = -EFAULT;
 		goto err_free;
@@ -5535,6 +5535,7 @@ static int check_cfg(struct bpf_verifier_env *env)
 err_free:
 	kvfree(insn_state);
 	kvfree(insn_stack);
+	env->cfg.insn_state = env->cfg.insn_stack = NULL;
 	return ret;
 }
 
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 2/2] bpf: drop bpf_verifier_lock
  2019-04-19 14:44 [PATCH bpf-next 0/2] bpf: parallel verification Alexei Starovoitov
  2019-04-19 14:44 ` [PATCH bpf-next 1/2] bpf: remove global variables Alexei Starovoitov
@ 2019-04-19 14:44 ` Alexei Starovoitov
  2019-04-19 19:34 ` [PATCH bpf-next 0/2] bpf: parallel verification Andrii Nakryiko
  2019-04-22 23:58 ` Daniel Borkmann
  3 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2019-04-19 14:44 UTC (permalink / raw)
  To: davem; +Cc: daniel, netdev, bpf, kernel-team

Drop bpf_verifier_lock for root to avoid being DoS-ed by unprivileged.
The BPF verifier is now fully parallel.
All unpriv users are still serialized by bpf_verifier_lock to avoid
exhausting kernel memory by running N parallel verifications.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/verifier.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 5f0eb5bd5589..423f242a5efb 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8132,9 +8132,11 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
 		env->insn_aux_data[i].orig_idx = i;
 	env->prog = *prog;
 	env->ops = bpf_verifier_ops[env->prog->type];
+	is_priv = capable(CAP_SYS_ADMIN);
 
 	/* grab the mutex to protect few globals used by verifier */
-	mutex_lock(&bpf_verifier_lock);
+	if (!is_priv)
+		mutex_lock(&bpf_verifier_lock);
 
 	if (attr->log_level || attr->log_buf || attr->log_size) {
 		/* user requested verbose verifier output
@@ -8157,7 +8159,6 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
 	if (attr->prog_flags & BPF_F_ANY_ALIGNMENT)
 		env->strict_alignment = false;
 
-	is_priv = capable(CAP_SYS_ADMIN);
 	env->allow_ptr_leaks = is_priv;
 
 	ret = replace_map_fd_with_map_ptr(env);
@@ -8270,7 +8271,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
 		release_maps(env);
 	*prog = env->prog;
 err_unlock:
-	mutex_unlock(&bpf_verifier_lock);
+	if (!is_priv)
+		mutex_unlock(&bpf_verifier_lock);
 	vfree(env->insn_aux_data);
 err_free_env:
 	kfree(env);
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 0/2] bpf: parallel verification
  2019-04-19 14:44 [PATCH bpf-next 0/2] bpf: parallel verification Alexei Starovoitov
  2019-04-19 14:44 ` [PATCH bpf-next 1/2] bpf: remove global variables Alexei Starovoitov
  2019-04-19 14:44 ` [PATCH bpf-next 2/2] bpf: drop bpf_verifier_lock Alexei Starovoitov
@ 2019-04-19 19:34 ` Andrii Nakryiko
  2019-04-22 23:58 ` Daniel Borkmann
  3 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2019-04-19 19:34 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: davem, Daniel Borkmann, Networking, bpf, Kernel Team

On Fri, Apr 19, 2019 at 11:24 AM Alexei Starovoitov <ast@kernel.org> wrote:
>
> Allow the bpf verifier to run in parallel for root.
>
> Alexei Starovoitov (2):
>   bpf: remove global variables
>   bpf: drop bpf_verifier_lock
>
>  include/linux/bpf_verifier.h |  5 +++++
>  kernel/bpf/verifier.c        | 33 ++++++++++++++++++---------------
>  2 files changed, 23 insertions(+), 15 deletions(-)
>
> --
> 2.20.0
>
lgtm, for the set:

Acked-by: Andrii Nakryiko <andriin@fb.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 0/2] bpf: parallel verification
  2019-04-19 14:44 [PATCH bpf-next 0/2] bpf: parallel verification Alexei Starovoitov
                   ` (2 preceding siblings ...)
  2019-04-19 19:34 ` [PATCH bpf-next 0/2] bpf: parallel verification Andrii Nakryiko
@ 2019-04-22 23:58 ` Daniel Borkmann
  2019-04-23  0:06   ` Alexei Starovoitov
  3 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2019-04-22 23:58 UTC (permalink / raw)
  To: Alexei Starovoitov, davem; +Cc: netdev, bpf, kernel-team

On 04/19/2019 04:44 PM, Alexei Starovoitov wrote:
> Allow the bpf verifier to run in parallel for root.
> 
> Alexei Starovoitov (2):
>   bpf: remove global variables
>   bpf: drop bpf_verifier_lock
> 
>  include/linux/bpf_verifier.h |  5 +++++
>  kernel/bpf/verifier.c        | 33 ++++++++++++++++++---------------
>  2 files changed, 23 insertions(+), 15 deletions(-)
> 

Too bad we still need to keep it around for unpriv. Anyway, applied, thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 0/2] bpf: parallel verification
  2019-04-22 23:58 ` Daniel Borkmann
@ 2019-04-23  0:06   ` Alexei Starovoitov
  0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2019-04-23  0:06 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov, davem; +Cc: netdev, bpf, Kernel Team

On 4/22/19 4:58 PM, Daniel Borkmann wrote:
> On 04/19/2019 04:44 PM, Alexei Starovoitov wrote:
>> Allow the bpf verifier to run in parallel for root.
>>
>> Alexei Starovoitov (2):
>>    bpf: remove global variables
>>    bpf: drop bpf_verifier_lock
>>
>>   include/linux/bpf_verifier.h |  5 +++++
>>   kernel/bpf/verifier.c        | 33 ++++++++++++++++++---------------
>>   2 files changed, 23 insertions(+), 15 deletions(-)
>>
> 
> Too bad we still need to keep it around for unpriv. Anyway, applied, thanks!

I'm planing to add memcg accounting and teach oom_badness
about the memory used during verification. Then we can drop
the mutex for unpriv as well.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-04-23  0:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-19 14:44 [PATCH bpf-next 0/2] bpf: parallel verification Alexei Starovoitov
2019-04-19 14:44 ` [PATCH bpf-next 1/2] bpf: remove global variables Alexei Starovoitov
2019-04-19 14:44 ` [PATCH bpf-next 2/2] bpf: drop bpf_verifier_lock Alexei Starovoitov
2019-04-19 19:34 ` [PATCH bpf-next 0/2] bpf: parallel verification Andrii Nakryiko
2019-04-22 23:58 ` Daniel Borkmann
2019-04-23  0:06   ` Alexei Starovoitov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.