On 11/28/22 6:37 PM, Yonghong Song wrote:
Commit 9bb00b2895cb ("bpf: Add kfunc bpf_rcu_read_lock/unlock()") introduced MEM_RCU and bpf_rcu_read_lock/unlock() support. In that commit, a rcu pointer is tagged with both MEM_RCU and PTR_TRUSTED so that it can be passed into kfuncs or helpers as an argument. Martin raised a good question in [1] such that the rcu pointer, although being able to accessing the object, might have reference count of 0. This might cause a problem if the rcu pointer is passed to a kfunc which expects trusted arguments where ref count should be greater than 0. So this patch tries to fix this problem by tagging rcu pointer with MEM_RCU only. Special acquire functions are needed to try to acquire a reference with the consideration that the original rcu pointer ref count could be 0. This special acquire function's argument needs to be KF_RCU, a new introduced kfunc flag. In verifier, KF_RCU will require the actual argument register type to be MEM_RCU. [1] https://lore.kernel.org/bpf/ac70f574-4023-664e-b711-e0d3b18117fd@xxxxxxxxx/ Fixes: 9bb00b2895cb ("bpf: Add kfunc bpf_rcu_read_lock/unlock()") Signed-off-by: Yonghong Song <yhs@xxxxxx> --- include/linux/bpf_verifier.h | 2 +- include/linux/btf.h | 1 + kernel/bpf/helpers.c | 14 ++++++++ kernel/bpf/verifier.c | 36 +++++++++++++------ .../selftests/bpf/prog_tests/cgrp_kfunc.c | 4 +-- .../selftests/bpf/prog_tests/task_kfunc.c | 4 +-- .../selftests/bpf/progs/rcu_read_lock.c | 7 ++-- 7 files changed, 50 insertions(+), 18 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index c05aa6e1f6f5..6f192dd9025e 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -683,7 +683,7 @@ static inline bool bpf_prog_check_recur(const struct bpf_prog *prog) } }-#define BPF_REG_TRUSTED_MODIFIERS (MEM_ALLOC | MEM_RCU | PTR_TRUSTED)+#define BPF_REG_TRUSTED_MODIFIERS (MEM_ALLOC | PTR_TRUSTED)static inline bool bpf_type_has_unsafe_modifiers(u32 type){ diff --git a/include/linux/btf.h b/include/linux/btf.h index 9ed00077db6e..cbd6e4096f8c 100644 --- a/include/linux/btf.h +++ b/include/linux/btf.h @@ -70,6 +70,7 @@ #define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer arguments */ #define KF_SLEEPABLE (1 << 5) /* kfunc may sleep */ #define KF_DESTRUCTIVE (1 << 6) /* kfunc performs destructive actions */ +#define KF_RCU (1 << 7) /* kfunc only takes rcu pointer arguments *//** Return the name of the passed struct, if exists, or halt the build if for diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index a5a511430f2a..46fbe027f3b6 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1837,6 +1837,19 @@ struct task_struct *bpf_task_acquire(struct task_struct *p) return p; }+/**+ * bpf_task_acquire_rcu - Acquire a reference to a rcu task object. A task + * acquired by this kfunc which is not stored in a map as a kptr, must be + * released by calling bpf_task_release(). + * @p: The task on which a reference is being acquired or NULL.
A typo. The argument @p cannot be NULL. Will wait for more feedbacks before sending next version with the fix.
+ */
+struct task_struct *bpf_task_acquire_rcu(struct task_struct *p)
+{
+ if (!refcount_inc_not_zero(&p->rcu_users))
+ return NULL;
+ return p;
+}
+
/**
[...]
