1. 08 Feb, 2024 3 commits
  2. 07 Feb, 2024 3 commits
    • Andrii Nakryiko's avatar
      Merge branch 'tools-resolve_btfids-fix-cross-compilation-to-non-host-endianness' · abae1ac5
      Andrii Nakryiko authored
      Viktor Malik says:
      
      ====================
      tools/resolve_btfids: fix cross-compilation to non-host endianness
      
      The .BTF_ids section is pre-filled with zeroed BTF ID entries during the
      build and afterwards patched by resolve_btfids with correct values.
      Since resolve_btfids always writes in host-native endianness, it relies
      on libelf to do the translation when the target ELF is cross-compiled to
      a different endianness (this was introduced in commit 61e8aeda
      ("bpf: Fix libelf endian handling in resolv_btfids")).
      
      Unfortunately, the translation will corrupt the flags fields of SET8
      entries because these were written during vmlinux compilation and are in
      the correct endianness already. This will lead to numerous selftests
      failures such as:
      
          $ sudo ./test_verifier 502 502
          #502/p sleepable fentry accept FAIL
          Failed to load prog 'Invalid argument'!
          bpf_fentry_test1 is not sleepable
          verification time 34 usec
          stack depth 0
          processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
          Summary: 0 PASSED, 0 SKIPPED, 1 FAILED
      
      Since it's not possible to instruct libelf to translate just certain
      values, let's manually bswap the flags (both global and entry flags) in
      resolve_btfids when needed, so that libelf then translates everything
      correctly.
      
      The first patch of the series refactors resolve_btfids by using types
      from btf_ids.h instead of accessing the BTF ID data using magic offsets.
      Acked-by: default avatarJiri Olsa <jolsa@kernel.org>
      ---
      Changes in v4:
      - remove unnecessary vars and pointer casts (suggested by Daniel Xu)
      
      Changes in v3:
      - add byte swap of global 'flags' field in btf_id_set8 (suggested by
        Jiri Olsa)
      - cleaner refactoring of sets_patch (suggested by Jiri Olsa)
      - add compile-time assertion that IDs are at the beginning of pairs
        struct in btf_id_set8 (suggested by Daniel Borkmann)
      
      Changes in v2:
      - use type defs from btf_ids.h (suggested by Andrii Nakryiko)
      ====================
      
      Link: https://lore.kernel.org/r/cover.1707223196.git.vmalik@redhat.comSigned-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      abae1ac5
    • Viktor Malik's avatar
      tools/resolve_btfids: Fix cross-compilation to non-host endianness · 903fad43
      Viktor Malik authored
      The .BTF_ids section is pre-filled with zeroed BTF ID entries during the
      build and afterwards patched by resolve_btfids with correct values.
      Since resolve_btfids always writes in host-native endianness, it relies
      on libelf to do the translation when the target ELF is cross-compiled to
      a different endianness (this was introduced in commit 61e8aeda
      ("bpf: Fix libelf endian handling in resolv_btfids")).
      
      Unfortunately, the translation will corrupt the flags fields of SET8
      entries because these were written during vmlinux compilation and are in
      the correct endianness already. This will lead to numerous selftests
      failures such as:
      
          $ sudo ./test_verifier 502 502
          #502/p sleepable fentry accept FAIL
          Failed to load prog 'Invalid argument'!
          bpf_fentry_test1 is not sleepable
          verification time 34 usec
          stack depth 0
          processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
          Summary: 0 PASSED, 0 SKIPPED, 1 FAILED
      
      Since it's not possible to instruct libelf to translate just certain
      values, let's manually bswap the flags (both global and entry flags) in
      resolve_btfids when needed, so that libelf then translates everything
      correctly.
      
      Fixes: ef2c6f37 ("tools/resolve_btfids: Add support for 8-byte BTF sets")
      Signed-off-by: default avatarViktor Malik <vmalik@redhat.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Link: https://lore.kernel.org/bpf/7b6bff690919555574ce0f13d2a5996cacf7bf69.1707223196.git.vmalik@redhat.com
      903fad43
    • Viktor Malik's avatar
      tools/resolve_btfids: Refactor set sorting with types from btf_ids.h · 9707ac4f
      Viktor Malik authored
      Instead of using magic offsets to access BTF ID set data, leverage types
      from btf_ids.h (btf_id_set and btf_id_set8) which define the actual
      layout of the data. Thanks to this change, set sorting should also
      continue working if the layout changes.
      
      This requires to sync the definition of 'struct btf_id_set8' from
      include/linux/btf_ids.h to tools/include/linux/btf_ids.h. We don't sync
      the rest of the file at the moment, b/c that would require to also sync
      multiple dependent headers and we don't need any other defs from
      btf_ids.h.
      Signed-off-by: default avatarViktor Malik <vmalik@redhat.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Acked-by: default avatarDaniel Xu <dxu@dxuuu.xyz>
      Link: https://lore.kernel.org/bpf/ff7f062ddf6a00815fda3087957c4ce667f50532.1707223196.git.vmalik@redhat.com
      9707ac4f
  3. 06 Feb, 2024 14 commits
  4. 05 Feb, 2024 5 commits
  5. 03 Feb, 2024 4 commits
  6. 02 Feb, 2024 8 commits
    • Shung-Hsi Yu's avatar
      selftests/bpf: trace_helpers.c: do not use poisoned type · a68b50f4
      Shung-Hsi Yu authored
      After commit c698eaeb ("selftests/bpf: trace_helpers.c: Optimize
      kallsyms cache") trace_helpers.c now includes libbpf_internal.h, and
      thus can no longer use the u32 type (among others) since they are poison
      in libbpf_internal.h. Replace u32 with __u32 to fix the following error
      when building trace_helpers.c on powerpc:
      
        error: attempt to use poisoned "u32"
      
      Fixes: c698eaeb ("selftests/bpf: trace_helpers.c: Optimize kallsyms cache")
      Signed-off-by: default avatarShung-Hsi Yu <shung-hsi.yu@suse.com>
      Acked-by: default avatarJiri Olsa <jolsa@kernel.org>
      Link: https://lore.kernel.org/r/20240202095559.12900-1-shung-hsi.yu@suse.comSigned-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>
      a68b50f4
    • Andrii Nakryiko's avatar
      Merge branch 'improvements-for-tracking-scalars-in-the-bpf-verifier' · 6fb3f727
      Andrii Nakryiko authored
      Maxim Mikityanskiy says:
      
      ====================
      Improvements for tracking scalars in the BPF verifier
      
      From: Maxim Mikityanskiy <maxim@isovalent.com>
      
      The goal of this series is to extend the verifier's capabilities of
      tracking scalars when they are spilled to stack, especially when the
      spill or fill is narrowing. It also contains a fix by Eduard for
      infinite loop detection and a state pruning optimization by Eduard that
      compensates for a verification complexity regression introduced by
      tracking unbounded scalars. These improvements reduce the surface of
      false rejections that I saw while working on Cilium codebase.
      
      Patches 1-9 of the original series were previously applied in v2.
      
      Patches 1-2 (Maxim): Support the case when boundary checks are first
      performed after the register was spilled to the stack.
      
      Patches 3-4 (Maxim): Support narrowing fills.
      
      Patches 5-6 (Eduard): Optimization for state pruning in stacksafe() to
      mitigate the verification complexity regression.
      
      veristat -e file,prog,states -f '!states_diff<50' -f '!states_pct<10' -f '!states_a<10' -f '!states_b<10' -C ...
      
       * Without patch 5:
      
      File                  Program   States (A)  States (B)  States    (DIFF)
      --------------------  --------  ----------  ----------  ----------------
      pyperf100.bpf.o       on_event        4878        6528   +1650 (+33.83%)
      pyperf180.bpf.o       on_event        6936       11032   +4096 (+59.05%)
      pyperf600.bpf.o       on_event       22271       39455  +17184 (+77.16%)
      pyperf600_iter.bpf.o  on_event         400         490     +90 (+22.50%)
      strobemeta.bpf.o      on_event        4895       14028  +9133 (+186.58%)
      
       * With patch 5:
      
      File                     Program        States (A)  States (B)  States   (DIFF)
      -----------------------  -------------  ----------  ----------  ---------------
      bpf_xdp.o                tail_lb_ipv4         2770        2224   -546 (-19.71%)
      pyperf100.bpf.o          on_event             4878        5848   +970 (+19.89%)
      pyperf180.bpf.o          on_event             6936        8868  +1932 (+27.85%)
      pyperf600.bpf.o          on_event            22271       29656  +7385 (+33.16%)
      pyperf600_iter.bpf.o     on_event              400         450    +50 (+12.50%)
      xdp_synproxy_kern.bpf.o  syncookie_tc          280         226    -54 (-19.29%)
      xdp_synproxy_kern.bpf.o  syncookie_xdp         302         228    -74 (-24.50%)
      
      v2 changes:
      
      Fixed comments in patch 1, moved endianness checks to header files in
      patch 12 where possible, added Eduard's ACKs.
      
      v3 changes:
      
      Maxim: Removed __is_scalar_unbounded altogether, addressed Andrii's
      comments.
      
      Eduard: Patch #5 (#14 in v2) changed significantly:
      - Logical changes:
        - Handling of STACK_{MISC,ZERO} mix turned out to be incorrect:
          a mix of MISC and ZERO in old state is not equivalent to e.g.
          just MISC is current state, because verifier could have deduced
          zero scalars from ZERO slots in old state for some loads.
        - There is no reason to limit the change only to cases when
          old or current stack is a spill of unbounded scalar,
          it is valid to compare any 64-bit scalar spill with fake
          register impersonating MISC.
        - STACK_ZERO vs spilled zero case was dropped,
          after recent changes for zero handling by Andrii and Yonghong
          it is hard (impossible?) to conjure all ZERO slots for an spi.
          => the case does not make any difference in veristat results.
      - Use global static variable for unbound_reg (Andrii)
      - Code shuffling to remove duplication in stacksafe() (Andrii)
      ====================
      
      Link: https://lore.kernel.org/r/20240127175237.526726-1-maxtram95@gmail.comSigned-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      6fb3f727
    • Eduard Zingerman's avatar
      selftests/bpf: States pruning checks for scalar vs STACK_MISC · 73a28d9d
      Eduard Zingerman authored
      Check that stacksafe() compares spilled scalars with STACK_MISC.
      The following combinations are explored:
      - old spill of imprecise scalar is equivalent to cur STACK_{MISC,INVALID}
        (plus error in unpriv mode);
      - old spill of precise scalar is not equivalent to cur STACK_MISC;
      - old STACK_MISC is equivalent to cur scalar;
      - old STACK_MISC is not equivalent to cur non-scalar.
      Signed-off-by: default avatarEduard Zingerman <eddyz87@gmail.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Link: https://lore.kernel.org/bpf/20240127175237.526726-7-maxtram95@gmail.com
      73a28d9d
    • Eduard Zingerman's avatar
      bpf: Handle scalar spill vs all MISC in stacksafe() · 6efbde20
      Eduard Zingerman authored
      When check_stack_read_fixed_off() reads value from an spi
      all stack slots of which are set to STACK_{MISC,INVALID},
      the destination register is set to unbound SCALAR_VALUE.
      
      Exploit this fact by allowing stacksafe() to use a fake
      unbound scalar register to compare 'mmmm mmmm' stack value
      in old state vs spilled 64-bit scalar in current state
      and vice versa.
      
      Veristat results after this patch show some gains:
      
      ./veristat -C -e file,prog,states -f 'states_pct>10'  not-opt after
      File                     Program                States   (DIFF)
      -----------------------  ---------------------  ---------------
      bpf_overlay.o            tail_rev_nodeport_lb4    -45 (-15.85%)
      bpf_xdp.o                tail_lb_ipv4            -541 (-19.57%)
      pyperf100.bpf.o          on_event                -680 (-10.42%)
      pyperf180.bpf.o          on_event               -2164 (-19.62%)
      pyperf600.bpf.o          on_event               -9799 (-24.84%)
      strobemeta.bpf.o         on_event               -9157 (-65.28%)
      xdp_synproxy_kern.bpf.o  syncookie_tc             -54 (-19.29%)
      xdp_synproxy_kern.bpf.o  syncookie_xdp            -74 (-24.50%)
      Signed-off-by: default avatarEduard Zingerman <eddyz87@gmail.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Link: https://lore.kernel.org/bpf/20240127175237.526726-6-maxtram95@gmail.com
      6efbde20
    • Maxim Mikityanskiy's avatar
      selftests/bpf: Add test cases for narrowing fill · 067313a8
      Maxim Mikityanskiy authored
      The previous commit allowed to preserve boundaries and track IDs of
      scalars on narrowing fills. Add test cases for that pattern.
      Signed-off-by: default avatarMaxim Mikityanskiy <maxim@isovalent.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Acked-by: default avatarEduard Zingerman <eddyz87@gmail.com>
      Link: https://lore.kernel.org/bpf/20240127175237.526726-5-maxtram95@gmail.com
      067313a8
    • Maxim Mikityanskiy's avatar
      bpf: Preserve boundaries and track scalars on narrowing fill · c1e6148c
      Maxim Mikityanskiy authored
      When the width of a fill is smaller than the width of the preceding
      spill, the information about scalar boundaries can still be preserved,
      as long as it's coerced to the right width (done by coerce_reg_to_size).
      Even further, if the actual value fits into the fill width, the ID can
      be preserved as well for further tracking of equal scalars.
      
      Implement the above improvements, which makes narrowing fills behave the
      same as narrowing spills and MOVs between registers.
      
      Two tests are adjusted to accommodate for endianness differences and to
      take into account that it's now allowed to do a narrowing fill from the
      least significant bits.
      
      reg_bounds_sync is added to coerce_reg_to_size to correctly adjust
      umin/umax boundaries after the var_off truncation, for example, a 64-bit
      value 0xXXXXXXXX00000000, when read as a 32-bit, gets umin = 0, umax =
      0xFFFFFFFF, var_off = (0x0; 0xffffffff00000000), which needs to be
      synced down to umax = 0, otherwise reg_bounds_sanity_check doesn't pass.
      Signed-off-by: default avatarMaxim Mikityanskiy <maxim@isovalent.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Link: https://lore.kernel.org/bpf/20240127175237.526726-4-maxtram95@gmail.com
      c1e6148c
    • Maxim Mikityanskiy's avatar
      selftests/bpf: Test tracking spilled unbounded scalars · 6be503ce
      Maxim Mikityanskiy authored
      The previous commit added tracking for unbounded scalars on spill. Add
      the test case to check the new functionality.
      Signed-off-by: default avatarMaxim Mikityanskiy <maxim@isovalent.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Acked-by: default avatarEduard Zingerman <eddyz87@gmail.com>
      Link: https://lore.kernel.org/bpf/20240127175237.526726-3-maxtram95@gmail.com
      6be503ce
    • Maxim Mikityanskiy's avatar
      bpf: Track spilled unbounded scalars · e67ddd9b
      Maxim Mikityanskiy authored
      Support the pattern where an unbounded scalar is spilled to the stack,
      then boundary checks are performed on the src register, after which the
      stack frame slot is refilled into a register.
      
      Before this commit, the verifier didn't treat the src register and the
      stack slot as related if the src register was an unbounded scalar. The
      register state wasn't copied, the id wasn't preserved, and the stack
      slot was marked as STACK_MISC. Subsequent boundary checks on the src
      register wouldn't result in updating the boundaries of the spilled
      variable on the stack.
      
      After this commit, the verifier will preserve the bond between src and
      dst even if src is unbounded, which permits to do boundary checks on src
      and refill dst later, still remembering its boundaries. Such a pattern
      is sometimes generated by clang when compiling complex long functions.
      
      One test is adjusted to reflect that now unbounded scalars are tracked.
      Signed-off-by: default avatarMaxim Mikityanskiy <maxim@isovalent.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Acked-by: default avatarEduard Zingerman <eddyz87@gmail.com>
      Link: https://lore.kernel.org/bpf/20240127175237.526726-2-maxtram95@gmail.com
      e67ddd9b
  7. 01 Feb, 2024 3 commits