Skip to content

Commit

Permalink
bpf: Consider non-owning refs to refcounted nodes RCU protected
Browse files Browse the repository at this point in the history
An earlier patch in the series ensures that the underlying memory of
nodes with bpf_refcount - which can have multiple owners - is not reused
until RCU grace period has elapsed. This prevents
use-after-free with non-owning references that may point to
recently-freed memory. While RCU read lock is held, it's safe to
dereference such a non-owning ref, as by definition RCU GP couldn't have
elapsed and therefore underlying memory couldn't have been reused.

From the perspective of verifier "trustedness" non-owning refs to
refcounted nodes are now trusted only in RCU CS and therefore should no
longer pass is_trusted_reg, but rather is_rcu_reg. Let's mark them
MEM_RCU in order to reflect this new state.

Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
Link: https://lore.kernel.org/r/20230821193311.3290257-6-davemarchevsky@fb.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Dave Marchevsky authored and Alexei Starovoitov committed Aug 25, 2023
1 parent ba2464c commit 0816b8c
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 2 deletions.
3 changes: 2 additions & 1 deletion include/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,8 @@ enum bpf_type_flag {
MEM_RCU = BIT(13 + BPF_BASE_TYPE_BITS),

/* Used to tag PTR_TO_BTF_ID | MEM_ALLOC references which are non-owning.
* Currently only valid for linked-list and rbtree nodes.
* Currently only valid for linked-list and rbtree nodes. If the nodes
* have a bpf_refcount_field, they must be tagged MEM_RCU as well.
*/
NON_OWN_REF = BIT(14 + BPF_BASE_TYPE_BITS),

Expand Down
13 changes: 12 additions & 1 deletion kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -8007,6 +8007,7 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env,
case PTR_TO_BTF_ID | PTR_TRUSTED:
case PTR_TO_BTF_ID | MEM_RCU:
case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF:
case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF | MEM_RCU:
/* When referenced PTR_TO_BTF_ID is passed to release function,
* its fixed offset must be 0. In the other cases, fixed offset
* can be non-zero. This was already checked above. So pass
Expand Down Expand Up @@ -10473,6 +10474,7 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
static int ref_set_non_owning(struct bpf_verifier_env *env, struct bpf_reg_state *reg)
{
struct bpf_verifier_state *state = env->cur_state;
struct btf_record *rec = reg_btf_record(reg);

if (!state->active_lock.ptr) {
verbose(env, "verifier internal error: ref_set_non_owning w/o active lock\n");
Expand All @@ -10485,6 +10487,9 @@ static int ref_set_non_owning(struct bpf_verifier_env *env, struct bpf_reg_state
}

reg->type |= NON_OWN_REF;
if (rec->refcount_off >= 0)
reg->type |= MEM_RCU;

return 0;
}

Expand Down Expand Up @@ -11322,6 +11327,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
struct bpf_func_state *state;
struct bpf_reg_state *reg;

if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
return -EACCES;
}

if (rcu_lock) {
verbose(env, "nested rcu read lock (kernel function %s)\n", func_name);
return -EINVAL;
Expand Down Expand Up @@ -16684,7 +16694,8 @@ static int do_check(struct bpf_verifier_env *env)
return -EINVAL;
}

if (env->cur_state->active_rcu_lock) {
if (env->cur_state->active_rcu_lock &&
!in_rbtree_lock_required_cb(env)) {
verbose(env, "bpf_rcu_read_unlock is missing\n");
return -EINVAL;
}
Expand Down

0 comments on commit 0816b8c

Please sign in to comment.