Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1676823

Re: [PATCH v3 net-next 02/12] bpf/verifier: rework value tracking

Show all headers | View raw

On 06/27/2017 02:56 PM, Edward Cree wrote:
> Tracks value alignment by means of tracking known & unknown bits.
> Tightens some min/max value checks and fixes a couple of bugs therein.

You mean the one in relation to patch 1/12? Would be good to elaborate
here since otherwise this gets forgotten few weeks later.

Could you also document all the changes that verifier will then start
allowing for after the patch?

> If pointer leaks are allowed, and adjust_ptr_min_max_vals returns -EACCES,
>   treat the pointer as an unknown scalar and try again, because we might be
>   able to conclude something about the result (e.g. pointer & 0x40 is either
>   0 or 0x40).
>
> Signed-off-by: Edward Cree <ecree@solarflare.com>
[...]
>   /* check whether memory at (regno + off) is accessible for t = (read | write)
> @@ -899,52 +965,79 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
>   	struct bpf_reg_state *reg = &state->regs[regno];
>   	int size, err = 0;
>
> -	if (reg->type == PTR_TO_STACK)
> -		off += reg->imm;
> -
>   	size = bpf_size_to_bytes(bpf_size);
>   	if (size < 0)
>   		return size;
>
[...]
> -	if (reg->type == PTR_TO_MAP_VALUE ||
> -	    reg->type == PTR_TO_MAP_VALUE_ADJ) {
> +	/* for access checks, reg->off is just part of off */
> +	off += reg->off;

Could you elaborate on why removing the reg->type == PTR_TO_STACK?
Also in context of below PTR_TO_CTX.

[...]
>   	} else if (reg->type == PTR_TO_CTX) {
> -		enum bpf_reg_type reg_type = UNKNOWN_VALUE;
> +		enum bpf_reg_type reg_type = SCALAR_VALUE;
>
>   		if (t == BPF_WRITE && value_regno >= 0 &&
>   		    is_pointer_value(env, value_regno)) {
>   			verbose("R%d leaks addr into ctx\n", value_regno);
>   			return -EACCES;
>   		}
> +		/* ctx accesses must be at a fixed offset, so that we can
> +		 * determine what type of data were returned.
> +		 */
> +		if (!tnum_is_const(reg->var_off)) {
> +			char tn_buf[48];
> +
> +			tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
> +			verbose("variable ctx access var_off=%s off=%d size=%d",
> +				tn_buf, off, size);
> +			return -EACCES;
> +		}
> +		off += reg->var_off.value;

... f.e. in PTR_TO_CTX case the only access that is currently
allowed is LDX/STX with fixed offset from insn->off, which is
passed as off param to check_mem_access(). Can you elaborate on
off += reg->var_off.value? Meaning we make this more dynamic
as long as access is known const?

>   		err = check_ctx_access(env, insn_idx, off, size, t, &reg_type);
>   		if (!err && t == BPF_READ && value_regno >= 0) {
> -			mark_reg_unknown_value_and_range(state->regs,
> -							 value_regno);
> -			/* note that reg.[id|off|range] == 0 */
> +			/* ctx access returns either a scalar, or a
> +			 * PTR_TO_PACKET[_END].  In the latter case, we know
> +			 * the offset is zero.
> +			 */
> +			if (reg_type == SCALAR_VALUE)
> +				mark_reg_unknown(state->regs, value_regno);
> +			else
> +				mark_reg_known_zero(state->regs, value_regno);
> +			state->regs[value_regno].id = 0;
> +			state->regs[value_regno].off = 0;
> +			state->regs[value_regno].range = 0;
>   			state->regs[value_regno].type = reg_type;
> -			state->regs[value_regno].aux_off = 0;
> -			state->regs[value_regno].aux_off_align = 0;
>   		}
>
> -	} else if (reg->type == FRAME_PTR || reg->type == PTR_TO_STACK) {
> +	} else if (reg->type == PTR_TO_STACK) {
[...]

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v3 net-next 00/12] bpf: rewrite value tracking in verifier Edward Cree <ecree@solarflare.com> - 2017-06-27 15:00 +0200
  [PATCH v3 net-next 06/12] selftests/bpf: change test_verifier  expectations Edward Cree <ecree@solarflare.com> - 2017-06-27 15:00 +0200
  [PATCH v3 net-next 03/12] nfp: change bpf verifier hooks to match new  verifier data structures Edward Cree <ecree@solarflare.com> - 2017-06-27 15:00 +0200
    Re: [PATCH v3 net-next 03/12] nfp: change bpf verifier hooks to match  new verifier data structures Daniel Borkmann <daniel@iogearbox.net> - 2017-06-28 22:50 +0200
    Re: [PATCH v3 net-next 03/12] nfp: change bpf verifier hooks to  match new verifier data structures Jakub Kicinski <jakub.kicinski@netronome.com> - 2017-06-29 05:50 +0200
  [PATCH v3 net-next 01/12] selftests/bpf: add test for mixed signed  and unsigned bounds checks Edward Cree <ecree@solarflare.com> - 2017-06-27 15:00 +0200
    Re: [PATCH v3 net-next 01/12] selftests/bpf: add test for mixed signed  and unsigned bounds checks Daniel Borkmann <daniel@iogearbox.net> - 2017-06-28 16:00 +0200
  [PATCH v3 net-next 07/12] selftests/bpf: rewrite test_align Edward Cree <ecree@solarflare.com> - 2017-06-27 15:00 +0200
  [PATCH v3 net-next 09/12] selftests/bpf: add test for bogus  operations on pointers Edward Cree <ecree@solarflare.com> - 2017-06-27 15:10 +0200
  [PATCH v3 net-next 12/12] selftests/bpf: variable offset negative  tests Edward Cree <ecree@solarflare.com> - 2017-06-27 15:10 +0200
  [PATCH v3 net-next 08/12] selftests/bpf: add a test to test_align Edward Cree <ecree@solarflare.com> - 2017-06-27 15:10 +0200
  [PATCH v3 net-next 10/12] selftests/bpf: don't try to access past  MAX_PACKET_OFF in test_verifier Edward Cree <ecree@solarflare.com> - 2017-06-27 15:10 +0200
  [PATCH v3 net-next 11/12] selftests/bpf: add tests for subtraction &  negative numbers Edward Cree <ecree@solarflare.com> - 2017-06-27 15:10 +0200
  Re: [PATCH v3 net-next 00/12] bpf: rewrite value tracking in verifier Daniel Borkmann <daniel@iogearbox.net> - 2017-06-28 16:00 +0200
    Re: [PATCH v3 net-next 00/12] bpf: rewrite value tracking in verifier Edward Cree <ecree@solarflare.com> - 2017-06-28 16:20 +0200
      Re: [PATCH v3 net-next 00/12] bpf: rewrite value tracking in verifier Daniel Borkmann <daniel@iogearbox.net> - 2017-06-28 22:50 +0200
        Re: [PATCH v3 net-next 00/12] bpf: rewrite value tracking in verifier Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2017-06-28 23:40 +0200
          Re: [PATCH v3 net-next 00/12] bpf: rewrite value tracking in verifier Edward Cree <ecree@solarflare.com> - 2017-06-30 18:50 +0200
            [TEST PATCH] bpf/verifier: roll back ptr&const handling, and fix  signed bounds Edward Cree <ecree@solarflare.com> - 2017-06-30 19:40 +0200
  Re: [PATCH v3 net-next 02/12] bpf/verifier: rework value tracking Daniel Borkmann <daniel@iogearbox.net> - 2017-06-28 17:20 +0200
    Re: [PATCH v3 net-next 02/12] bpf/verifier: rework value tracking Edward Cree <ecree@solarflare.com> - 2017-06-28 18:10 +0200
      Re: [PATCH v3 net-next 02/12] bpf/verifier: rework value tracking Daniel Borkmann <daniel@iogearbox.net> - 2017-06-28 21:50 +0200
  Re: [PATCH v3 net-next 02/12] bpf/verifier: rework value tracking Daniel Borkmann <daniel@iogearbox.net> - 2017-06-28 20:00 +0200
    Re: [PATCH v3 net-next 02/12] bpf/verifier: rework value tracking Edward Cree <ecree@solarflare.com> - 2017-06-28 20:30 +0200

csiph-web