1. Dec 12, 2023
  2. Dec 11, 2023
  3. Dec 10, 2023
  4. Dec 09, 2023
    • Sergei Trofimovich's avatar
      libbpf: Add pr_warn() for EINVAL cases in linker_sanity_check_elf · 32fa0583
      Sergei Trofimovich authored
      
      
      Before the change on `i686-linux` `systemd` build failed as:
      
          $ bpftool gen object src/core/bpf/socket_bind/socket-bind.bpf.o src/core/bpf/socket_bind/socket-bind.bpf.unstripped.o
          Error: failed to link 'src/core/bpf/socket_bind/socket-bind.bpf.unstripped.o': Invalid argument (22)
      
      After the change it fails as:
      
          $ bpftool gen object src/core/bpf/socket_bind/socket-bind.bpf.o src/core/bpf/socket_bind/socket-bind.bpf.unstripped.o
          libbpf: ELF section #9 has inconsistent alignment addr=8 != d=4 in src/core/bpf/socket_bind/socket-bind.bpf.unstripped.o
          Error: failed to link 'src/core/bpf/socket_bind/socket-bind.bpf.unstripped.o': Invalid argument (22)
      
      Now it's slightly easier to figure out what is wrong with an ELF file.
      
      Signed-off-by: default avatarSergei Trofimovich <slyich@gmail.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Acked-by: default avatarEduard Zingerman <eddyz87@gmail.com>
      Link: https://lore.kernel.org/bpf/20231208215100.435876-1-slyich@gmail.com
      32fa0583
    • Martin KaFai Lau's avatar
      Merge branch 'bpf: Expand bpf_cgrp_storage to support cgroup1 non-attach case' · 09115c33
      Martin KaFai Lau authored
      
      
      Yafang Shao says:
      
      ====================
      In the current cgroup1 environment, associating operations between a cgroup
      and applications in a BPF program requires storing a mapping of cgroup_id
      to application either in a hash map or maintaining it in userspace.
      However, by enabling bpf_cgrp_storage for cgroup1, it becomes possible to
      conveniently store application-specific information in cgroup-local storage
      and utilize it within BPF programs. Furthermore, enabling this feature for
      cgroup1 involves minor modifications for the non-attach case, streamlining
      the process.
      
      However, when it comes to enabling this functionality for the cgroup1
      attach case, it presents challenges. Therefore, the decision is to focus on
      enabling it solely for the cgroup1 non-attach case at present. If
      attempting to attach to a cgroup1 fd, the operation will simply fail with
      the error code -EBADF.
      
      Changes:
      - RFC -> v1:
        - Collect acked-by
        - Avoid unnecessary is_cgroup1 check (Yonghong)
        - Keep the code patterns consistent (Yonghong)
      ====================
      
      Signed-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>
      09115c33
    • Yafang Shao's avatar
      selftests/bpf: Add selftests for cgroup1 local storage · a2c6380b
      Yafang Shao authored
      
      
      Expanding the test coverage from cgroup2 to include cgroup1. The result
      as follows,
      
      Already existing test cases for cgroup2:
        #48/1    cgrp_local_storage/tp_btf:OK
        #48/2    cgrp_local_storage/attach_cgroup:OK
        #48/3    cgrp_local_storage/recursion:OK
        #48/4    cgrp_local_storage/negative:OK
        #48/5    cgrp_local_storage/cgroup_iter_sleepable:OK
        #48/6    cgrp_local_storage/yes_rcu_lock:OK
        #48/7    cgrp_local_storage/no_rcu_lock:OK
      
      Expanded test cases for cgroup1:
        #48/8    cgrp_local_storage/cgrp1_tp_btf:OK
        #48/9    cgrp_local_storage/cgrp1_recursion:OK
        #48/10   cgrp_local_storage/cgrp1_negative:OK
        #48/11   cgrp_local_storage/cgrp1_iter_sleepable:OK
        #48/12   cgrp_local_storage/cgrp1_yes_rcu_lock:OK
        #48/13   cgrp_local_storage/cgrp1_no_rcu_lock:OK
      
      Summary:
        #48      cgrp_local_storage:OK
        Summary: 1/13 PASSED, 0 SKIPPED, 0 FAILED
      
      Signed-off-by: default avatarYafang Shao <laoar.shao@gmail.com>
      Acked-by: default avatarTejun Heo <tj@kernel.org>
      Acked-by: default avatarYonghong Song <yonghong.song@linux.dev>
      Link: https://lore.kernel.org/r/20231206115326.4295-4-laoar.shao@gmail.com
      
      
      Signed-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>
      a2c6380b
    • Yafang Shao's avatar
      selftests/bpf: Add a new cgroup helper open_classid() · f4199271
      Yafang Shao authored
      
      
      This new helper allows us to obtain the fd of a net_cls cgroup, which will
      be utilized in the subsequent patch.
      
      Signed-off-by: default avatarYafang Shao <laoar.shao@gmail.com>
      Acked-by: default avatarTejun Heo <tj@kernel.org>
      Acked-by: default avatarYonghong Song <yonghong.song@linux.dev>
      Link: https://lore.kernel.org/r/20231206115326.4295-3-laoar.shao@gmail.com
      
      
      Signed-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>
      f4199271
    • Yafang Shao's avatar
      bpf: Enable bpf_cgrp_storage for cgroup1 non-attach case · 73d9eb34
      Yafang Shao authored
      
      
      In the current cgroup1 environment, associating operations between cgroups
      and applications in a BPF program requires storing a mapping of cgroup_id
      to application either in a hash map or maintaining it in userspace.
      However, by enabling bpf_cgrp_storage for cgroup1, it becomes possible to
      conveniently store application-specific information in cgroup-local storage
      and utilize it within BPF programs. Furthermore, enabling this feature for
      cgroup1 involves minor modifications for the non-attach case, streamlining
      the process.
      
      However, when it comes to enabling this functionality for the cgroup1
      attach case, it presents challenges. Therefore, the decision is to focus on
      enabling it solely for the cgroup1 non-attach case at present. If
      attempting to attach to a cgroup1 fd, the operation will simply fail with
      the error code -EBADF.
      
      Signed-off-by: default avatarYafang Shao <laoar.shao@gmail.com>
      Acked-by: default avatarTejun Heo <tj@kernel.org>
      Acked-by: default avatarYonghong Song <yonghong.song@linux.dev>
      Link: https://lore.kernel.org/r/20231206115326.4295-2-laoar.shao@gmail.com
      
      
      Signed-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>
      73d9eb34
    • Andrii Nakryiko's avatar
      selftests/bpf: fix timer/test_bad_ret subtest on test_progs-cpuv4 flavor · 1720c42b
      Andrii Nakryiko authored
      Because test_bad_ret main program is not written in assembly, we don't
      control instruction indices in timer_cb_ret_bad() subprog. This bites us
      in timer/test_bad_ret subtest, where we see difference between cpuv4 and
      other flavors.
      
      For now, make __msg() expectations not rely on instruction indices by
      anchoring them around bpf_get_prandom_u32 call. Once we have regex/glob
      support for __msg(), this can be expressed a bit more nicely, but for
      now just mitigating the problem with available means.
      
      Fixes: e02dea15
      
       ("selftests/bpf: validate async callback return value check correctness")
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Link: https://lore.kernel.org/r/20231208233028.3412690-1-andrii@kernel.org
      
      
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      1720c42b
    • Andrii Nakryiko's avatar
      Merge branch 'bpf-fix-accesses-to-uninit-stack-slots' · 4af20ab9
      Andrii Nakryiko authored
      Andrei Matei says:
      
      ====================
      bpf: fix accesses to uninit stack slots
      
      Fix two related issues issues around verifying stack accesses:
      1. accesses to uninitialized stack memory was allowed inconsistently
      2. the maximum stack depth needed for a program was not always
      maintained correctly
      
      The two issues are fixed together in one commit because the code for one
      affects the other.
      
      V4 to V5:
      - target bpf-next (Alexei)
      
      V3 to V4:
      - minor fixup to comment in patch 1 (Eduard)
      - C89-style in patch 3 (Andrii)
      
      V2 to V3:
      - address review comments from Andrii and Eduard
      - drop new verifier tests in favor of editing existing tests to check
        for stack depth
      - append a patch with a bit of cleanup coming out of the previous review
      ====================
      
      Link: https://lore.kernel.org/r/20231208032519.260451-1-andreimatei1@gmail.com
      
      
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      4af20ab9
    • Andrei Matei's avatar
      bpf: Minor cleanup around stack bounds · 2929bfac
      Andrei Matei authored
      
      
      Push the rounding up of stack offsets into the function responsible for
      growing the stack, rather than relying on all the callers to do it.
      Uncertainty about whether the callers did it or not tripped up people in
      a previous review.
      
      Signed-off-by: default avatarAndrei Matei <andreimatei1@gmail.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Acked-by: default avatarEduard Zingerman <eddyz87@gmail.com>
      Link: https://lore.kernel.org/bpf/20231208032519.260451-4-andreimatei1@gmail.com
      2929bfac
    • Andrei Matei's avatar
      bpf: Fix accesses to uninit stack slots · 6b4a64ba
      Andrei Matei authored
      Privileged programs are supposed to be able to read uninitialized stack
      memory (ever since 6715df8d) but, before this patch, these accesses
      were permitted inconsistently. In particular, accesses were permitted
      above state->allocated_stack, but not below it. In other words, if the
      stack was already "large enough", the access was permitted, but
      otherwise the access was rejected instead of being allowed to "grow the
      stack". This undesired rejection was happening in two places:
      - in check_stack_slot_within_bounds()
      - in check_stack_range_initialized()
      This patch arranges for these accesses to be permitted. A bunch of tests
      that were relying on the old rejection had to change; all of them were
      changed to add also run unprivileged, in which case the old behavior
      persists. One tests couldn't be updated - global_func16 - because it
      can't run unprivileged for other reasons.
      
      This patch also fixes the tracking of the stack size for variable-offset
      reads. This second fix is bundled in the same commit as the first one
      because they're inter-related. Before this patch, writes to the stack
      using registers containing a variable offset (as opposed to registers
      with fixed, known values) were not properly contributing to the
      function's needed stack size. As a result, it was possible for a program
      to verify, but then to attempt to read out-of-bounds data at runtime
      because a too small stack had been allocated for it.
      
      Each function tracks the size of the stack it needs in
      bpf_subprog_info.stack_depth, which is maintained by
      update_stack_depth(). For regular memory accesses, check_mem_access()
      was calling update_state_depth() but it was passing in only the fixed
      part of the offset register, ignoring the variable offset. This was
      incorrect; the minimum possible value of that register should be used
      instead.
      
      This tracking is now fixed by centralizing the tracking of stack size in
      grow_stack_state(), and by lifting the calls to grow_stack_state() to
      check_stack_access_within_bounds() as suggested by Andrii. The code is
      now simpler and more convincingly tracks the correct maximum stack size.
      check_stack_range_initialized() can now rely on enough stack having been
      allocated for the access; this helps with the fix for the first issue.
      
      A few tests were changed to also check the stack depth computation. The
      one that fails without this patch is verifier_var_off:stack_write_priv_vs_unpriv.
      
      Fixes: 01f810ac
      
       ("bpf: Allow variable-offset stack access")
      Reported-by: default avatarHao Sun <sunhao.th@gmail.com>
      Signed-off-by: default avatarAndrei Matei <andreimatei1@gmail.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Acked-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Link: https://lore.kernel.org/bpf/20231208032519.260451-3-andreimatei1@gmail.com
      
      Closes: https://lore.kernel.org/bpf/CABWLsev9g8UP_c3a=1qbuZUi20tGoUXoU07FPf-5FLvhOKOY+Q@mail.gmail.com/
      6b4a64ba
    • Andrei Matei's avatar
      bpf: Add some comments to stack representation · 92e1567e
      Andrei Matei authored
      
      
      Add comments to the datastructure tracking the stack state, as the
      mapping between each stack slot and where its state is stored is not
      entirely obvious.
      
      Signed-off-by: default avatarAndrei Matei <andreimatei1@gmail.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Acked-by: default avatarEduard Zingerman <eddyz87@gmail.com>
      Link: https://lore.kernel.org/bpf/20231208032519.260451-2-andreimatei1@gmail.com
      92e1567e
    • David Vernet's avatar
      bpf: Load vmlinux btf for any struct_ops map · 8b7b0e5f
      David Vernet authored
      
      
      In libbpf, when determining whether we need to load vmlinux btf, we're
      currently (among other things) checking whether there is any struct_ops
      program present in the object. This works for most realistic struct_ops
      maps, as a struct_ops map is of course typically composed of one or more
      struct_ops programs. However, that technically need not be the case. A
      struct_ops interface could be defined which allows a map to be specified
      which one or more non-prog fields, and which provides default behavior
      if no struct_ops progs is actually provided otherwise. For sched_ext,
      for example, you technically only need to specify the name of the
      scheduler in the struct_ops map, with the core scheduler logic providing
      default behavior if no prog is actually specified.
      
      If we were to define and try to load such a struct_ops map, we would
      crash in libbpf when initializing it as obj->btf_vmlinux will be NULL:
      
      Reading symbols from minimal...
      (gdb) r
      Starting program: minimal_example
      [Thread debugging using libthread_db enabled]
      Using host libthread_db library "/usr/lib/libthread_db.so.1".
      
      Program received signal SIGSEGV, Segmentation fault.
      0x000055555558308c in btf__type_cnt (btf=0x0) at btf.c:612
      612             return btf->start_id + btf->nr_types;
      (gdb) bt
          type_name=0x5555555d99e3 "sched_ext_ops", kind=4) at btf.c:914
          kind=4) at btf.c:942
          type=0x7fffffffe558, type_id=0x7fffffffe548, ...
          data_member=0x7fffffffe568) at libbpf.c:948
          kern_btf=0x0) at libbpf.c:1017
          at libbpf.c:8059
      
      So as to account for such bare-bones struct_ops maps, let's update
      obj_needs_vmlinux_btf() to also iterate over an obj's maps and check
      whether any of them are struct_ops maps.
      
      Signed-off-by: default avatarDavid Vernet <void@manifault.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Reviewed-by: default avatarAlan Maguire <alan.maguire@oracle.com>
      Link: https://lore.kernel.org/bpf/20231208061704.400463-1-void@manifault.com
      8b7b0e5f
  5. Dec 08, 2023
  6. Dec 07, 2023