Commit 55795ef5 authored by Rabin Vincent's avatar Rabin Vincent Committed by David S. Miller
Browse files

net: filter: make JITs zero A for SKF_AD_ALU_XOR_X 



The SKF_AD_ALU_XOR_X ancillary is not like the other ancillary data
instructions since it XORs A with X while all the others replace A with
some loaded value.  All the BPF JITs fail to clear A if this is used as
the first instruction in a filter.  This was found using american fuzzy
lop.

Add a helper to determine if A needs to be cleared given the first
instruction in a filter, and use this in the JITs.  Except for ARM, the
rest have only been compile-tested.

Fixes: 34805931 ("net: filter: get rid of BPF_S_* enum")
Signed-off-by: default avatarRabin Vincent <rabin@rab.in>
Acked-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent ff621985

arch/arm/net/bpf_jit_32.c

+1 −15
Original line number Diff line number Diff line
@@ -187,19 +187,6 @@ static inline int mem_words_used(struct jit_ctx *ctx) 
	return fls(ctx->seen & SEEN_MEM);

}



static inline bool is_load_to_a(u16 inst)

{

	switch (inst) {

	case BPF_LD | BPF_W | BPF_LEN:

	case BPF_LD | BPF_W | BPF_ABS:

	case BPF_LD | BPF_H | BPF_ABS:

	case BPF_LD | BPF_B | BPF_ABS:

		return true;

	default:

		return false;

	}

}



static void jit_fill_hole(void *area, unsigned int size)

{

	u32 *ptr;
@@ -211,7 +198,6 @@ static void jit_fill_hole(void *area, unsigned int size) 
static void build_prologue(struct jit_ctx *ctx)

{

	u16 reg_set = saved_regs(ctx);

	u16 first_inst = ctx->skf->insns[0].code;

	u16 off;



#ifdef CONFIG_FRAME_POINTER
@@ -241,7 +227,7 @@ static void build_prologue(struct jit_ctx *ctx) 
		emit(ARM_MOV_I(r_X, 0), ctx);



	/* do not leak kernel data to userspace */

	if ((first_inst != (BPF_RET | BPF_K)) && !(is_load_to_a(first_inst)))

	if (bpf_needs_clear_a(&ctx->skf->insns[0]))

		emit(ARM_MOV_I(r_A, 0), ctx);



	/* stack space for the BPF_MEM words */

arch/mips/net/bpf_jit.c

+1 −15
Original line number Diff line number Diff line
@@ -521,19 +521,6 @@ static inline u16 align_sp(unsigned int num) 
	return num;

}



static bool is_load_to_a(u16 inst)

{

	switch (inst) {

	case BPF_LD | BPF_W | BPF_LEN:

	case BPF_LD | BPF_W | BPF_ABS:

	case BPF_LD | BPF_H | BPF_ABS:

	case BPF_LD | BPF_B | BPF_ABS:

		return true;

	default:

		return false;

	}

}



static void save_bpf_jit_regs(struct jit_ctx *ctx, unsigned offset)

{

	int i = 0, real_off = 0;
@@ -614,7 +601,6 @@ static unsigned int get_stack_depth(struct jit_ctx *ctx) 


static void build_prologue(struct jit_ctx *ctx)

{

	u16 first_inst = ctx->skf->insns[0].code;

	int sp_off;



	/* Calculate the total offset for the stack pointer */
@@ -641,7 +627,7 @@ static void build_prologue(struct jit_ctx *ctx) 
		emit_jit_reg_move(r_X, r_zero, ctx);



	/* Do not leak kernel data to userspace */

	if ((first_inst != (BPF_RET | BPF_K)) && !(is_load_to_a(first_inst)))

	if (bpf_needs_clear_a(&ctx->skf->insns[0]))

		emit_jit_reg_move(r_A, r_zero, ctx);

}

arch/powerpc/net/bpf_jit_comp.c

+2 −11
Original line number Diff line number Diff line
@@ -78,19 +78,10 @@ static void bpf_jit_build_prologue(struct bpf_prog *fp, u32 *image, 
		PPC_LI(r_X, 0);

	}



	switch (filter[0].code) {

	case BPF_RET | BPF_K:

	case BPF_LD | BPF_W | BPF_LEN:

	case BPF_LD | BPF_W | BPF_ABS:

	case BPF_LD | BPF_H | BPF_ABS:

	case BPF_LD | BPF_B | BPF_ABS:

		/* first instruction sets A register (or is RET 'constant') */

		break;

	default:

	/* make sure we dont leak kernel information to user */

	if (bpf_needs_clear_a(&filter[0]))

		PPC_LI(r_A, 0);

}

}



static void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx)

{

arch/sparc/net/bpf_jit_comp.c

+2 −15
Original line number Diff line number Diff line
@@ -420,22 +420,9 @@ void bpf_jit_compile(struct bpf_prog *fp) 
		}

		emit_reg_move(O7, r_saved_O7);



		switch (filter[0].code) {

		case BPF_RET | BPF_K:

		case BPF_LD | BPF_W | BPF_LEN:

		case BPF_LD | BPF_W | BPF_ABS:

		case BPF_LD | BPF_H | BPF_ABS:

		case BPF_LD | BPF_B | BPF_ABS:

			/* The first instruction sets the A register (or is

			 * a "RET 'constant'")

			 */

			break;

		default:

			/* Make sure we dont leak kernel information to the

			 * user.

			 */

		/* Make sure we dont leak kernel information to the user. */

		if (bpf_needs_clear_a(&filter[0]))

			emit_clear(r_A); /* A = 0 */

		}



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

			unsigned int K = filter[i].k;

include/linux/filter.h

+19 −0
Original line number Diff line number Diff line
@@ -493,6 +493,25 @@ static inline void bpf_jit_free(struct bpf_prog *fp) 


#define BPF_ANC		BIT(15)



static inline bool bpf_needs_clear_a(const struct sock_filter *first)

{

	switch (first->code) {

	case BPF_RET | BPF_K:

	case BPF_LD | BPF_W | BPF_LEN:

		return false;



	case BPF_LD | BPF_W | BPF_ABS:

	case BPF_LD | BPF_H | BPF_ABS:

	case BPF_LD | BPF_B | BPF_ABS:

		if (first->k == SKF_AD_OFF + SKF_AD_ALU_XOR_X)

			return true;

		return false;



	default:

		return true;

	}

}



static inline u16 bpf_anc_helper(const struct sock_filter *ftest)

{

	BUG_ON(ftest->code & BPF_ANC);

CRA Git | Maintained and supported by SUSTech CRA and CCSE