Commit c1b08ebe authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'bpf-jit-fixes' 



Daniel Borkmann says:

====================
Two fixes that deal with buggy usage of bpf_helper_changes_pkt_data()
in the sense that they also reload cached skb data when there's no
skb context but xdp one, for example. A fix where skb meta data is
reloaded out of the wrong register on helper call, rest is test cases
and making sure on verifier side that there's always the guarantee
that ctx sits in r1. Thanks!
====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 2d17d8d7 87ab8194

arch/powerpc/net/bpf_jit_comp64.c

+4 −2
Original line number Diff line number Diff line
@@ -763,7 +763,8 @@ emit_clear: 
			func = (u8 *) __bpf_call_base + imm;



			/* Save skb pointer if we need to re-cache skb data */

			if (bpf_helper_changes_pkt_data(func))

			if ((ctx->seen & SEEN_SKB) &&

			    bpf_helper_changes_pkt_data(func))

				PPC_BPF_STL(3, 1, bpf_jit_stack_local(ctx));



			bpf_jit_emit_func_call(image, ctx, (u64)func);
@@ -772,7 +773,8 @@ emit_clear: 
			PPC_MR(b2p[BPF_REG_0], 3);



			/* refresh skb cache */

			if (bpf_helper_changes_pkt_data(func)) {

			if ((ctx->seen & SEEN_SKB) &&

			    bpf_helper_changes_pkt_data(func)) {

				/* reload skb pointer to r3 */

				PPC_BPF_LL(3, 1, bpf_jit_stack_local(ctx));

				bpf_jit_emit_skb_loads(image, ctx);

arch/s390/net/bpf_jit_comp.c

+5 −6
Original line number Diff line number Diff line
@@ -55,8 +55,7 @@ struct bpf_jit { 
#define SEEN_LITERAL	8	/* code uses literals */

#define SEEN_FUNC	16	/* calls C functions */

#define SEEN_TAIL_CALL	32	/* code uses tail calls */

#define SEEN_SKB_CHANGE	64	/* code changes skb data */

#define SEEN_REG_AX	128	/* code uses constant blinding */

#define SEEN_REG_AX	64	/* code uses constant blinding */

#define SEEN_STACK	(SEEN_FUNC | SEEN_MEM | SEEN_SKB)



/*
@@ -448,13 +447,13 @@ static void bpf_jit_prologue(struct bpf_jit *jit, u32 stack_depth) 
			EMIT6_DISP_LH(0xe3000000, 0x0024, REG_W1, REG_0,

				      REG_15, 152);

	}

	if (jit->seen & SEEN_SKB)

	if (jit->seen & SEEN_SKB) {

		emit_load_skb_data_hlen(jit);

	if (jit->seen & SEEN_SKB_CHANGE)

		/* stg %b1,ST_OFF_SKBP(%r0,%r15) */

		EMIT6_DISP_LH(0xe3000000, 0x0024, BPF_REG_1, REG_0, REG_15,

			      STK_OFF_SKBP);

	}

}



/*

 * Function epilogue
@@ -983,8 +982,8 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i 
		EMIT2(0x0d00, REG_14, REG_W1);

		/* lgr %b0,%r2: load return value into %b0 */

		EMIT4(0xb9040000, BPF_REG_0, REG_2);

		if (bpf_helper_changes_pkt_data((void *)func)) {

			jit->seen |= SEEN_SKB_CHANGE;

		if ((jit->seen & SEEN_SKB) &&

		    bpf_helper_changes_pkt_data((void *)func)) {

			/* lg %b1,ST_OFF_SKBP(%r15) */

			EMIT6_DISP_LH(0xe3000000, 0x0004, BPF_REG_1, REG_0,

				      REG_15, STK_OFF_SKBP);

arch/sparc/net/bpf_jit_comp_64.c

+4 −2
Original line number Diff line number Diff line
@@ -1245,14 +1245,16 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx) 
		u8 *func = ((u8 *)__bpf_call_base) + imm;



		ctx->saw_call = true;

		if (ctx->saw_ld_abs_ind && bpf_helper_changes_pkt_data(func))

			emit_reg_move(bpf2sparc[BPF_REG_1], L7, ctx);



		emit_call((u32 *)func, ctx);

		emit_nop(ctx);



		emit_reg_move(O0, bpf2sparc[BPF_REG_0], ctx);



		if (bpf_helper_changes_pkt_data(func) && ctx->saw_ld_abs_ind)

			load_skb_regs(ctx, bpf2sparc[BPF_REG_6]);

		if (ctx->saw_ld_abs_ind && bpf_helper_changes_pkt_data(func))

			load_skb_regs(ctx, L7);

		break;

	}

kernel/bpf/verifier.c

+6 −0
Original line number Diff line number Diff line
@@ -1674,7 +1674,13 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx) 
		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) {

		verbose(env, "kernel subsystem misconfigured func %s#%d: r1 != ctx\n",

			func_id_name(func_id), func_id);

		return -EINVAL;

	}



	memset(&meta, 0, sizeof(meta));

	meta.pkt_access = fn->pkt_access;

lib/test_bpf.c

+43 −0
Original line number Diff line number Diff line
@@ -435,6 +435,41 @@ loop: 
	return 0;

}



static int bpf_fill_ld_abs_vlan_push_pop2(struct bpf_test *self)

{

	struct bpf_insn *insn;



	insn = kmalloc_array(16, sizeof(*insn), GFP_KERNEL);

	if (!insn)

		return -ENOMEM;



	/* Due to func address being non-const, we need to

	 * assemble this here.

	 */

	insn[0] = BPF_MOV64_REG(R6, R1);

	insn[1] = BPF_LD_ABS(BPF_B, 0);

	insn[2] = BPF_LD_ABS(BPF_H, 0);

	insn[3] = BPF_LD_ABS(BPF_W, 0);

	insn[4] = BPF_MOV64_REG(R7, R6);

	insn[5] = BPF_MOV64_IMM(R6, 0);

	insn[6] = BPF_MOV64_REG(R1, R7);

	insn[7] = BPF_MOV64_IMM(R2, 1);

	insn[8] = BPF_MOV64_IMM(R3, 2);

	insn[9] = BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,

			       bpf_skb_vlan_push_proto.func - __bpf_call_base);

	insn[10] = BPF_MOV64_REG(R6, R7);

	insn[11] = BPF_LD_ABS(BPF_B, 0);

	insn[12] = BPF_LD_ABS(BPF_H, 0);

	insn[13] = BPF_LD_ABS(BPF_W, 0);

	insn[14] = BPF_MOV64_IMM(R0, 42);

	insn[15] = BPF_EXIT_INSN();



	self->u.ptr.insns = insn;

	self->u.ptr.len = 16;



	return 0;

}



static int bpf_fill_jump_around_ld_abs(struct bpf_test *self)

{

	unsigned int len = BPF_MAXINSNS;
@@ -6066,6 +6101,14 @@ static struct bpf_test tests[] = { 
		{},

		{ {0x1, 0x42 } },

	},

	{

		"LD_ABS with helper changing skb data",

		{ },

		INTERNAL,

		{ 0x34 },

		{ { ETH_HLEN, 42 } },

		.fill_helper = bpf_fill_ld_abs_vlan_push_pop2,

	},

};



static struct net_device dev;

CRA Git | Maintained and supported by SUSTech CRA and CCSE