The unwinder will release the resources of each frame from that frame's own stack slots. Each reference in the verifier state must therefore belong to exactly one frame. The verifier has no such notion today: the state holds the references, and a frame only knows how many of the references held at its entry are still held. Record the owner frame in the reference state: - The frame that acquires a reference owns it. - When a frame returns, the references it still holds pass to its caller, which is the only frame left that can release them. - A function that is verified on its own inherits the references of its callers without their frames, see call_func_on_its_own(). No frame of the state owns these inherited references, so their owner is -1. References that the function returns with, beyond the inherited ones, pass to the caller when the call is resumed in bpf_flow_return(). Callbacks release everything they acquire, so nothing changes owner when a callback returns, and the transfer in prepare_func_exit() has nothing to do for them. RCU read and preemption disabled sections have no reference state. The verifier tracks, per frame, how many of the sections held at its entry are still held, and release_context_state() lowers that count on every unlock. A frame that left a section of its caller therefore looks like a frame that was entered with fewer sections. Also count how many sections each frame released below its entry count, so that the exit of the frame can tell that it returns with fewer sections than it was called with. Signed-off-by: Kumar Kartikeya Dwivedi --- include/linux/bpf_verifier.h | 13 +++++++++++++ kernel/bpf/verifier.c | 22 ++++++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 7a2b6507f562..c18efe4bd54a 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -327,6 +327,12 @@ struct bpf_reference_state { * is used purely to inform the user of a reference leak. */ int insn_idx; + /* + * The frame that owns the reference: the one that acquired it, or the + * caller a returning frame handed it to. -1 for what the callers of a + * function verified on its own hold, which are not in the state. + */ + s8 frameno; union { /* For REF_TYPE_PTR */ int parent_id; @@ -503,6 +509,13 @@ struct bpf_func_state { u32 entry_refs; u32 entry_rcu_locks; u32 entry_preempt_locks; + /* + * How many of the RCU read and preemption disabled sections held when the + * frame was entered it left, or a callee of it did: a frame that returns + * after that is in fewer sections than it was called in. + */ + u32 rcu_released_below; + u32 preempt_released_below; /* The following fields should be last. See copy_func_state() */ /* The state of the stack. Each element of the array describes BPF_REG_SIZE diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 0684ac1f4f22..6c71ceb1dd4c 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1545,6 +1545,7 @@ static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_e if (err) return NULL; state->refs[new_ofs].insn_idx = insn_idx; + state->refs[new_ofs].frameno = state->curframe; return &state->refs[new_ofs]; } @@ -1625,7 +1626,11 @@ static void release_reference_state(struct bpf_verifier_state *state, int idx) return; } -/* bpf_rcu_read_unlock() or bpf_preempt_enable() is done, see release_reference_state() */ +/* + * bpf_rcu_read_unlock() or bpf_preempt_enable() is done, see release_reference_state(). + * A frame that was entered with more sections than are held now has left one of its + * callers', and counts that for its exit to tell. + */ static void release_context_state(struct bpf_verifier_state *state) { struct bpf_func_state *frame; @@ -1633,6 +1638,11 @@ static void release_context_state(struct bpf_verifier_state *state) for (fr = 0; fr <= state->curframe; fr++) { frame = state->frame[fr]; + if (frame->entry_rcu_locks > state->active_rcu_locks) + frame->rcu_released_below += frame->entry_rcu_locks - state->active_rcu_locks; + if (frame->entry_preempt_locks > state->active_preempt_locks) + frame->preempt_released_below += frame->entry_preempt_locks - + state->active_preempt_locks; frame->entry_rcu_locks = min(frame->entry_rcu_locks, state->active_rcu_locks); frame->entry_preempt_locks = min(frame->entry_preempt_locks, state->active_preempt_locks); @@ -11709,6 +11719,8 @@ static int call_func_on_its_own(struct bpf_verifier_env *env, int subprog, int i entry.refs[i].id = inherited_id(state, entry.refs[i].id); if (entry.refs[i].type == REF_TYPE_PTR) entry.refs[i].parent_id = inherited_id(state, entry.refs[i].parent_id); + /* the frames that own what is inherited are not in the state */ + entry.refs[i].frameno = -1; } entry.active_irq_id = inherited_id(state, entry.active_irq_id); entry.active_lock_id = inherited_id(state, entry.active_lock_id); @@ -11878,11 +11890,12 @@ int bpf_flow_return(struct bpf_verifier_env *env, struct bpf_verifier_state *cal reg->parent_id = fresh_id(env, idmap, reg->parent_id); reg->map_uid = fresh_id(env, idmap, reg->map_uid); } - /* those that were held are the first */ + /* those that were held are the first; what the function took is the caller's now */ for (i = nheld; i < caller->acquired_refs; i++) { caller->refs[i].id = fresh_id(env, idmap, caller->refs[i].id); if (caller->refs[i].type == REF_TYPE_PTR) caller->refs[i].parent_id = fresh_id(env, idmap, caller->refs[i].parent_id); + caller->refs[i].frameno = caller->curframe; } caller->active_irq_id = fresh_id(env, idmap, caller->active_irq_id); caller->active_lock_id = fresh_id(env, idmap, caller->active_lock_id); @@ -12688,6 +12701,11 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) return err; invalidate_stack_slices(env, callee->frameno, INT_MIN, 0); + /* What the frame holds when it returns is the caller's to release. */ + for (i = 0; i < state->acquired_refs; i++) + if (state->refs[i].frameno == callee->frameno) + state->refs[i].frameno = caller->frameno; + /* for callbacks like bpf_loop or bpf_for_each_map_elem go back to callsite, * there function call logic would reschedule callback visit. */ -- 2.53.0