Commit dbcfe5f7 authored by Gianluca Borello's avatar Gianluca Borello Committed by David S. Miller
Browse files

bpf: split check_mem_access logic for map values 



Move the logic to check memory accesses to a PTR_TO_MAP_VALUE_ADJ from
check_mem_access() to a separate helper check_map_access_adj(). This
enables to use those checks in other parts of the verifier as well,
where boundaries on PTR_TO_MAP_VALUE_ADJ might need to be checked, for
example when checking helper function arguments. The same thing is
already happening for other types such as PTR_TO_PACKET and its
check_packet_access() helper.

The code has been copied verbatim, with the only difference of removing
the "off += reg->max_value" statement and moving the sum into the call
statement to check_map_access(), as that was only needed due to the
earlier common check_map_access() call.

Signed-off-by: default avatarGianluca Borello <g.borello@gmail.com>
Acked-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent f3a3e248

kernel/bpf/verifier.c

+49 −39
Original line number Diff line number Diff line
@@ -635,6 +635,51 @@ static int check_map_access(struct bpf_verifier_env *env, u32 regno, int off, 
	return 0;

}



/* check read/write into an adjusted map element */

static int check_map_access_adj(struct bpf_verifier_env *env, u32 regno,

				int off, int size)

{

	struct bpf_verifier_state *state = &env->cur_state;

	struct bpf_reg_state *reg = &state->regs[regno];

	int err;



	/* We adjusted the register to this map value, so we

	 * need to change off and size to min_value and max_value

	 * respectively to make sure our theoretical access will be

	 * safe.

	 */

	if (log_level)

		print_verifier_state(state);

	env->varlen_map_value_access = true;

	/* The minimum value is only important with signed

	 * comparisons where we can't assume the floor of a

	 * value is 0.  If we are using signed variables for our

	 * index'es we need to make sure that whatever we use

	 * will have a set floor within our range.

	 */

	if (reg->min_value < 0) {

		verbose("R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",

			regno);

		return -EACCES;

	}

	err = check_map_access(env, regno, reg->min_value + off, size);

	if (err) {

		verbose("R%d min value is outside of the array range\n",

			regno);

		return err;

	}



	/* If we haven't set a max value then we need to bail

	 * since we can't be sure we won't do bad things.

	 */

	if (reg->max_value == BPF_REGISTER_MAX_RANGE) {

		verbose("R%d unbounded memory access, make sure to bounds check any array access into a map\n",

			regno);

		return -EACCES;

	}

	return check_map_access(env, regno, reg->max_value + off, size);

}



#define MAX_PACKET_OFF 0xffff



static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
@@ -775,44 +820,9 @@ static int check_mem_access(struct bpf_verifier_env *env, u32 regno, int off, 
			return -EACCES;

		}



		/* If we adjusted the register to this map value at all then we

		 * need to change off and size to min_value and max_value

		 * respectively to make sure our theoretical access will be

		 * safe.

		 */

		if (reg->type == PTR_TO_MAP_VALUE_ADJ) {

			if (log_level)

				print_verifier_state(state);

			env->varlen_map_value_access = true;

			/* The minimum value is only important with signed

			 * comparisons where we can't assume the floor of a

			 * value is 0.  If we are using signed variables for our

			 * index'es we need to make sure that whatever we use

			 * will have a set floor within our range.

			 */

			if (reg->min_value < 0) {

				verbose("R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",

					regno);

				return -EACCES;

			}

			err = check_map_access(env, regno, reg->min_value + off,

					       size);

			if (err) {

				verbose("R%d min value is outside of the array range\n",

					regno);

				return err;

			}



			/* If we haven't set a max value then we need to bail

			 * since we can't be sure we won't do bad things.

			 */

			if (reg->max_value == BPF_REGISTER_MAX_RANGE) {

				verbose("R%d unbounded memory access, make sure to bounds check any array access into a map\n",

					regno);

				return -EACCES;

			}

			off += reg->max_value;

		}

		if (reg->type == PTR_TO_MAP_VALUE_ADJ)

			err = check_map_access_adj(env, regno, off, size);

		else

			err = check_map_access(env, regno, off, size);

		if (!err && t == BPF_READ && value_regno >= 0)

			mark_reg_unknown_value(state->regs, value_regno);

CRA Git | Maintained and supported by SUSTech CRA and CCSE