From: Daniel Borkmann <daniel@iogearbox.net>
To: Edward Cree <ecree@solarflare.com>,
davem@davemloft.net,
Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Alexei Starovoitov <ast@fb.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
iovisor-dev <iovisor-dev@lists.iovisor.org>
Subject: Re: [PATCH v4 net-next 01/13] bpf/verifier: rework value tracking
Date: Mon, 07 Aug 2017 01:35:59 +0200 [thread overview]
Message-ID: <5987A7DF.6080203@iogearbox.net> (raw)
In-Reply-To: <8a5e37eb-2397-c986-79c5-02908fbbdee0@solarflare.com>
On 08/03/2017 06:11 PM, Edward Cree wrote:
> Unifies adjusted and unadjusted register value types (e.g. FRAME_POINTER is
> now just a PTR_TO_STACK with zero offset).
> Tracks value alignment by means of tracking known & unknown bits. This
> also replaces the 'reg->imm' (leading zero bits) calculations for (what
> were) UNKNOWN_VALUEs.
> 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>
[...]
> +static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
> + struct bpf_insn *insn,
> + struct bpf_reg_state *dst_reg,
> + struct bpf_reg_state *src_reg)
> {
> struct bpf_reg_state *regs = env->cur_state.regs;
[...]
> - } else if (insn->imm < BPF_REGISTER_MAX_RANGE &&
> - (s64)insn->imm > BPF_REGISTER_MIN_RANGE) {
> - min_val = max_val = insn->imm;
> - src_align = calc_align(insn->imm);
> + 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);
> }
> -
[...]
> - dst_reg->max_value = BPF_REGISTER_MAX_RANGE;
> + 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);
> }
Looks like the same check was added twice here right after
the first one? Shouldn't we just temporarily coerce the src
reg to 32 bit here given in the actual op the src reg is not
being modified?
Thanks,
Daniel
> + min_val = src_reg->min_value;
> + max_val = src_reg->max_value;
> + src_known = tnum_is_const(src_reg->var_off);
> + dst_known = tnum_is_const(dst_reg->var_off);
>
> switch (opcode) {
next prev parent reply other threads:[~2017-08-06 23:36 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-03 16:07 [PATCH v4 net-next 00/13] bpf: rewrite value tracking in verifier Edward Cree
2017-08-03 16:11 ` [PATCH v4 net-next 01/13] bpf/verifier: rework value tracking Edward Cree
2017-08-06 23:35 ` Daniel Borkmann [this message]
2017-08-07 12:39 ` Edward Cree
2017-08-03 16:11 ` [PATCH v4 net-next 02/13] nfp: change bpf verifier hooks to match new verifier data structures Edward Cree
2017-08-07 4:01 ` David Miller
2017-08-03 16:12 ` [PATCH v4 net-next 03/13] bpf/verifier: track signed and unsigned min/max values Edward Cree
2017-08-03 16:12 ` [PATCH v4 net-next 04/13] bpf/verifier: more concise register state logs for constant var_off Edward Cree
2017-08-03 16:12 ` [PATCH v4 net-next 05/13] selftests/bpf: change test_verifier expectations Edward Cree
2017-08-03 16:13 ` [PATCH v4 net-next 06/13] selftests/bpf: rewrite test_align Edward Cree
2017-08-03 16:13 ` [PATCH v4 net-next 07/13] selftests/bpf: add a test to test_align Edward Cree
2017-08-03 16:14 ` [PATCH v4 net-next 08/13] selftests/bpf: add test for bogus operations on pointers Edward Cree
2017-08-03 16:14 ` [PATCH v4 net-next 09/13] selftests/bpf: don't try to access past MAX_PACKET_OFF in test_verifier Edward Cree
2017-08-03 16:15 ` [PATCH v4 net-next 10/13] selftests/bpf: add tests for subtraction & negative numbers Edward Cree
2017-08-03 16:15 ` [PATCH v4 net-next 11/13] selftests/bpf: variable offset negative tests Edward Cree
2017-08-03 16:15 ` [PATCH v4 net-next 12/13] Documentation: describe the new eBPF verifier value tracking behaviour Edward Cree
2017-08-03 16:16 ` [PATCH v4 net-next 13/13] bpf/verifier: increase complexity limit to 128k Edward Cree
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5987A7DF.6080203@iogearbox.net \
--to=daniel@iogearbox.net \
--cc=alexei.starovoitov@gmail.com \
--cc=ast@fb.com \
--cc=davem@davemloft.net \
--cc=ecree@solarflare.com \
--cc=iovisor-dev@lists.iovisor.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).