| Message ID | 20230602022647.1571784-1-davemarchevsky@fb.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | bpf_refcount followups (part 1) | expand |
Hello: This series was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <ast@kernel.org>: On Thu, 1 Jun 2023 19:26:38 -0700 you wrote: > 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. > > [...] Here is the summary with links: - [v2,bpf-next,1/9,DONOTAPPLY] Revert "bpf: Disable bpf_refcount_acquire kfunc calls until race conditions are fixed" (no matching commit) - [v2,bpf-next,2/9] bpf: Set kptr_struct_meta for node param to list and rbtree insert funcs https://git.kernel.org/bpf/bpf-next/c/2140a6e3422d - [v2,bpf-next,3/9] bpf: Fix __bpf_{list,rbtree}_add's beginning-of-node calculation https://git.kernel.org/bpf/bpf-next/c/cc0d76cafebb - [v2,bpf-next,4/9] bpf: Make bpf_refcount_acquire fallible for non-owning refs https://git.kernel.org/bpf/bpf-next/c/7793fc3babe9 - [v2,bpf-next,5/9,DONOTAPPLY] bpf: Allow KF_DESTRUCTIVE-flagged kfuncs to be called under spinlock (no matching commit) - [v2,bpf-next,6/9,DONOTAPPLY] selftests/bpf: Add unsafe lock/unlock and refcount_read kfuncs to bpf_testmod (no matching commit) - [v2,bpf-next,7/9,DONOTAPPLY] selftests/bpf: Add test exercising bpf_refcount_acquire race condition (no matching commit) - [v2,bpf-next,8/9,DONOTAPPLY] selftests/bpf: Disable newly-added refcounted_kptr_races test (no matching commit) - [v2,bpf-next,9/9,DONOTAPPLY] Revert "selftests/bpf: Disable newly-added refcounted_kptr_races test" (no matching commit) You are awesome, thank you!
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 fixes the bpf_refcount_acquire issue by making it fallible for non-owning references * Patch 5 allows KF_DESTRUCTIVE kfuncs to be called when spinlock is held * This patch, and all patches further in the series, should not be applied * Patch 6 introduces some destructive bpf_testmod kfuncs which the selftest added later in the series needs * 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 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/ Changelog: v1 -> v2: lore.kernel.org/bpf/20230504053338.1778690-1-davemarchevsky@fb.com Patch #s used below refer to the patch's position in v1 unless otherwise specified. * General * Rebase onto latest bpf-next * All bpf_testmod-related changes (destructive kfuncs, etc) have been marked [DONOTAPPLY] in response to Alexei's comments in v1 and followup conversations offline. They're still included as part of the series so that CI can exercise the fixed functionality. * v1's Patch 5 - "selftests/bpf: Add unsafe lock/unlock and refcount_read kfuncs to bpf_testmod" - was moved after the patches which are meant to be applied to make it more obvious that it shouldn't be applied. * 4d585f48ee6b ("bpf: Remove anonymous union in bpf_kfunc_call_arg_meta") was shipped separately from this series in response to Alexei's comments about the anonymous union in kfunc_call_arg_meta. That patch removes the anonymous union and its members (arg_obj_drop, etc) in favor of the simpler approach suggested by Alexei in v1. This series' kfunc_call_arg_meta changes are modified to follow the new pattern. 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: Make bpf_refcount_acquire fallible for non-owning refs [DONOTAPPLY] bpf: Allow KF_DESTRUCTIVE-flagged kfuncs to be called under spinlock [DONOTAPPLY] selftests/bpf: Add unsafe lock/unlock and refcount_read kfuncs to bpf_testmod [DONOTAPPLY] selftests/bpf: Add test exercising bpf_refcount_acquire race condition [DONOTAPPLY] 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 | 55 ++++-- .../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, 379 insertions(+), 19 deletions(-)