A frame-owned reference is dropped through the same descendant walk as a program-owned one, so a program that uses a callback argument after the callback returns is told "resource release invalidated this value". No resource was released, and nothing the program did caused it. Add __release_reference() with the reason parameter. The message names the callback rather than the frame because a callback is the only thing that declares a frame-scoped argument today, and it is what the program author recognises. Signed-off-by: Ihor Solodrai --- kernel/bpf/diagnostics.c | 3 +++ kernel/bpf/diagnostics.h | 1 + kernel/bpf/verifier.c | 18 +++++++++++------- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c index 5ecfa86ed49f..0610c3f6b334 100644 --- a/kernel/bpf/diagnostics.c +++ b/kernel/bpf/diagnostics.c @@ -2233,6 +2233,9 @@ static void diag_print_mod(struct bpf_verifier_env *env, const struct bpf_diag_h "resource release invalidated " "this value"; break; + case BPF_DIAG_MOD_FRAME_RELEASE: + reason = "the callback that owned this value returned"; + break; case BPF_DIAG_MOD_PKT_DATA_CHANGE: reason = "packet data may have moved"; break; diff --git a/kernel/bpf/diagnostics.h b/kernel/bpf/diagnostics.h index a4102fb049ec..b5cab4d79c1a 100644 --- a/kernel/bpf/diagnostics.h +++ b/kernel/bpf/diagnostics.h @@ -22,6 +22,7 @@ enum bpf_diag_mod_reason { BPF_DIAG_MOD_SPILL, BPF_DIAG_MOD_VAR_WRITE, BPF_DIAG_MOD_REF_RELEASE, + BPF_DIAG_MOD_FRAME_RELEASE, BPF_DIAG_MOD_PKT_DATA_CHANGE, BPF_DIAG_MOD_NON_OWN_REF, BPF_DIAG_MOD_CALLER_SAVED, diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 9775a6d38d3b..ba0c8c27b45f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -10169,7 +10169,8 @@ static int idstack_pop(struct bpf_idmap *idmap) } /* Release id and objects derived from it iteratively in a DFS manner */ -static int release_reference(struct bpf_verifier_env *env, int id) +static int __release_reference(struct bpf_verifier_env *env, int id, + enum bpf_diag_mod_reason reason) { u32 mask = (1 << STACK_SPILL) | (1 << STACK_DYNPTR); struct bpf_verifier_state *vstate = env->cur_state; @@ -10224,14 +10225,12 @@ static int release_reference(struct bpf_verifier_env *env, int id) if (reg->dynptr.first_slot) dyn_stack--; - bpf_diag_record_scrub(env, &dyn_stack[0].spilled_ptr, - BPF_DIAG_MOD_REF_RELEASE); - bpf_diag_record_scrub(env, &dyn_stack[1].spilled_ptr, - BPF_DIAG_MOD_REF_RELEASE); + bpf_diag_record_scrub(env, &dyn_stack[0].spilled_ptr, reason); + bpf_diag_record_scrub(env, &dyn_stack[1].spilled_ptr, reason); invalidate_dynptr(env, dyn_stack); continue; } - bpf_diag_record_scrub(env, reg, BPF_DIAG_MOD_REF_RELEASE); + bpf_diag_record_scrub(env, reg, reason); if (!stack || stack->slot_type[BPF_REG_SIZE - 1] == STACK_SPILL) mark_reg_invalid(env, reg); })); @@ -10240,6 +10239,11 @@ static int release_reference(struct bpf_verifier_env *env, int id) return 0; } +static int release_reference(struct bpf_verifier_env *env, int id) +{ + return __release_reference(env, id, BPF_DIAG_MOD_REF_RELEASE); +} + /* Find the first reference owned by frame @frameno, or 0 if it owns none. */ static u32 frame_reference_id(struct bpf_verifier_state *state, u32 frameno) { @@ -10265,7 +10269,7 @@ static int release_frame_reference(struct bpf_verifier_env *env, int id) break; } - return release_reference(env, id); + return __release_reference(env, id, BPF_DIAG_MOD_FRAME_RELEASE); } static void invalidate_non_owning_refs(struct bpf_verifier_env *env) -- 2.55.0