Commit 8b6ca2bf authored by David S. Miller's avatar David S. Miller
Browse files

Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf 



Daniel Borkmann says:

====================
pull-request: bpf 2017-12-21

The following pull-request contains BPF updates for your *net* tree.

The main changes are:

1) Fix multiple security issues in the BPF verifier mostly related
   to the value and min/max bounds tracking rework in 4.14. Issues
   range from incorrect bounds calculation in some BPF_RSH cases,
   to improper sign extension and reg size handling on 32 bit
   ALU ops, missing strict alignment checks on stack pointers, and
   several others that got fixed, from Jann, Alexei and Edward.

2) Fix various build failures in BPF selftests on sparc64. More
   specifically, librt needed to be added to the libs to link
   against and few format string fixups for sizeof, from David.

3) Fix one last remaining issue from BPF selftest build that was
   still occuring on s390x from the asm/bpf_perf_event.h include
   which could not find the asm/ptrace.h copy, from Hendrik.
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents b4681c28 82abbf8d

include/linux/bpf_verifier.h

+2 −2
Original line number Diff line number Diff line
@@ -15,11 +15,11 @@ 
 * In practice this is far bigger than any realistic pointer offset; this limit

 * ensures that umax_value + (int)off + (int)size cannot overflow a u64.

 */

#define BPF_MAX_VAR_OFF	(1ULL << 31)

#define BPF_MAX_VAR_OFF	(1 << 29)

/* Maximum variable size permitted for ARG_CONST_SIZE[_OR_ZERO].  This ensures

 * that converting umax_value to int cannot overflow.

 */

#define BPF_MAX_VAR_SIZ	INT_MAX

#define BPF_MAX_VAR_SIZ	(1 << 29)



/* Liveness marks, used for registers and spilled-regs (in stack slots).

 * Read marks propagate upwards until they find a write mark; they record that

kernel/bpf/verifier.c

+160 −117
Original line number Diff line number Diff line
@@ -1059,6 +1059,11 @@ static int check_ptr_alignment(struct bpf_verifier_env *env, 
		break;

	case PTR_TO_STACK:

		pointer_desc = "stack ";

		/* The stack spill tracking logic in check_stack_write()

		 * and check_stack_read() relies on stack accesses being

		 * aligned.

		 */

		strict = true;

		break;

	default:

		break;
@@ -1067,6 +1072,29 @@ static int check_ptr_alignment(struct bpf_verifier_env *env, 
					   strict);

}



/* truncate register to smaller size (in bytes)

 * must be called with size < BPF_REG_SIZE

 */

static void coerce_reg_to_size(struct bpf_reg_state *reg, int size)

{

	u64 mask;



	/* clear high bits in bit representation */

	reg->var_off = tnum_cast(reg->var_off, size);



	/* fix arithmetic bounds */

	mask = ((u64)1 << (size * 8)) - 1;

	if ((reg->umin_value & ~mask) == (reg->umax_value & ~mask)) {

		reg->umin_value &= mask;

		reg->umax_value &= mask;

	} else {

		reg->umin_value = 0;

		reg->umax_value = mask;

	}

	reg->smin_value = reg->umin_value;

	reg->smax_value = reg->umax_value;

}



/* check whether memory at (regno + off) is accessible for t = (read | write)

 * if t==write, value_regno is a register which value is stored into memory

 * if t==read, value_regno is a register which will receive the value from memory
@@ -1200,9 +1228,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn 
	if (!err && size < BPF_REG_SIZE && value_regno >= 0 && t == BPF_READ &&

	    regs[value_regno].type == SCALAR_VALUE) {

		/* b/h/w load zero-extends, mark upper bits as known 0 */

		regs[value_regno].var_off =

			tnum_cast(regs[value_regno].var_off, size);

		__update_reg_bounds(&regs[value_regno]);

		coerce_reg_to_size(&regs[value_regno], size);

	}

	return err;

}
@@ -1282,6 +1308,7 @@ static int check_stack_boundary(struct bpf_verifier_env *env, int regno, 
		tnum_strn(tn_buf, sizeof(tn_buf), regs[regno].var_off);

		verbose(env, "invalid variable stack read R%d var_off=%s\n",

			regno, tn_buf);

		return -EACCES;

	}

	off = regs[regno].off + regs[regno].var_off.value;

	if (off >= 0 || off < -MAX_BPF_STACK || off + access_size > 0 ||
@@ -1772,14 +1799,6 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx) 
	return 0;

}



static void coerce_reg_to_32(struct bpf_reg_state *reg)

{

	/* clear high 32 bits */

	reg->var_off = tnum_cast(reg->var_off, 4);

	/* Update bounds */

	__update_reg_bounds(reg);

}



static bool signed_add_overflows(s64 a, s64 b)

{

	/* Do the add in u64, where overflow is well-defined */
@@ -1800,6 +1819,41 @@ static bool signed_sub_overflows(s64 a, s64 b) 
	return res > a;

}



static bool check_reg_sane_offset(struct bpf_verifier_env *env,

				  const struct bpf_reg_state *reg,

				  enum bpf_reg_type type)

{

	bool known = tnum_is_const(reg->var_off);

	s64 val = reg->var_off.value;

	s64 smin = reg->smin_value;



	if (known && (val >= BPF_MAX_VAR_OFF || val <= -BPF_MAX_VAR_OFF)) {

		verbose(env, "math between %s pointer and %lld is not allowed\n",

			reg_type_str[type], val);

		return false;

	}



	if (reg->off >= BPF_MAX_VAR_OFF || reg->off <= -BPF_MAX_VAR_OFF) {

		verbose(env, "%s pointer offset %d is not allowed\n",

			reg_type_str[type], reg->off);

		return false;

	}



	if (smin == S64_MIN) {

		verbose(env, "math between %s pointer and register with unbounded min value is not allowed\n",

			reg_type_str[type]);

		return false;

	}



	if (smin >= BPF_MAX_VAR_OFF || smin <= -BPF_MAX_VAR_OFF) {

		verbose(env, "value %lld makes %s pointer be out of bounds\n",

			smin, reg_type_str[type]);

		return false;

	}



	return true;

}



/* Handles arithmetic on a pointer and a scalar: computes new min/max and var_off.

 * Caller should also handle BPF_MOV case separately.

 * If we return -EACCES, caller may want to try again treating pointer as a
@@ -1836,7 +1890,6 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, 


	if (BPF_CLASS(insn->code) != BPF_ALU64) {

		/* 32-bit ALU ops on pointers produce (meaningless) scalars */

		if (!env->allow_ptr_leaks)

		verbose(env,

			"R%d 32-bit pointer arithmetic prohibited\n",

			dst);
@@ -1844,19 +1897,16 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, 
	}



	if (ptr_reg->type == PTR_TO_MAP_VALUE_OR_NULL) {

		if (!env->allow_ptr_leaks)

		verbose(env, "R%d pointer arithmetic on PTR_TO_MAP_VALUE_OR_NULL prohibited, null-check it first\n",

			dst);

		return -EACCES;

	}

	if (ptr_reg->type == CONST_PTR_TO_MAP) {

		if (!env->allow_ptr_leaks)

		verbose(env, "R%d pointer arithmetic on CONST_PTR_TO_MAP prohibited\n",

			dst);

		return -EACCES;

	}

	if (ptr_reg->type == PTR_TO_PACKET_END) {

		if (!env->allow_ptr_leaks)

		verbose(env, "R%d pointer arithmetic on PTR_TO_PACKET_END prohibited\n",

			dst);

		return -EACCES;
@@ -1868,6 +1918,10 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, 
	dst_reg->type = ptr_reg->type;

	dst_reg->id = ptr_reg->id;



	if (!check_reg_sane_offset(env, off_reg, ptr_reg->type) ||

	    !check_reg_sane_offset(env, ptr_reg, ptr_reg->type))

		return -EINVAL;



	switch (opcode) {

	case BPF_ADD:

		/* We can take a fixed offset as long as it doesn't overflow
@@ -1921,7 +1975,6 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, 
	case BPF_SUB:

		if (dst_reg == off_reg) {

			/* scalar -= pointer.  Creates an unknown scalar */

			if (!env->allow_ptr_leaks)

			verbose(env, "R%d tried to subtract pointer from scalar\n",

				dst);

			return -EACCES;
@@ -1931,7 +1984,6 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, 
		 * be able to deal with it.

		 */

		if (ptr_reg->type == PTR_TO_STACK) {

			if (!env->allow_ptr_leaks)

			verbose(env, "R%d subtraction from stack pointer prohibited\n",

				dst);

			return -EACCES;
@@ -1982,28 +2034,30 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, 
	case BPF_AND:

	case BPF_OR:

	case BPF_XOR:

		/* bitwise ops on pointers are troublesome, prohibit for now.

		 * (However, in principle we could allow some cases, e.g.

		 * ptr &= ~3 which would reduce min_value by 3.)

		 */

		if (!env->allow_ptr_leaks)

		/* bitwise ops on pointers are troublesome, prohibit. */

		verbose(env, "R%d bitwise operator %s on pointer prohibited\n",

			dst, bpf_alu_string[opcode >> 4]);

		return -EACCES;

	default:

		/* other operators (e.g. MUL,LSH) produce non-pointer results */

		if (!env->allow_ptr_leaks)

		verbose(env, "R%d pointer arithmetic with %s operator prohibited\n",

			dst, bpf_alu_string[opcode >> 4]);

		return -EACCES;

	}



	if (!check_reg_sane_offset(env, dst_reg, ptr_reg->type))

		return -EINVAL;



	__update_reg_bounds(dst_reg);

	__reg_deduce_bounds(dst_reg);

	__reg_bound_offset(dst_reg);

	return 0;

}



/* WARNING: This function does calculations on 64-bit values, but the actual

 * execution may occur on 32-bit values. Therefore, things like bitshifts

 * need extra checks in the 32-bit case.

 */

static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,

				      struct bpf_insn *insn,

				      struct bpf_reg_state *dst_reg,
@@ -2014,12 +2068,8 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, 
	bool src_known, dst_known;

	s64 smin_val, smax_val;

	u64 umin_val, umax_val;

	u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32;



	if (BPF_CLASS(insn->code) != BPF_ALU64) {

		/* 32-bit ALU ops are (32,32)->64 */

		coerce_reg_to_32(dst_reg);

		coerce_reg_to_32(&src_reg);

	}

	smin_val = src_reg.smin_value;

	smax_val = src_reg.smax_value;

	umin_val = src_reg.umin_value;
@@ -2027,6 +2077,12 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, 
	src_known = tnum_is_const(src_reg.var_off);

	dst_known = tnum_is_const(dst_reg->var_off);



	if (!src_known &&

	    opcode != BPF_ADD && opcode != BPF_SUB && opcode != BPF_AND) {

		__mark_reg_unknown(dst_reg);

		return 0;

	}



	switch (opcode) {

	case BPF_ADD:

		if (signed_add_overflows(dst_reg->smin_value, smin_val) ||
@@ -2155,9 +2211,9 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, 
		__update_reg_bounds(dst_reg);

		break;

	case BPF_LSH:

		if (umax_val > 63) {

			/* Shifts greater than 63 are undefined.  This includes

			 * shifts by a negative number.

		if (umax_val >= insn_bitness) {

			/* Shifts greater than 31 or 63 are undefined.

			 * This includes shifts by a negative number.

			 */

			mark_reg_unknown(env, regs, insn->dst_reg);

			break;
@@ -2183,27 +2239,29 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, 
		__update_reg_bounds(dst_reg);

		break;

	case BPF_RSH:

		if (umax_val > 63) {

			/* Shifts greater than 63 are undefined.  This includes

			 * shifts by a negative number.

		if (umax_val >= insn_bitness) {

			/* Shifts greater than 31 or 63 are undefined.

			 * This includes shifts by a negative number.

			 */

			mark_reg_unknown(env, regs, insn->dst_reg);

			break;

		}

		/* BPF_RSH is an unsigned shift, so make the appropriate casts */

		if (dst_reg->smin_value < 0) {

			if (umin_val) {

				/* Sign bit will be cleared */

				dst_reg->smin_value = 0;

			} else {

				/* Lost sign bit information */

		/* BPF_RSH is an unsigned shift.  If the value in dst_reg might

		 * be negative, then either:

		 * 1) src_reg might be zero, so the sign bit of the result is

		 *    unknown, so we lose our signed bounds

		 * 2) it's known negative, thus the unsigned bounds capture the

		 *    signed bounds

		 * 3) the signed bounds cross zero, so they tell us nothing

		 *    about the result

		 * If the value in dst_reg is known nonnegative, then again the

		 * unsigned bounts capture the signed bounds.

		 * Thus, in all cases it suffices to blow away our signed bounds

		 * and rely on inferring new ones from the unsigned bounds and

		 * var_off of the result.

		 */

		dst_reg->smin_value = S64_MIN;

		dst_reg->smax_value = S64_MAX;

			}

		} else {

			dst_reg->smin_value =

				(u64)(dst_reg->smin_value) >> umax_val;

		}

		if (src_known)

			dst_reg->var_off = tnum_rshift(dst_reg->var_off,

						       umin_val);
@@ -2219,6 +2277,12 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, 
		break;

	}



	if (BPF_CLASS(insn->code) != BPF_ALU64) {

		/* 32-bit ALU ops are (32,32)->32 */

		coerce_reg_to_size(dst_reg, 4);

		coerce_reg_to_size(&src_reg, 4);

	}



	__reg_deduce_bounds(dst_reg);

	__reg_bound_offset(dst_reg);

	return 0;
@@ -2233,7 +2297,6 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env, 
	struct bpf_reg_state *regs = cur_regs(env), *dst_reg, *src_reg;

	struct bpf_reg_state *ptr_reg = NULL, off_reg = {0};

	u8 opcode = BPF_OP(insn->code);

	int rc;



	dst_reg = &regs[insn->dst_reg];

	src_reg = NULL;
@@ -2244,43 +2307,29 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env, 
		if (src_reg->type != SCALAR_VALUE) {

			if (dst_reg->type != SCALAR_VALUE) {

				/* Combining two pointers by any ALU op yields

				 * an arbitrary scalar.

				 * an arbitrary scalar. Disallow all math except

				 * pointer subtraction

				 */

				if (!env->allow_ptr_leaks) {

				if (opcode == BPF_SUB){

					mark_reg_unknown(env, regs, insn->dst_reg);

					return 0;

				}

				verbose(env, "R%d pointer %s pointer prohibited\n",

					insn->dst_reg,

					bpf_alu_string[opcode >> 4]);

				return -EACCES;

				}

				mark_reg_unknown(env, regs, insn->dst_reg);

				return 0;

			} else {

				/* scalar += pointer

				 * This is legal, but we have to reverse our

				 * src/dest handling in computing the range

				 */

				rc = adjust_ptr_min_max_vals(env, insn,

				return adjust_ptr_min_max_vals(env, insn,

							       src_reg, dst_reg);

				if (rc == -EACCES && env->allow_ptr_leaks) {

					/* scalar += unknown scalar */

					__mark_reg_unknown(&off_reg);

					return adjust_scalar_min_max_vals(

							env, insn,

							dst_reg, off_reg);

				}

				return rc;

			}

		} else if (ptr_reg) {

			/* pointer += scalar */

			rc = adjust_ptr_min_max_vals(env, insn,

			return adjust_ptr_min_max_vals(env, insn,

						       dst_reg, src_reg);

			if (rc == -EACCES && env->allow_ptr_leaks) {

				/* unknown scalar += scalar */

				__mark_reg_unknown(dst_reg);

				return adjust_scalar_min_max_vals(

						env, insn, dst_reg, *src_reg);

			}

			return rc;

		}

	} else {

		/* Pretend the src is a reg with a known value, since we only
@@ -2289,17 +2338,9 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env, 
		off_reg.type = SCALAR_VALUE;

		__mark_reg_known(&off_reg, insn->imm);

		src_reg = &off_reg;

		if (ptr_reg) { /* pointer += K */

			rc = adjust_ptr_min_max_vals(env, insn,

		if (ptr_reg) /* pointer += K */

			return adjust_ptr_min_max_vals(env, insn,

						       ptr_reg, src_reg);

			if (rc == -EACCES && env->allow_ptr_leaks) {

				/* unknown scalar += K */

				__mark_reg_unknown(dst_reg);

				return adjust_scalar_min_max_vals(

						env, insn, dst_reg, off_reg);

			}

			return rc;

		}

	}



	/* Got here implies adding two SCALAR_VALUEs */
@@ -2396,17 +2437,20 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) 
					return -EACCES;

				}

				mark_reg_unknown(env, regs, insn->dst_reg);

				/* high 32 bits are known zero. */

				regs[insn->dst_reg].var_off = tnum_cast(

						regs[insn->dst_reg].var_off, 4);

				__update_reg_bounds(&regs[insn->dst_reg]);

				coerce_reg_to_size(&regs[insn->dst_reg], 4);

			}

		} else {

			/* case: R = imm

			 * remember the value we stored into this reg

			 */

			regs[insn->dst_reg].type = SCALAR_VALUE;

			__mark_reg_known(regs + insn->dst_reg, insn->imm);

			if (BPF_CLASS(insn->code) == BPF_ALU64) {

				__mark_reg_known(regs + insn->dst_reg,

						 insn->imm);

			} else {

				__mark_reg_known(regs + insn->dst_reg,

						 (u32)insn->imm);

			}

		}



	} else if (opcode > BPF_END) {
@@ -3437,15 +3481,14 @@ static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur, 
			return range_within(rold, rcur) &&

			       tnum_in(rold->var_off, rcur->var_off);

		} else {

			/* if we knew anything about the old value, we're not

			 * equal, because we can't know anything about the

			 * scalar value of the pointer in the new value.

			 */

			return rold->umin_value == 0 &&

			       rold->umax_value == U64_MAX &&

			       rold->smin_value == S64_MIN &&

			       rold->smax_value == S64_MAX &&

			       tnum_is_unknown(rold->var_off);

			/* We're trying to use a pointer in place of a scalar.

			 * Even if the scalar was unbounded, this could lead to

			 * pointer leaks because scalars are allowed to leak

			 * while pointers are not. We could make this safe in

			 * special cases if root is calling us, but it's

			 * probably not worth the hassle.

			 */

			return false;

		}

	case PTR_TO_MAP_VALUE:

		/* If the new min/max/var_off satisfy the old ones and

tools/arch/s390/include/uapi/asm/bpf_perf_event.h

+1 −1
Original line number Diff line number Diff line
@@ -2,7 +2,7 @@ 
#ifndef _UAPI__ASM_BPF_PERF_EVENT_H__

#define _UAPI__ASM_BPF_PERF_EVENT_H__



#include <asm/ptrace.h>

#include "ptrace.h"



typedef user_pt_regs bpf_user_pt_regs_t;

tools/testing/selftests/bpf/Makefile

+1 −1
Original line number Diff line number Diff line
@@ -11,7 +11,7 @@ ifneq ($(wildcard $(GENHDR)),) 
endif



CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include

LDLIBS += -lcap -lelf

LDLIBS += -lcap -lelf -lrt



TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \

	test_align test_verifier_log test_dev_cgroup

tools/testing/selftests/bpf/test_progs.c

+4 −4
Original line number Diff line number Diff line
@@ -351,7 +351,7 @@ static void test_bpf_obj_id(void) 
			  info_len != sizeof(struct bpf_map_info) ||

			  strcmp((char *)map_infos[i].name, expected_map_name),

			  "get-map-info(fd)",

			  "err %d errno %d type %d(%d) info_len %u(%lu) key_size %u value_size %u max_entries %u map_flags %X name %s(%s)\n",

			  "err %d errno %d type %d(%d) info_len %u(%Zu) key_size %u value_size %u max_entries %u map_flags %X name %s(%s)\n",

			  err, errno,

			  map_infos[i].type, BPF_MAP_TYPE_ARRAY,

			  info_len, sizeof(struct bpf_map_info),
@@ -395,7 +395,7 @@ static void test_bpf_obj_id(void) 
			  *(int *)prog_infos[i].map_ids != map_infos[i].id ||

			  strcmp((char *)prog_infos[i].name, expected_prog_name),

			  "get-prog-info(fd)",

			  "err %d errno %d i %d type %d(%d) info_len %u(%lu) jit_enabled %d jited_prog_len %u xlated_prog_len %u jited_prog %d xlated_prog %d load_time %lu(%lu) uid %u(%u) nr_map_ids %u(%u) map_id %u(%u) name %s(%s)\n",

			  "err %d errno %d i %d type %d(%d) info_len %u(%Zu) jit_enabled %d jited_prog_len %u xlated_prog_len %u jited_prog %d xlated_prog %d load_time %lu(%lu) uid %u(%u) nr_map_ids %u(%u) map_id %u(%u) name %s(%s)\n",

			  err, errno, i,

			  prog_infos[i].type, BPF_PROG_TYPE_SOCKET_FILTER,

			  info_len, sizeof(struct bpf_prog_info),
@@ -463,7 +463,7 @@ static void test_bpf_obj_id(void) 
		      memcmp(&prog_info, &prog_infos[i], info_len) ||

		      *(int *)prog_info.map_ids != saved_map_id,

		      "get-prog-info(next_id->fd)",

		      "err %d errno %d info_len %u(%lu) memcmp %d map_id %u(%u)\n",

		      "err %d errno %d info_len %u(%Zu) memcmp %d map_id %u(%u)\n",

		      err, errno, info_len, sizeof(struct bpf_prog_info),

		      memcmp(&prog_info, &prog_infos[i], info_len),

		      *(int *)prog_info.map_ids, saved_map_id);
@@ -509,7 +509,7 @@ static void test_bpf_obj_id(void) 
		      memcmp(&map_info, &map_infos[i], info_len) ||

		      array_value != array_magic_value,

		      "check get-map-info(next_id->fd)",

		      "err %d errno %d info_len %u(%lu) memcmp %d array_value %llu(%llu)\n",

		      "err %d errno %d info_len %u(%Zu) memcmp %d array_value %llu(%llu)\n",

		      err, errno, info_len, sizeof(struct bpf_map_info),

		      memcmp(&map_info, &map_infos[i], info_len),

		      array_value, array_magic_value);

CRA Git | Maintained and supported by SUSTech CRA and CCSE