[PATCH bpf-next v1 3/6] bpf: Name the callback in frame-release diagnostics

[Ihor Solodrai]

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 <ihor.solodrai@xxxxxxxxx>
---
 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