Commit 52875a04 authored by Jakub Kicinski's avatar Jakub Kicinski Committed by Alexei Starovoitov
Browse files

bpf: verifier: remove dead code 



Instead of overwriting dead code with jmp -1 instructions
remove it completely for root.  Adjust verifier state and
line info appropriately.

v2:
 - adjust func_info (Alexei);
 - make sure first instruction retains line info (Alexei).
v4: (Yonghong)
 - remove unnecessary if (!insn to remove) checks;
 - always keep last line info if first live instruction lacks one.
v5: (Martin Lau)
 - improve and clarify comments.

Signed-off-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: default avatarYonghong Song <yhs@fb.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent e2ae4ca2

include/linux/filter.h

+1 −0
Original line number Diff line number Diff line
@@ -778,6 +778,7 @@ static inline bool bpf_dump_raw_ok(void) 


struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off,

				       const struct bpf_insn *patch, u32 len);

int bpf_remove_insns(struct bpf_prog *prog, u32 off, u32 cnt);



void bpf_clear_redirect_map(struct bpf_map *map);

kernel/bpf/core.c

+12 −0
Original line number Diff line number Diff line
@@ -462,6 +462,18 @@ struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off, 
	return prog_adj;

}



int bpf_remove_insns(struct bpf_prog *prog, u32 off, u32 cnt)

{

	/* Branch offsets can't overflow when program is shrinking, no need

	 * to call bpf_adj_branches(..., true) here

	 */

	memmove(prog->insnsi + off, prog->insnsi + off + cnt,

		sizeof(struct bpf_insn) * (prog->len - off - cnt));

	prog->len -= cnt;



	return WARN_ON_ONCE(bpf_adj_branches(prog, off, off + cnt, off, false));

}



void bpf_prog_kallsyms_del_subprogs(struct bpf_prog *fp)

{

	int i;

kernel/bpf/verifier.c

+173 −3
Original line number Diff line number Diff line
@@ -6432,6 +6432,150 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of 
	return new_prog;

}



static int adjust_subprog_starts_after_remove(struct bpf_verifier_env *env,

					      u32 off, u32 cnt)

{

	int i, j;



	/* find first prog starting at or after off (first to remove) */

	for (i = 0; i < env->subprog_cnt; i++)

		if (env->subprog_info[i].start >= off)

			break;

	/* find first prog starting at or after off + cnt (first to stay) */

	for (j = i; j < env->subprog_cnt; j++)

		if (env->subprog_info[j].start >= off + cnt)

			break;

	/* if j doesn't start exactly at off + cnt, we are just removing

	 * the front of previous prog

	 */

	if (env->subprog_info[j].start != off + cnt)

		j--;



	if (j > i) {

		struct bpf_prog_aux *aux = env->prog->aux;

		int move;



		/* move fake 'exit' subprog as well */

		move = env->subprog_cnt + 1 - j;



		memmove(env->subprog_info + i,

			env->subprog_info + j,

			sizeof(*env->subprog_info) * move);

		env->subprog_cnt -= j - i;



		/* remove func_info */

		if (aux->func_info) {

			move = aux->func_info_cnt - j;



			memmove(aux->func_info + i,

				aux->func_info + j,

				sizeof(*aux->func_info) * move);

			aux->func_info_cnt -= j - i;

			/* func_info->insn_off is set after all code rewrites,

			 * in adjust_btf_func() - no need to adjust

			 */

		}

	} else {

		/* convert i from "first prog to remove" to "first to adjust" */

		if (env->subprog_info[i].start == off)

			i++;

	}



	/* update fake 'exit' subprog as well */

	for (; i <= env->subprog_cnt; i++)

		env->subprog_info[i].start -= cnt;



	return 0;

}



static int bpf_adj_linfo_after_remove(struct bpf_verifier_env *env, u32 off,

				      u32 cnt)

{

	struct bpf_prog *prog = env->prog;

	u32 i, l_off, l_cnt, nr_linfo;

	struct bpf_line_info *linfo;



	nr_linfo = prog->aux->nr_linfo;

	if (!nr_linfo)

		return 0;



	linfo = prog->aux->linfo;



	/* find first line info to remove, count lines to be removed */

	for (i = 0; i < nr_linfo; i++)

		if (linfo[i].insn_off >= off)

			break;



	l_off = i;

	l_cnt = 0;

	for (; i < nr_linfo; i++)

		if (linfo[i].insn_off < off + cnt)

			l_cnt++;

		else

			break;



	/* First live insn doesn't match first live linfo, it needs to "inherit"

	 * last removed linfo.  prog is already modified, so prog->len == off

	 * means no live instructions after (tail of the program was removed).

	 */

	if (prog->len != off && l_cnt &&

	    (i == nr_linfo || linfo[i].insn_off != off + cnt)) {

		l_cnt--;

		linfo[--i].insn_off = off + cnt;

	}



	/* remove the line info which refer to the removed instructions */

	if (l_cnt) {

		memmove(linfo + l_off, linfo + i,

			sizeof(*linfo) * (nr_linfo - i));



		prog->aux->nr_linfo -= l_cnt;

		nr_linfo = prog->aux->nr_linfo;

	}



	/* pull all linfo[i].insn_off >= off + cnt in by cnt */

	for (i = l_off; i < nr_linfo; i++)

		linfo[i].insn_off -= cnt;



	/* fix up all subprogs (incl. 'exit') which start >= off */

	for (i = 0; i <= env->subprog_cnt; i++)

		if (env->subprog_info[i].linfo_idx > l_off) {

			/* program may have started in the removed region but

			 * may not be fully removed

			 */

			if (env->subprog_info[i].linfo_idx >= l_off + l_cnt)

				env->subprog_info[i].linfo_idx -= l_cnt;

			else

				env->subprog_info[i].linfo_idx = l_off;

		}



	return 0;

}



static int verifier_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt)

{

	struct bpf_insn_aux_data *aux_data = env->insn_aux_data;

	unsigned int orig_prog_len = env->prog->len;

	int err;



	err = bpf_remove_insns(env->prog, off, cnt);

	if (err)

		return err;



	err = adjust_subprog_starts_after_remove(env, off, cnt);

	if (err)

		return err;



	err = bpf_adj_linfo_after_remove(env, off, cnt);

	if (err)

		return err;



	memmove(aux_data + off,	aux_data + off + cnt,

		sizeof(*aux_data) * (orig_prog_len - off - cnt));



	return 0;

}



/* The verifier does more data flow analysis than llvm and will not

 * explore branches that are dead at run time. Malicious programs can

 * have dead code too. Therefore replace all dead at-run-time code
@@ -6492,6 +6636,30 @@ static void opt_hard_wire_dead_code_branches(struct bpf_verifier_env *env) 
	}

}



static int opt_remove_dead_code(struct bpf_verifier_env *env)

{

	struct bpf_insn_aux_data *aux_data = env->insn_aux_data;

	int insn_cnt = env->prog->len;

	int i, err;



	for (i = 0; i < insn_cnt; i++) {

		int j;



		j = 0;

		while (i + j < insn_cnt && !aux_data[i + j].seen)

			j++;

		if (!j)

			continue;



		err = verifier_remove_insns(env, i, j);

		if (err)

			return err;

		insn_cnt = env->prog->len;

	}



	return 0;

}



/* convert load instructions that access fields of a context type into a

 * sequence of instructions that access fields of the underlying structure:

 *     struct __sk_buff    -> struct sk_buff
@@ -7282,10 +7450,12 @@ skip_full_check: 
	if (is_priv) {

		if (ret == 0)

			opt_hard_wire_dead_code_branches(env);

	}



		if (ret == 0)

			ret = opt_remove_dead_code(env);

	} else {

		if (ret == 0)

			sanitize_dead_code(env);

	}



	if (ret == 0)

		/* program is valid, convert *(u32*)(ctx + off) accesses */

CRA Git | Maintained and supported by SUSTech CRA and CCSE