Commit 4117b344 authored by Andrii Nakryiko's avatar Andrii Nakryiko Committed by Pu Lehui
Browse files

bpf: mostly decouple jump history management from is_state_visited() 

mainline inclusion
from mainline-v6.2-rc1
commit a095f421
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/IB2AQ3
CVE: CVE-2023-52920

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a095f421057e



--------------------------------

Jump history updating and state equivalence checks are conceptually
independent, so move push_jmp_history() out of is_state_visited(). Also
make a decision whether to perform state equivalence checks or not one
layer higher in do_check(), keeping is_state_visited() unconditionally
performing state checks.

push_jmp_history() should be performed after state checks. There is just
one small non-uniformity. When is_state_visited() finds already
validated equivalent state, it propagates precision marks to current
state's parent chain. For this to work correctly, jump history has to be
updated, so is_state_visited() is doing that internally.

But if no equivalent verified state is found, jump history has to be
updated in a newly cloned child state, so is_jmp_point()
+ push_jmp_history() is performed after is_state_visited() exited with
zero result, which means "proceed with validation".

This change has no functional changes. It's not strictly necessary, but
feels right to decouple these two processes.

Acked-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20221206233345.438540-3-andrii@kernel.org


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Signed-off-by: default avatarPu Lehui <pulehui@huawei.com>
parent ec43762a

kernel/bpf/verifier.c

+26 −23
Original line number Diff line number Diff line
@@ -10037,13 +10037,6 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx) 
	int i, j, err, states_cnt = 0;

	bool add_new_state = env->test_state_freq ? true : false;



	cur->last_insn_idx = env->prev_insn_idx;

	if (!is_prune_point(env, insn_idx))

		/* this 'insn_idx' instruction wasn't marked, so we will not

		 * be doing state search here

		 */

		return push_jmp_history(env, cur);



	/* bpf progs typically have pruning point every 4 instructions

	 * http://vger.kernel.org/bpfconf2019.html#session-1

	 * Do not add new state for future pruning if the verifier hasn't seen
@@ -10162,10 +10155,10 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx) 
		env->max_states_per_insn = states_cnt;



	if (!env->bpf_capable && states_cnt > BPF_COMPLEXITY_LIMIT_STATES)

		return push_jmp_history(env, cur);

		return 0;



	if (!add_new_state)

		return push_jmp_history(env, cur);

		return 0;



	/* There were no equivalent states, remember the current one.

	 * Technically the current state is not proven to be safe yet,
@@ -10305,6 +10298,9 @@ static int do_check(struct bpf_verifier_env *env) 
			return -E2BIG;

		}



		state->last_insn_idx = env->prev_insn_idx;



		if (is_prune_point(env, env->insn_idx)) {

			err = is_state_visited(env, env->insn_idx);

			if (err < 0)

				return err;
@@ -10321,6 +10317,13 @@ static int do_check(struct bpf_verifier_env *env) 
				}

				goto process_bpf_exit;

			}

		}



		if (is_jmp_point(env, env->insn_idx)) {

			err = push_jmp_history(env, state);

			if (err)

				return err;

		}



		if (signal_pending(current))

			return -EAGAIN;