This patch eliminates extra bpf_reg_state memory usage added due to previous patch keeping a copy of lock identity in reg state for non-owning refs. Instead of copying lock identity around, this patch changes non_owning_ref_lock field to be a bool, taking advantage of the following: * There can currently only be one active lock at a time * non-owning refs are only valid in the critical section So if a verifier_state has an active_lock, any non-owning ref must've been obtained under that lock, and any non-owning ref not obtained under that lock must have been invalidated previously. Therefore if a non-owning ref is associated with a lock, it's the active_lock of the current state. So we can keep a bool "are we associated with active_lock of current state" instead of copying lock identity around. Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx> --- include/linux/bpf_verifier.h | 2 +- kernel/bpf/verifier.c | 25 ++++++++++--------------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 7b5fbb66446c..d25446dd0413 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -84,7 +84,7 @@ struct bpf_reg_state { struct { struct btf *btf; u32 btf_id; - struct bpf_active_lock non_owning_ref_lock; + bool non_owning_ref_lock; }; struct { /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */ diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index f693cc97c574..89c09752421c 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -190,8 +190,7 @@ struct bpf_verifier_stack_elem { static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx); static int release_reference(struct bpf_verifier_env *env, int ref_obj_id); -static void invalidate_non_owning_refs(struct bpf_verifier_env *env, - struct bpf_active_lock *lock); +static void invalidate_non_owning_refs(struct bpf_verifier_env *env); static int ref_set_non_owning_lock(struct bpf_verifier_env *env, struct bpf_reg_state *reg); @@ -1077,9 +1076,8 @@ static void print_verifier_state(struct bpf_verifier_env *env, verbose_a("id=%d", reg->id); if (reg->ref_obj_id) verbose_a("ref_obj_id=%d", reg->ref_obj_id); - if (reg->non_owning_ref_lock.ptr) - verbose_a("non_own_id=(%p,%d)", reg->non_owning_ref_lock.ptr, - reg->non_owning_ref_lock.id); + if (reg->non_owning_ref_lock) + verbose_a("%s", "non_own_ref"); if (t != SCALAR_VALUE) verbose_a("off=%d", reg->off); if (type_is_pkt_pointer(t)) @@ -5049,7 +5047,7 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, } if (type_is_alloc(reg->type) && !reg->ref_obj_id && - !reg->non_owning_ref_lock.ptr) { + !reg->non_owning_ref_lock) { verbose(env, "verifier internal error: ref_obj_id for allocated object must be non-zero\n"); return -EFAULT; } @@ -6056,7 +6054,7 @@ static int process_spin_lock(struct bpf_verifier_env *env, int regno, return -EINVAL; } - invalidate_non_owning_refs(env, &cur->active_lock); + invalidate_non_owning_refs(env); cur->active_lock.ptr = NULL; cur->active_lock.id = 0; @@ -7373,16 +7371,14 @@ static int release_reference(struct bpf_verifier_env *env, return 0; } -static void invalidate_non_owning_refs(struct bpf_verifier_env *env, - struct bpf_active_lock *lock) +static void invalidate_non_owning_refs(struct bpf_verifier_env *env) { struct bpf_func_state *unused; struct bpf_reg_state *reg; bpf_for_each_reg_in_vstate(env->cur_state, unused, reg, ({ - if (reg->non_owning_ref_lock.ptr && - reg->non_owning_ref_lock.ptr == lock->ptr && - reg->non_owning_ref_lock.id == lock->id) + if (type_is_ptr_alloc_obj(reg->type) && + reg->non_owning_ref_lock) __mark_reg_unknown(env, reg); })); } @@ -8948,13 +8944,12 @@ static int ref_set_non_owning_lock(struct bpf_verifier_env *env, struct bpf_reg_ return -EFAULT; } - if (reg->non_owning_ref_lock.ptr) { + if (reg->non_owning_ref_lock) { verbose(env, "verifier internal error: non_owning_ref_lock already set\n"); return -EFAULT; } - reg->non_owning_ref_lock.id = state->active_lock.id; - reg->non_owning_ref_lock.ptr = state->active_lock.ptr; + reg->non_owning_ref_lock = true; return 0; } -- 2.30.2