1. 29 4月, 2020 5 次提交
    • A
      selftests/bpf: Ensure test flavors use correct skeletons · 76148faa
      Andrii Nakryiko 提交于
      Ensure that test runner flavors include their own skeletons from <flavor>/
      directory. Previously, skeletons generated for no-flavor test_progs were used.
      Apart from fixing correctness, this also makes it possible to compile only
      flavors individually:
      
        $ make clean && make test_progs-no_alu32
        ... now succeeds ...
      
      Fixes: 74b5a596 ("selftests/bpf: Replace test_progs and test_maps w/ general rule")
      Signed-off-by: NAndrii Nakryiko <andriin@fb.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200429012111.277390-2-andriin@fb.com
      76148faa
    • A
      libbpf: Add BTF-defined map-in-map support · 646f02ff
      Andrii Nakryiko 提交于
      As discussed at LPC 2019 ([0]), this patch brings (a quite belated) support
      for declarative BTF-defined map-in-map support in libbpf. It allows to define
      ARRAY_OF_MAPS and HASH_OF_MAPS BPF maps without any user-space initialization
      code involved.
      
      Additionally, it allows to initialize outer map's slots with references to
      respective inner maps at load time, also completely declaratively.
      
      Despite a weak type system of C, the way BTF-defined map-in-map definition
      works, it's actually quite hard to accidentally initialize outer map with
      incompatible inner maps. This being C, of course, it's still possible, but
      even that would be caught at load time and error returned with helpful debug
      log pointing exactly to the slot that failed to be initialized.
      
      As an example, here's a rather advanced HASH_OF_MAPS declaration and
      initialization example, filling slots #0 and #4 with two inner maps:
      
        #include <bpf/bpf_helpers.h>
      
        struct inner_map {
                __uint(type, BPF_MAP_TYPE_ARRAY);
                __uint(max_entries, 1);
                __type(key, int);
                __type(value, int);
        } inner_map1 SEC(".maps"),
          inner_map2 SEC(".maps");
      
        struct outer_hash {
                __uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);
                __uint(max_entries, 5);
                __uint(key_size, sizeof(int));
                __array(values, struct inner_map);
        } outer_hash SEC(".maps") = {
                .values = {
                        [0] = &inner_map2,
                        [4] = &inner_map1,
                },
        };
      
      Here's the relevant part of libbpf debug log showing pretty clearly of what's
      going on with map-in-map initialization:
      
        libbpf: .maps relo #0: for 6 value 0 rel.r_offset 96 name 260 ('inner_map1')
        libbpf: .maps relo #0: map 'outer_arr' slot [0] points to map 'inner_map1'
        libbpf: .maps relo #1: for 7 value 32 rel.r_offset 112 name 249 ('inner_map2')
        libbpf: .maps relo #1: map 'outer_arr' slot [2] points to map 'inner_map2'
        libbpf: .maps relo #2: for 7 value 32 rel.r_offset 144 name 249 ('inner_map2')
        libbpf: .maps relo #2: map 'outer_hash' slot [0] points to map 'inner_map2'
        libbpf: .maps relo #3: for 6 value 0 rel.r_offset 176 name 260 ('inner_map1')
        libbpf: .maps relo #3: map 'outer_hash' slot [4] points to map 'inner_map1'
        libbpf: map 'inner_map1': created successfully, fd=4
        libbpf: map 'inner_map2': created successfully, fd=5
        libbpf: map 'outer_hash': created successfully, fd=7
        libbpf: map 'outer_hash': slot [0] set to map 'inner_map2' fd=5
        libbpf: map 'outer_hash': slot [4] set to map 'inner_map1' fd=4
      
      Notice from the log above that fd=6 (not logged explicitly) is used for inner
      "prototype" map, necessary for creation of outer map. It is destroyed
      immediately after outer map is created.
      
      See also included selftest with some extra comments explaining extra details
      of usage. Additionally, similar initialization syntax and libbpf functionality
      can be used to do initialization of BPF_PROG_ARRAY with references to BPF
      sub-programs. This can be done in follow up patches, if there will be a demand
      for this.
      
        [0] https://linuxplumbersconf.org/event/4/contributions/448/Signed-off-by: NAndrii Nakryiko <andriin@fb.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Acked-by: NToke Høiland-Jørgensen <toke@redhat.com>
      Link: https://lore.kernel.org/bpf/20200429002739.48006-4-andriin@fb.com
      646f02ff
    • A
      selftests/bpf: Test bpf_link's get_next_id, get_fd_by_id, and get_obj_info · 2c2837b0
      Andrii Nakryiko 提交于
      Extend bpf_obj_id selftest to verify bpf_link's observability APIs.
      Signed-off-by: NAndrii Nakryiko <andriin@fb.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200429001614.1544-7-andriin@fb.com
      2c2837b0
    • A
      selftests/bpf: fix test_sysctl_prog with alu32 · 9b329d0d
      Alexei Starovoitov 提交于
      Similar to commit b7a0d65d ("bpf, testing: Workaround a verifier failure for test_progs")
      fix test_sysctl_prog.c as well.
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      9b329d0d
    • V
      selftests/bpf: Copy runqslower to OUTPUT directory · b26d1e2b
      Veronika Kabatova 提交于
      $(OUTPUT)/runqslower makefile target doesn't actually create runqslower
      binary in the $(OUTPUT) directory. As lib.mk expects all
      TEST_GEN_PROGS_EXTENDED (which runqslower is a part of) to be present in
      the OUTPUT directory, this results in an error when running e.g. `make
      install`:
      
      rsync: link_stat "tools/testing/selftests/bpf/runqslower" failed: No
             such file or directory (2)
      
      Copy the binary into the OUTPUT directory after building it to fix the
      error.
      
      Fixes: 3a0d3092 ("selftests/bpf: Build runqslower from selftests")
      Signed-off-by: NVeronika Kabatova <vkabatov@redhat.com>
      Signed-off-by: NDaniel Borkmann <daniel@iogearbox.net>
      Acked-by: NAndrii Nakryiko <andriin@fb.com>
      Link: https://lore.kernel.org/bpf/20200428173742.2988395-1-vkabatov@redhat.com
      b26d1e2b
  2. 27 4月, 2020 2 次提交
    • L
      selftests/bpf: Add cls_redirect classifier · 23458901
      Lorenz Bauer 提交于
      cls_redirect is a TC clsact based replacement for the glb-redirect iptables
      module available at [1]. It enables what GitHub calls "second chance"
      flows [2], similarly proposed by the Beamer paper [3]. In contrast to
      glb-redirect, it also supports migrating UDP flows as long as connected
      sockets are used. cls_redirect is in production at Cloudflare, as part of
      our own L4 load balancer.
      
      We have modified the encapsulation format slightly from glb-redirect:
      glbgue_chained_routing.private_data_type has been repurposed to form a
      version field and several flags. Both have been arranged in a way that
      a private_data_type value of zero matches the current glb-redirect
      behaviour. This means that cls_redirect will understand packets in
      glb-redirect format, but not vice versa.
      
      The test suite only covers basic features. For example, cls_redirect will
      correctly forward path MTU discovery packets, but this is not exercised.
      It is also possible to switch the encapsulation format to GRE on the last
      hop, which is also not tested.
      
      There are two major distinctions from glb-redirect: first, cls_redirect
      relies on receiving encapsulated packets directly from a router. This is
      because we don't have access to the neighbour tables from BPF, yet. See
      forward_to_next_hop for details. Second, cls_redirect performs decapsulation
      instead of using separate ipip and sit tunnel devices. This
      avoids issues with the sit tunnel [4] and makes deploying the classifier
      easier: decapsulated packets appear on the same interface, so existing
      firewall rules continue to work as expected.
      
      The code base started it's life on v4.19, so there are most likely still
      hold overs from old workarounds. In no particular order:
      
      - The function buf_off is required to defeat a clang optimization
        that leads to the verifier rejecting the program due to pointer
        arithmetic in the wrong order.
      
      - The function pkt_parse_ipv6 is force inlined, because it would
        otherwise be rejected due to returning a pointer to stack memory.
      
      - The functions fill_tuple and classify_tcp contain kludges, because
        we've run out of function arguments.
      
      - The logic in general is rather nested, due to verifier restrictions.
        I think this is either because the verifier loses track of constants
        on the stack, or because it can't track enum like variables.
      
      1: https://github.com/github/glb-director/tree/master/src/glb-redirect
      2: https://github.com/github/glb-director/blob/master/docs/development/second-chance-design.md
      3: https://www.usenix.org/conference/nsdi18/presentation/olteanu
      4: https://github.com/github/glb-director/issues/64Signed-off-by: NLorenz Bauer <lmb@cloudflare.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200424185556.7358-2-lmb@cloudflare.com
      23458901
    • A
      bpf: Make verifier log more relevant by default · 6f8a57cc
      Andrii Nakryiko 提交于
      To make BPF verifier verbose log more releavant and easier to use to debug
      verification failures, "pop" parts of log that were successfully verified.
      This has effect of leaving only verifier logs that correspond to code branches
      that lead to verification failure, which in practice should result in much
      shorter and more relevant verifier log dumps. This behavior is made the
      default behavior and can be overriden to do exhaustive logging by specifying
      BPF_LOG_LEVEL2 log level.
      
      Using BPF_LOG_LEVEL2 to disable this behavior is not ideal, because in some
      cases it's good to have BPF_LOG_LEVEL2 per-instruction register dump
      verbosity, but still have only relevant verifier branches logged. But for this
      patch, I didn't want to add any new flags. It might be worth-while to just
      rethink how BPF verifier logging is performed and requested and streamline it
      a bit. But this trimming of successfully verified branches seems to be useful
      and a good default behavior.
      
      To test this, I modified runqslower slightly to introduce read of
      uninitialized stack variable. Log (**truncated in the middle** to save many
      lines out of this commit message) BEFORE this change:
      
      ; int handle__sched_switch(u64 *ctx)
      0: (bf) r6 = r1
      ; struct task_struct *prev = (struct task_struct *)ctx[1];
      1: (79) r1 = *(u64 *)(r6 +8)
      func 'sched_switch' arg1 has btf_id 151 type STRUCT 'task_struct'
      2: (b7) r2 = 0
      ; struct event event = {};
      3: (7b) *(u64 *)(r10 -24) = r2
      last_idx 3 first_idx 0
      regs=4 stack=0 before 2: (b7) r2 = 0
      4: (7b) *(u64 *)(r10 -32) = r2
      5: (7b) *(u64 *)(r10 -40) = r2
      6: (7b) *(u64 *)(r10 -48) = r2
      ; if (prev->state == TASK_RUNNING)
      
      [ ... instruction dump from insn #7 through #50 are cut out ... ]
      
      51: (b7) r2 = 16
      52: (85) call bpf_get_current_comm#16
      last_idx 52 first_idx 42
      regs=4 stack=0 before 51: (b7) r2 = 16
      ; bpf_perf_event_output(ctx, &events, BPF_F_CURRENT_CPU,
      53: (bf) r1 = r6
      54: (18) r2 = 0xffff8881f3868800
      56: (18) r3 = 0xffffffff
      58: (bf) r4 = r7
      59: (b7) r5 = 32
      60: (85) call bpf_perf_event_output#25
      last_idx 60 first_idx 53
      regs=20 stack=0 before 59: (b7) r5 = 32
      61: (bf) r2 = r10
      ; event.pid = pid;
      62: (07) r2 += -16
      ; bpf_map_delete_elem(&start, &pid);
      63: (18) r1 = 0xffff8881f3868000
      65: (85) call bpf_map_delete_elem#3
      ; }
      66: (b7) r0 = 0
      67: (95) exit
      
      from 44 to 66: safe
      
      from 34 to 66: safe
      
      from 11 to 28: R1_w=inv0 R2_w=inv0 R6_w=ctx(id=0,off=0,imm=0) R10=fp0 fp-8=mmmm???? fp-24_w=00000000 fp-32_w=00000000 fp-40_w=00000000 fp-48_w=00000000
      ; bpf_map_update_elem(&start, &pid, &ts, 0);
      28: (bf) r2 = r10
      ;
      29: (07) r2 += -16
      ; tsp = bpf_map_lookup_elem(&start, &pid);
      30: (18) r1 = 0xffff8881f3868000
      32: (85) call bpf_map_lookup_elem#1
      invalid indirect read from stack off -16+0 size 4
      processed 65 insns (limit 1000000) max_states_per_insn 1 total_states 5 peak_states 5 mark_read 4
      
      Notice how there is a successful code path from instruction 0 through 67, few
      successfully verified jumps (44->66, 34->66), and only after that 11->28 jump
      plus error on instruction #32.
      
      AFTER this change (full verifier log, **no truncation**):
      
      ; int handle__sched_switch(u64 *ctx)
      0: (bf) r6 = r1
      ; struct task_struct *prev = (struct task_struct *)ctx[1];
      1: (79) r1 = *(u64 *)(r6 +8)
      func 'sched_switch' arg1 has btf_id 151 type STRUCT 'task_struct'
      2: (b7) r2 = 0
      ; struct event event = {};
      3: (7b) *(u64 *)(r10 -24) = r2
      last_idx 3 first_idx 0
      regs=4 stack=0 before 2: (b7) r2 = 0
      4: (7b) *(u64 *)(r10 -32) = r2
      5: (7b) *(u64 *)(r10 -40) = r2
      6: (7b) *(u64 *)(r10 -48) = r2
      ; if (prev->state == TASK_RUNNING)
      7: (79) r2 = *(u64 *)(r1 +16)
      ; if (prev->state == TASK_RUNNING)
      8: (55) if r2 != 0x0 goto pc+19
       R1_w=ptr_task_struct(id=0,off=0,imm=0) R2_w=inv0 R6_w=ctx(id=0,off=0,imm=0) R10=fp0 fp-24_w=00000000 fp-32_w=00000000 fp-40_w=00000000 fp-48_w=00000000
      ; trace_enqueue(prev->tgid, prev->pid);
      9: (61) r1 = *(u32 *)(r1 +1184)
      10: (63) *(u32 *)(r10 -4) = r1
      ; if (!pid || (targ_pid && targ_pid != pid))
      11: (15) if r1 == 0x0 goto pc+16
      
      from 11 to 28: R1_w=inv0 R2_w=inv0 R6_w=ctx(id=0,off=0,imm=0) R10=fp0 fp-8=mmmm???? fp-24_w=00000000 fp-32_w=00000000 fp-40_w=00000000 fp-48_w=00000000
      ; bpf_map_update_elem(&start, &pid, &ts, 0);
      28: (bf) r2 = r10
      ;
      29: (07) r2 += -16
      ; tsp = bpf_map_lookup_elem(&start, &pid);
      30: (18) r1 = 0xffff8881db3ce800
      32: (85) call bpf_map_lookup_elem#1
      invalid indirect read from stack off -16+0 size 4
      processed 65 insns (limit 1000000) max_states_per_insn 1 total_states 5 peak_states 5 mark_read 4
      
      Notice how in this case, there are 0-11 instructions + jump from 11 to
      28 is recorded + 28-32 instructions with error on insn #32.
      
      test_verifier test runner was updated to specify BPF_LOG_LEVEL2 for
      VERBOSE_ACCEPT expected result due to potentially "incomplete" success verbose
      log at BPF_LOG_LEVEL1.
      
      On success, verbose log will only have a summary of number of processed
      instructions, etc, but no branch tracing log. Having just a last succesful
      branch tracing seemed weird and confusing. Having small and clean summary log
      in success case seems quite logical and nice, though.
      Signed-off-by: NAndrii Nakryiko <andriin@fb.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200423195850.1259827-1-andriin@fb.com
      6f8a57cc
  3. 26 4月, 2020 1 次提交
    • S
      bpf: Enable more helpers for BPF_PROG_TYPE_CGROUP_{DEVICE,SYSCTL,SOCKOPT} · 0456ea17
      Stanislav Fomichev 提交于
      Currently the following prog types don't fall back to bpf_base_func_proto()
      (instead they have cgroup_base_func_proto which has a limited set of
      helpers from bpf_base_func_proto):
      * BPF_PROG_TYPE_CGROUP_DEVICE
      * BPF_PROG_TYPE_CGROUP_SYSCTL
      * BPF_PROG_TYPE_CGROUP_SOCKOPT
      
      I don't see any specific reason why we shouldn't use bpf_base_func_proto(),
      every other type of program (except bpf-lirc and, understandably, tracing)
      use it, so let's fall back to bpf_base_func_proto for those prog types
      as well.
      
      This basically boils down to adding access to the following helpers:
      * BPF_FUNC_get_prandom_u32
      * BPF_FUNC_get_smp_processor_id
      * BPF_FUNC_get_numa_node_id
      * BPF_FUNC_tail_call
      * BPF_FUNC_ktime_get_ns
      * BPF_FUNC_spin_lock (CAP_SYS_ADMIN)
      * BPF_FUNC_spin_unlock (CAP_SYS_ADMIN)
      * BPF_FUNC_jiffies64 (CAP_SYS_ADMIN)
      
      I've also added bpf_perf_event_output() because it's really handy for
      logging and debugging.
      Signed-off-by: NStanislav Fomichev <sdf@google.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200420174610.77494-1-sdf@google.com
      0456ea17
  4. 25 4月, 2020 2 次提交
    • S
      selftests/bpf: Fix a couple of broken test_btf cases · e1cebd84
      Stanislav Fomichev 提交于
      Commit 51c39bb1 ("bpf: Introduce function-by-function verification")
      introduced function linkage flag and changed the error message from
      "vlen != 0" to "Invalid func linkage" and broke some fake BPF programs.
      
      Adjust the test accordingly.
      
      AFACT, the programs don't really need any arguments and only look
      at BTF for maps, so let's drop the args altogether.
      
      Before:
      BTF raw test[103] (func (Non zero vlen)): do_test_raw:3703:FAIL expected
      err_str:vlen != 0
      magic: 0xeb9f
      version: 1
      flags: 0x0
      hdr_len: 24
      type_off: 0
      type_len: 72
      str_off: 72
      str_len: 10
      btf_total_size: 106
      [1] INT (anon) size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
      [2] INT (anon) size=4 bits_offset=0 nr_bits=32 encoding=(none)
      [3] FUNC_PROTO (anon) return=0 args=(1 a, 2 b)
      [4] FUNC func type_id=3 Invalid func linkage
      
      BTF libbpf test[1] (test_btf_haskv.o): libbpf: load bpf program failed:
      Invalid argument
      libbpf: -- BEGIN DUMP LOG ---
      libbpf:
      Validating test_long_fname_2() func#1...
      Arg#0 type PTR in test_long_fname_2() is not supported yet.
      processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
      peak_states 0 mark_read 0
      
      libbpf: -- END LOG --
      libbpf: failed to load program 'dummy_tracepoint'
      libbpf: failed to load object 'test_btf_haskv.o'
      do_test_file:4201:FAIL bpf_object__load: -4007
      BTF libbpf test[2] (test_btf_newkv.o): libbpf: load bpf program failed:
      Invalid argument
      libbpf: -- BEGIN DUMP LOG ---
      libbpf:
      Validating test_long_fname_2() func#1...
      Arg#0 type PTR in test_long_fname_2() is not supported yet.
      processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
      peak_states 0 mark_read 0
      
      libbpf: -- END LOG --
      libbpf: failed to load program 'dummy_tracepoint'
      libbpf: failed to load object 'test_btf_newkv.o'
      do_test_file:4201:FAIL bpf_object__load: -4007
      BTF libbpf test[3] (test_btf_nokv.o): libbpf: load bpf program failed:
      Invalid argument
      libbpf: -- BEGIN DUMP LOG ---
      libbpf:
      Validating test_long_fname_2() func#1...
      Arg#0 type PTR in test_long_fname_2() is not supported yet.
      processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
      peak_states 0 mark_read 0
      
      libbpf: -- END LOG --
      libbpf: failed to load program 'dummy_tracepoint'
      libbpf: failed to load object 'test_btf_nokv.o'
      do_test_file:4201:FAIL bpf_object__load: -4007
      
      Fixes: 51c39bb1 ("bpf: Introduce function-by-function verification")
      Signed-off-by: NStanislav Fomichev <sdf@google.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200422003753.124921-1-sdf@google.com
      e1cebd84
    • T
      selftests/bpf: Add test for freplace program with expected_attach_type · 1d8a0af5
      Toke Høiland-Jørgensen 提交于
      This adds a new selftest that tests the ability to attach an freplace
      program to a program type that relies on the expected_attach_type of the
      target program to pass verification.
      Signed-off-by: NToke Høiland-Jørgensen <toke@redhat.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/158773526831.293902.16011743438619684815.stgit@toke.dk
      1d8a0af5
  5. 21 4月, 2020 2 次提交
    • L
      bpf, selftests: Add test for BPF_STX BPF_B storing R10 · d2b6c3ab
      Luke Nelson 提交于
      This patch adds a test to test_verifier that writes the lower 8 bits of
      R10 (aka FP) using BPF_B to an array map and reads the result back. The
      expected behavior is that the result should be the same as first copying
      R10 to R9, and then storing / loading the lower 8 bits of R9.
      
      This test catches a bug that was present in the x86-64 JIT that caused
      an incorrect encoding for BPF_STX BPF_B when the source operand is R10.
      Signed-off-by: NXi Wang <xi.wang@gmail.com>
      Signed-off-by: NLuke Nelson <luke.r.nels@gmail.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200418232655.23870-2-luke.r.nels@gmail.com
      d2b6c3ab
    • J
      bpf: Forbid XADD on spilled pointers for unprivileged users · 6e7e63cb
      Jann Horn 提交于
      When check_xadd() verifies an XADD operation on a pointer to a stack slot
      containing a spilled pointer, check_stack_read() verifies that the read,
      which is part of XADD, is valid. However, since the placeholder value -1 is
      passed as `value_regno`, check_stack_read() can only return a binary
      decision and can't return the type of the value that was read. The intent
      here is to verify whether the value read from the stack slot may be used as
      a SCALAR_VALUE; but since check_stack_read() doesn't check the type, and
      the type information is lost when check_stack_read() returns, this is not
      enforced, and a malicious user can abuse XADD to leak spilled kernel
      pointers.
      
      Fix it by letting check_stack_read() verify that the value is usable as a
      SCALAR_VALUE if no type information is passed to the caller.
      
      To be able to use __is_pointer_value() in check_stack_read(), move it up.
      
      Fix up the expected unprivileged error message for a BPF selftest that,
      until now, assumed that unprivileged users can use XADD on stack-spilled
      pointers. This also gives us a test for the behavior introduced in this
      patch for free.
      
      In theory, this could also be fixed by forbidding XADD on stack spills
      entirely, since XADD is a locked operation (for operations on memory with
      concurrency) and there can't be any concurrency on the BPF stack; but
      Alexei has said that he wants to keep XADD on stack slots working to avoid
      changes to the test suite [1].
      
      The following BPF program demonstrates how to leak a BPF map pointer as an
      unprivileged user using this bug:
      
          // r7 = map_pointer
          BPF_LD_MAP_FD(BPF_REG_7, small_map),
          // r8 = launder(map_pointer)
          BPF_STX_MEM(BPF_DW, BPF_REG_FP, BPF_REG_7, -8),
          BPF_MOV64_IMM(BPF_REG_1, 0),
          ((struct bpf_insn) {
            .code  = BPF_STX | BPF_DW | BPF_XADD,
            .dst_reg = BPF_REG_FP,
            .src_reg = BPF_REG_1,
            .off = -8
          }),
          BPF_LDX_MEM(BPF_DW, BPF_REG_8, BPF_REG_FP, -8),
      
          // store r8 into map
          BPF_MOV64_REG(BPF_REG_ARG1, BPF_REG_7),
          BPF_MOV64_REG(BPF_REG_ARG2, BPF_REG_FP),
          BPF_ALU64_IMM(BPF_ADD, BPF_REG_ARG2, -4),
          BPF_ST_MEM(BPF_W, BPF_REG_ARG2, 0, 0),
          BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
          BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
          BPF_EXIT_INSN(),
          BPF_STX_MEM(BPF_DW, BPF_REG_0, BPF_REG_8, 0),
      
          BPF_MOV64_IMM(BPF_REG_0, 0),
          BPF_EXIT_INSN()
      
      [1] https://lore.kernel.org/bpf/20200416211116.qxqcza5vo2ddnkdq@ast-mbp.dhcp.thefacebook.com/
      
      Fixes: 17a52670 ("bpf: verifier (add verifier core)")
      Signed-off-by: NJann Horn <jannh@google.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200417000007.10734-1-jannh@google.com
      6e7e63cb
  6. 15 4月, 2020 3 次提交
  7. 08 4月, 2020 1 次提交
  8. 03 4月, 2020 2 次提交
  9. 31 3月, 2020 7 次提交
  10. 30 3月, 2020 2 次提交
  11. 29 3月, 2020 1 次提交
  12. 28 3月, 2020 1 次提交
  13. 26 3月, 2020 1 次提交
    • J
      bpf: Test_verifier, #70 error message updates for 32-bit right shift · aa131ed4
      John Fastabend 提交于
      After changes to add update_reg_bounds after ALU ops and adding ALU32
      bounds tracking the error message is changed in the 32-bit right shift
      tests.
      
      Test "#70/u bounds check after 32-bit right shift with 64-bit input FAIL"
      now fails with,
      
      Unexpected error message!
      	EXP: R0 invalid mem access
      	RES: func#0 @0
      
      7: (b7) r1 = 2
      8: R0_w=map_value(id=0,off=0,ks=8,vs=8,imm=0) R1_w=invP2 R10=fp0 fp-8_w=mmmmmmmm
      8: (67) r1 <<= 31
      9: R0_w=map_value(id=0,off=0,ks=8,vs=8,imm=0) R1_w=invP4294967296 R10=fp0 fp-8_w=mmmmmmmm
      9: (74) w1 >>= 31
      10: R0_w=map_value(id=0,off=0,ks=8,vs=8,imm=0) R1_w=invP0 R10=fp0 fp-8_w=mmmmmmmm
      10: (14) w1 -= 2
      11: R0_w=map_value(id=0,off=0,ks=8,vs=8,imm=0) R1_w=invP4294967294 R10=fp0 fp-8_w=mmmmmmmm
      11: (0f) r0 += r1
      math between map_value pointer and 4294967294 is not allowed
      
      And test "#70/p bounds check after 32-bit right shift with 64-bit input
      FAIL" now fails with,
      
      Unexpected error message!
      	EXP: R0 invalid mem access
      	RES: func#0 @0
      
      7: (b7) r1 = 2
      8: R0_w=map_value(id=0,off=0,ks=8,vs=8,imm=0) R1_w=inv2 R10=fp0 fp-8_w=mmmmmmmm
      8: (67) r1 <<= 31
      9: R0_w=map_value(id=0,off=0,ks=8,vs=8,imm=0) R1_w=inv4294967296 R10=fp0 fp-8_w=mmmmmmmm
      9: (74) w1 >>= 31
      10: R0_w=map_value(id=0,off=0,ks=8,vs=8,imm=0) R1_w=inv0 R10=fp0 fp-8_w=mmmmmmmm
      10: (14) w1 -= 2
      11: R0_w=map_value(id=0,off=0,ks=8,vs=8,imm=0) R1_w=inv4294967294 R10=fp0 fp-8_w=mmmmmmmm
      11: (0f) r0 += r1
      last_idx 11 first_idx 0
      regs=2 stack=0 before 10: (14) w1 -= 2
      regs=2 stack=0 before 9: (74) w1 >>= 31
      regs=2 stack=0 before 8: (67) r1 <<= 31
      regs=2 stack=0 before 7: (b7) r1 = 2
      math between map_value pointer and 4294967294 is not allowed
      
      Before this series we did not trip the "math between map_value pointer..."
      error because check_reg_sane_offset is never called in
      adjust_ptr_min_max_vals(). Instead we have a register state that looks
      like this at line 11*,
      
      11: R0_w=map_value(id=0,off=0,ks=8,vs=8,
                         smin_value=0,smax_value=0,
                         umin_value=0,umax_value=0,
                         var_off=(0x0; 0x0))
          R1_w=invP(id=0,
                    smin_value=0,smax_value=4294967295,
                    umin_value=0,umax_value=4294967295,
                    var_off=(0xfffffffe; 0x0))
          R10=fp(id=0,off=0,
                 smin_value=0,smax_value=0,
                 umin_value=0,umax_value=0,
                 var_off=(0x0; 0x0)) fp-8_w=mmmmmmmm
      11: (0f) r0 += r1
      
      In R1 'smin_val != smax_val' yet we have a tnum_const as seen
      by 'var_off(0xfffffffe; 0x0))' with a 0x0 mask. So we hit this check
      in adjust_ptr_min_max_vals()
      
       if ((known && (smin_val != smax_val || umin_val != umax_val)) ||
            smin_val > smax_val || umin_val > umax_val) {
             /* Taint dst register if offset had invalid bounds derived from
              * e.g. dead branches.
              */
             __mark_reg_unknown(env, dst_reg);
             return 0;
       }
      
      So we don't throw an error here and instead only throw an error
      later in the verification when the memory access is made.
      
      The root cause in verifier without alu32 bounds tracking is having
      'umin_value = 0' and 'umax_value = U64_MAX' from BPF_SUB which we set
      when 'umin_value < umax_val' here,
      
       if (dst_reg->umin_value < umax_val) {
          /* Overflow possible, we know nothing */
          dst_reg->umin_value = 0;
          dst_reg->umax_value = U64_MAX;
       } else { ...}
      
      Later in adjust_calar_min_max_vals we previously did a
      coerce_reg_to_size() which will clamp the U64_MAX to U32_MAX by
      truncating to 32bits. But either way without a call to update_reg_bounds
      the less precise bounds tracking will fall out of the alu op
      verification.
      
      After latest changes we now exit adjust_scalar_min_max_vals with the
      more precise umin value, due to zero extension propogating bounds from
      alu32 bounds into alu64 bounds and then calling update_reg_bounds.
      This then causes the verifier to trigger an earlier error and we get
      the error in the output above.
      
      This patch updates tests to reflect new error message.
      
      * I have a local patch to print entire verifier state regardless if we
       believe it is a constant so we can get a full picture of the state.
       Usually if tnum_is_const() then bounds are also smin=smax, etc. but
       this is not always true and is a bit subtle. Being able to see these
       states helps understand dataflow imo. Let me know if we want something
       similar upstream.
      Signed-off-by: NJohn Fastabend <john.fastabend@gmail.com>
      Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/158507161475.15666.3061518385241144063.stgit@john-Precision-5820-Tower
      aa131ed4
  14. 25 3月, 2020 1 次提交
  15. 24 3月, 2020 2 次提交
  16. 21 3月, 2020 1 次提交
  17. 18 3月, 2020 5 次提交
  18. 14 3月, 2020 1 次提交