[v1,bpf-next,0/9] bpf_refcount followups (part 1)

Message

Dave Marchevsky May 4, 2023, 5:33 a.m. UTC

This series is the first of two (or more) followups to address issues in the
bpf_refcount shared ownership implementation discovered by Kumar.
Specifically, this series addresses the "bpf_refcount_acquire on non-owning ref
in another tree" scenario described in [0], and does _not_ address issues
raised in [1]. Further followups will address the other issues.

The series can be applied without re-enabling bpf_refcount_acquire calls, which
were disabled in commit 7deca5eae833 ("bpf: Disable bpf_refcount_acquire kfunc
calls until race conditions are fixed") until all issues are addressed. Some
extra patches are included so that BPF CI tests will exercise test changes in
the series.

Patch contents:
  * Patch 1 reverts earlier disabling of bpf_refcount_acquire calls
    * Selftest added later in the series need to call bpf_refcount_acquire
    * This patch should not be applied and is included to allow CI to run the
      newly-added test and exercise test changes in patch 6
  * Patches 2 and 3 fix other bugs introduced in bpf_refcount series which were
    discovered while reproducing the main issue this series addresses
  * Patch 4 allows KF_DESTRUCTIVE kfuncs to be called when spinlock is held
  * Patch 5 introduces some destructive bpf_testmod kfuncs which the selftest
    added later in the series needs
    * Marked [RFC] as there's some copying of internal implementation that
      probably isn't correct. Suggestions needed for how to proceed.
  * Patch 6 fixes the bpf_refcount_acquire issue by making it fallible for
    non-owning references
  * Patch 7 adds a selftest which uses the kfuncs introduced in patch 5 to
    replicate the exact scenario raised by Kumar
  * Patch 8 disables the test added in patch 7
    * This is so the series (aside from DONOTAPPLY patches) can be applied
      without re-enabling bpf_refcount_acquire yet.
  * Patch 9 reverts patch 8 so that CI can run the newly-added test
    * This patch should not be applied

The first and last patches in the series are included to allow the CI to run
newly-added tests and should not be applied. First patch reverts earlier
disabling of bpf_refcount_acquire calls as the test reproducing
"bpf_refcount_acquire on non-owning ref in another tree" scenario obviously
needs to be able to call bpf_refcount_acquire.

While reproducing the scenario Kumar described in [0], which should cause a
refcount use-after-free, two unrelated bugs were found and are fixed by this
series.

  [0]: https://lore.kernel.org/bpf/atfviesiidev4hu53hzravmtlau3wdodm2vqs7rd7tnwft34e3@xktodqeqevir/
  [1]: https://lore.kernel.org/bpf/d7hyspcow5wtjcmw4fugdgyp3fwhljwuscp3xyut5qnwivyeru@ysdq543otzv2/

Dave Marchevsky (9):
  [DONOTAPPLY] Revert "bpf: Disable bpf_refcount_acquire kfunc calls
    until race conditions are fixed"
  bpf: Set kptr_struct_meta for node param to list and rbtree insert
    funcs
  bpf: Fix __bpf_{list,rbtree}_add's beginning-of-node calculation
  bpf: Allow KF_DESTRUCTIVE-flagged kfuncs to be called under spinlock
  [RFC] selftests/bpf: Add unsafe lock/unlock and refcount_read kfuncs
    to bpf_testmod
  bpf: Make bpf_refcount_acquire fallible for non-owning refs
  selftests/bpf: Add test exercising bpf_refcount_acquire race condition
  selftests/bpf: Disable newly-added refcounted_kptr_races test
  [DONOTAPPLY] Revert "selftests/bpf: Disable newly-added
    refcounted_kptr_races test"

 kernel/bpf/helpers.c                          |  12 +-
 kernel/bpf/verifier.c                         |  70 +++++---
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  61 +++++++
 .../bpf/prog_tests/refcounted_kptr.c          | 106 +++++++++++-
 .../selftests/bpf/progs/refcounted_kptr.c     | 160 ++++++++++++++++++
 .../bpf/progs/refcounted_kptr_fail.c          |   4 +-
 6 files changed, 388 insertions(+), 25 deletions(-)