[PATCH bpf-next v2 0/7] Follow ups for verifier errors set

[PATCH bpf-next v2 0/7] Follow ups for verifier errors set

[Kumar Kartikeya Dwivedi]

Some follow up changes based on comments from Eduard, Sashiko, and BPF
CI Bot. See commits for details.

Changelog:
----------
v1 -> v2
v1: https://lore.kernel.org/bpf/20260816015746.2632990-1-memxor@gmail.com

 * Drop v1 patches 3, 5-7, 11, and 13-14 because they are already present
   in bpf-next.
 * Use instruction-neutral wording for variable-offset stack accesses
   rather than inferring an atomic operation from value_regno == -1. (Eduard)
 * Update the variable-offset assertions and cover the retained
   uninitialized stack-read diagnostic through the existing CAP_PERFMON-less
   read inside the allocated stack. (BPF CI)
 * Keep v1 patch 4 unchanged after confirming that the write filter is
   reachable for lineage-preserving ALU operations with unchanged
   diagnostic snapshots; only its spill arm is impossible. Say so in the
   commit message. (Eduard)
 * Consider active critical sections only for sleepable programs and let
   non-sleepable programs reach the existing no-active-context fallback
   instead of adding an early branch. A non-sleepable program can still
   hold RCU, preempt, IRQ, or lock state, so the fallback alone would
   keep blaming the region. Keep the "non-sleepable prog" description.
   (Eduard, BPF CI)
 * Select the helper and global-function suggestions by cause, matching
   the kfunc site. (BPF CI)
 * Split the kfunc coverage into a non-sleepable program case and a
   sleepable program inside an RCU read-side critical section. (Eduard)
 * Scan subprogram properties with nested loops and drop the partial
   recursive-edge details together with their assertion. (Eduard)
 * Reject CO-RE relocations targeting a truncated final LD_IMM64 before
   applying them, since BTF validation now runs before the subprogram
   layout check, and add raw CO-RE coverage for the rejection. Add the
   Fixes tag. (BPF CI)
 * Rebase on the current bpf-next/master and let the new property scan
   also set the callx marker that check_subprogs() collects upstream.

Kumar Kartikeya Dwivedi (7):
  bpf: Correct verifier diagnostic attribution for stack reads
  selftests/bpf: Test verifier stack-read diagnostic attribution
  bpf: Drop dead spill diagnostic condition
  bpf: Report non-sleepable kfunc programs accurately
  selftests/bpf: Test non-sleepable kfunc context
  bpf: Correct Program Structure diagnostic context
  selftests/bpf: Test Program Structure diagnostic context

 kernel/bpf/cfg.c                              |   6 +-
 kernel/bpf/check_btf.c                        |  15 +-
 kernel/bpf/diagnostics.c                      |  24 +--
 kernel/bpf/verifier.c                         | 179 ++++++++++++------
 .../selftests/bpf/prog_tests/core_reloc_raw.c | 172 +++++++++++------
 .../testing/selftests/bpf/progs/dynptr_fail.c |   3 +
 tools/testing/selftests/bpf/progs/irq.c       |  13 ++
 .../selftests/bpf/progs/iters_state_safety.c  |   3 +
 .../selftests/bpf/progs/preempt_lock.c        |  26 +++
 .../selftests/bpf/progs/verifier_cfg.c        |  40 ++++
 .../selftests/bpf/progs/verifier_gotox.c      |   2 +
 .../selftests/bpf/progs/verifier_spill_fill.c |   2 +
 .../selftests/bpf/progs/verifier_xadd.c       |  27 +++
 13 files changed, 376 insertions(+), 136 deletions(-)


base-commit: 24629aac43d2884109ae47a993fd51b504e2b09b
-- 
2.53.0

[PATCH bpf-next v2 1/7] bpf: Correct verifier diagnostic attribution for stack reads

[Kumar Kartikeya Dwivedi]

Verifier memory diagnostics currently label every fixed-offset stack
read rejected by check_stack_read_fixed_off() as uninitialized. Dynptr,
iterator, and IRQ-flag slots instead contain initialized
verifier-managed state, so the report incorrectly suggests
initialization or CAP_PERFMON.

The variable-offset read without a destination register is currently
reached by atomic read-modify-write instructions, but that follows from
the call sites rather than the check_stack_read() interface. Its
diagnostic also attributes the access to a helper.

Classify rejected stack reads by slot type. Retain the existing
uninitialized report for STACK_INVALID, and describe verifier-managed
slots as opaque state. Use instruction-neutral wording for the
variable-offset read while leaving the existing verifier messages
unchanged.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/bpf/20260815065956.49D2B1F000E9@smtp.kernel.org/
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 63 ++++++++++++++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 18 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fec5a1ae6a4d..dbb3153fe1b5 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3975,19 +3975,46 @@ static int mark_reg_stack_read(struct bpf_verifier_env *env,
 	return 0;
 }
 
-static void bpf_diag_stack_read_uninit(struct bpf_verifier_env *env, int off, int i,
-				       int size)
+static void bpf_diag_stack_read_invalid(struct bpf_verifier_env *env, int off, int i, int size,
+					enum bpf_stack_slot_type type)
 {
-	const char *reason;
+	const char *problem, *reason, *suggestion, *kind;
+
+	if (type == STACK_INVALID) {
+		reason = bpf_diag_fmt(
+			env, "This rejected read uses %d bytes at stack offset %d, but byte %d in that range is uninitialized on this path. "
+			"Programs loaded with CAP_PERFMON can be allowed to read uninitialized stack bytes, but this program is being rejected without that allowance.",
+			size, off, i);
+		bpf_diag_memory(
+			env, env->insn_idx, "uninitialized stack read", reason,
+			"Initialize every byte in the stack range before reading it, adjust the offset and size so the read covers only initialized bytes, "
+			"or load with CAP_PERFMON if uninitialized stack reads are intended.");
+		return;
+	}
 
-	reason = bpf_diag_fmt(env,
-			      "This rejected read uses %d bytes at stack offset %d, but byte %d in that range is uninitialized on this path. "
-		"Programs loaded with CAP_PERFMON can be allowed to read uninitialized stack bytes, but this program is being rejected without that allowance.",
-		size, off, i);
-	bpf_diag_memory(
-		env, env->insn_idx, "uninitialized stack read", reason,
-		"Initialize every byte in the stack range before reading it, adjust the offset and size so the read covers only initialized bytes, "
-		"or load with CAP_PERFMON if uninitialized stack reads are intended.");
+	switch (type) {
+	case STACK_DYNPTR:
+		kind = "dynptr";
+		suggestion = "Use dynptr helpers or kfuncs to access the object represented by the dynptr instead of reading the dynptr state directly.";
+		break;
+	case STACK_ITER:
+		kind = "iterator";
+		suggestion = "Use iterator kfuncs to advance or destroy the iterator instead of reading its state directly.";
+		break;
+	case STACK_IRQ_FLAG:
+		kind = "IRQ flag";
+		suggestion = "Pass the saved IRQ flag to the matching restore kfunc instead of reading its state directly.";
+		break;
+	default:
+		return;
+	}
+
+	problem = bpf_diag_fmt(env, "direct read of %s stack state", kind);
+	reason = bpf_diag_fmt(
+		env, "This rejected read uses %d bytes at stack offset %d, but byte %d in that range belongs to verifier-managed %s state. "
+		"This state has an opaque representation that BPF programs cannot read directly.",
+		size, off, i, kind);
+	bpf_diag_memory(env, env->insn_idx, problem, reason, suggestion);
 }
 
 /* Read the stack at 'off' and put the results into the register indicated by
@@ -4079,7 +4106,7 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
 					} else {
 						verbose(env, "invalid read from stack off %d+%d size %d\n",
 							off, i, size);
-						bpf_diag_stack_read_uninit(env, off, i, size);
+						bpf_diag_stack_read_invalid(env, off, i, size, type);
 					}
 					return -EACCES;
 				}
@@ -4138,7 +4165,7 @@ static int check_stack_read_fixed_off(struct bpf_verifier_env *env,
 			} else {
 				verbose(env, "invalid read from stack off %d+%d size %d\n",
 					off, i, size);
-				bpf_diag_stack_read_uninit(env, off, i, size);
+				bpf_diag_stack_read_invalid(env, off, i, size, type);
 			}
 			return -EACCES;
 		}
@@ -4237,13 +4264,13 @@ static int check_stack_read(struct bpf_verifier_env *env,
 		tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
 		verbose(env, "variable offset stack pointer cannot be passed into helper function; var_off=%s off=%d size=%d\n",
 			tn_buf, off, size);
-		reason = bpf_diag_fmt(env,
-				      "The helper would access the stack through variable offset %s plus fixed offset %d and size %d. "
-			"Helper stack memory arguments require a constant stack offset and a precise initialized range.",
+		reason = bpf_diag_fmt(
+			env, "The instruction would access the stack through variable offset %s plus fixed offset %d and size %d. "
+			"This stack access requires a constant stack offset and a precise initialized range.",
 			tn_buf, off, size);
 		bpf_diag_memory(
-			env, env->insn_idx, "variable stack access", reason,
-			"Use a fixed stack offset for helper memory arguments, or copy the needed bytes into a fixed stack slot first.");
+			env, env->insn_idx, "variable-offset stack access", reason,
+			"Use a fixed stack offset for the instruction, selecting the target stack slot on separate control-flow paths if necessary.");
 		return -EACCES;
 	}
 	/* Variable offset is prohibited for unprivileged mode for simplicity
-- 
2.53.0

[PATCH bpf-next v2 2/7] selftests/bpf: Test verifier stack-read diagnostic attribution

[Kumar Kartikeya Dwivedi]

Verifier diagnostics distinguish uninitialized stack bytes from opaque
dynptr, iterator, and IRQ-flag state, and identify variable-offset
stack accesses without changing the existing verbose messages.

Add output assertions to the existing uninitialized-stack, dynptr, and
iterator rejection cases. The uninitialized-stack assertion uses the
CAP_PERFMON-less read inside the allocated stack, because a read below
the allocated stack is rejected by the bounds check before the slot
classification runs. Add a direct IRQ-flag read and a variable-offset
atomic stack access to cover the other classifications. Retain an
assertion for the legacy helper-worded verbose message in the atomic
test.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../testing/selftests/bpf/progs/dynptr_fail.c |  3 +++
 tools/testing/selftests/bpf/progs/irq.c       | 13 +++++++++
 .../selftests/bpf/progs/iters_state_safety.c  |  3 +++
 .../selftests/bpf/progs/verifier_spill_fill.c |  2 ++
 .../selftests/bpf/progs/verifier_xadd.c       | 27 +++++++++++++++++++
 5 files changed, 48 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
index 1cd61d72c166..9418dfe4d7b7 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
@@ -560,6 +560,9 @@ int global(void *ctx)
 /* A direct read should fail */
 SEC("?raw_tp")
 __failure __msg("invalid read from stack")
+__msg("Verification failed: Memory Safety: Direct read of dynptr stack state")
+__msg("verifier-managed dynptr state")
+__msg("Use dynptr helpers or kfuncs")
 int invalid_read1(void *ctx)
 {
 	struct bpf_dynptr ptr;
diff --git a/tools/testing/selftests/bpf/progs/irq.c b/tools/testing/selftests/bpf/progs/irq.c
index 53df6d248e26..50727fa9b11d 100644
--- a/tools/testing/selftests/bpf/progs/irq.c
+++ b/tools/testing/selftests/bpf/progs/irq.c
@@ -14,6 +14,19 @@ extern int bpf_copy_from_user_str(void *dst, u32 dst__sz, const void *unsafe_ptr
 struct bpf_res_spin_lock lockA __hidden SEC(".data.A");
 struct bpf_res_spin_lock lockB __hidden SEC(".data.B");
 
+SEC("?tc")
+__failure __msg("invalid read from stack")
+__msg("Verification failed: Memory Safety: Direct read of IRQ flag stack state")
+__msg("verifier-managed IRQ flag state")
+__msg("Pass the saved IRQ flag to the matching restore kfunc")
+int irq_flag_direct_read(struct __sk_buff *ctx)
+{
+	unsigned long flags;
+
+	bpf_local_irq_save(&flags);
+	return flags;
+}
+
 SEC("?tc")
 __failure __msg("R1 type=map_value expected=fp")
 int irq_save_bad_arg(struct __sk_buff *ctx)
diff --git a/tools/testing/selftests/bpf/progs/iters_state_safety.c b/tools/testing/selftests/bpf/progs/iters_state_safety.c
index 646026430e9b..4723ae578e53 100644
--- a/tools/testing/selftests/bpf/progs/iters_state_safety.c
+++ b/tools/testing/selftests/bpf/progs/iters_state_safety.c
@@ -332,6 +332,9 @@ int next_after_destroy_fail(void *ctx)
 
 SEC("?raw_tp")
 __failure __msg("invalid read from stack")
+__msg("Verification failed: Memory Safety: Direct read of iterator stack state")
+__msg("verifier-managed iterator state")
+__msg("Use iterator kfuncs")
 int __naked read_from_iter_slot_fail(void)
 {
 	asm volatile (
diff --git a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
index 39a1766dae3f..e1a698db0364 100644
--- a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
+++ b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
@@ -1277,6 +1277,8 @@ __description("stack_noperfmon: reject read of invalid slots")
 __success
 __caps_unpriv(CAP_BPF)
 __failure_unpriv __msg_unpriv("invalid read from stack off -8+1 size 8")
+__msg_unpriv("Verification failed: Memory Safety: Uninitialized stack read")
+__msg_unpriv("Initialize every byte in the stack range before reading it")
 __naked void stack_noperfmon_reject_invalid_read(void)
 {
 	asm volatile ("					\
diff --git a/tools/testing/selftests/bpf/progs/verifier_xadd.c b/tools/testing/selftests/bpf/progs/verifier_xadd.c
index 05a0a55adb45..7bde3da2f36e 100644
--- a/tools/testing/selftests/bpf/progs/verifier_xadd.c
+++ b/tools/testing/selftests/bpf/progs/verifier_xadd.c
@@ -121,4 +121,31 @@ l0_%=:	r0 = 42;					\
 "	::: __clobber_all);
 }
 
+SEC("tc")
+__description("xadd with variable stack offset")
+__failure
+__msg("variable offset stack pointer cannot be passed into helper function")
+__msg("Verification failed: Memory Safety: Variable-offset stack access")
+__msg("The instruction would access the stack")
+__msg("Use a fixed stack offset for the instruction")
+__naked void xadd_variable_stack_offset(void)
+{
+	asm volatile ("					\
+	r1 = 0;						\
+	*(u64 *)(r10 - 16) = r1;			\
+	*(u64 *)(r10 - 8) = r1;				\
+	call %[bpf_get_prandom_u32];			\
+	r0 &= 8;					\
+	r1 = r10;					\
+	r1 += -16;					\
+	r1 += r0;					\
+	r2 = 1;						\
+	lock *(u64 *)(r1 + 0) += r2;			\
+	r0 = 0;						\
+	exit;						\
+"	:
+	: __imm(bpf_get_prandom_u32)
+	: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.53.0

[PATCH bpf-next v2 3/7] bpf: Drop dead spill diagnostic condition

[Kumar Kartikeya Dwivedi]

diag_record_mod() suppresses lineage-preserving register writes when the
diagnostic snapshot is unchanged. This is reachable for ALU operations that
do not change the represented type or bounds, avoiding a redundant history
event while retaining the register's earlier lineage.

The condition appears to extend this suppression to spill events, but
diag_mod_keeps_lineage() only accepts register writes. The spill producer also
supplies a stack target and an explicit origin, so a spill can never satisfy
the conjunction.

Restrict the condition to writes to reflect the filter's actual contract and
avoid suggesting that it applies to spills.

Link: https://lore.kernel.org/bpf/48e6f021b89562f68850fe21ef8c78719819b04cf9c4e4f50bc791937d37ace8@mail.kernel.org/
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/diagnostics.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c
index 5ecfa86ed49f..496d24064532 100644
--- a/kernel/bpf/diagnostics.c
+++ b/kernel/bpf/diagnostics.c
@@ -1567,8 +1567,7 @@ static void diag_record_mod(struct bpf_verifier_env *env, u32 insn_idx,
 	} else if (diag_mod_insn_origin(env, insn_idx, &target, &event.mod.origin)) {
 		event.mod.origin_valid = true;
 	}
-	if (old_reg && new_reg &&
-	    (reason == BPF_DIAG_MOD_WRITE || reason == BPF_DIAG_MOD_SPILL) &&
+	if (old_reg && new_reg && reason == BPF_DIAG_MOD_WRITE &&
 	    !memcmp(&event.mod.old, &event.mod.new, sizeof(event.mod.old)) &&
 	    !event.mod.origin_valid &&
 	    diag_mod_keeps_lineage(env, &event))
-- 
2.53.0

[PATCH bpf-next v2 4/7] bpf: Report non-sleepable kfunc programs accurately

[Kumar Kartikeya Dwivedi]

A sleepable kfunc call can fail either because the program is not
sleepable or because an otherwise sleepable program has entered a
non-sleepable critical section. check_kfunc_call() checks these
conditions separately. The first check is only gated by in_sleepable(),
so the shared diagnostic can blame an active RCU, preemption-disabled,
IRQ-disabled, or locked region even though leaving that region would not
make the program sleepable. Adding a second diagnostic entry point only
to force the program context would duplicate the API.

Reject the call once based on in_sleepable_context(). Teach the shared
bpf_diag_ctx_forbidden() reporter and non_sleepable_context_description()
to consider active RCU, preempt, IRQ, and lock regions only when the
program is sleepable, and to fall back to the non-sleepable program
otherwise. A non-sleepable program can still hold that context state, so
relying on the existing no-context fallback alone would keep blaming the
critical section.

Select the suggestion at the kfunc, helper, and global-function sites
according to that cause, and choose between the two existing kfunc
verifier messages the same way, so a sleepable program is told to leave
the critical section and a non-sleepable program is told to become
sleepable.

This avoids a diagnostic-only wrapper while retaining context history for
sleepable programs that enter a forbidden region.

Link: https://lore.kernel.org/bpf/2e42a1a2bf45f4d2aba7495bdc9f147558055740e2f3c8b9dae255f6c57fc13c@mail.kernel.org/
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/diagnostics.c | 21 +++++++-------
 kernel/bpf/verifier.c    | 62 ++++++++++++++++++++++++----------------
 2 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c
index 496d24064532..69c168e03732 100644
--- a/kernel/bpf/diagnostics.c
+++ b/kernel/bpf/diagnostics.c
@@ -1119,16 +1119,17 @@ void bpf_diag_ctx_forbidden(struct bpf_verifier_env *env, u32 insn_idx,
 	const char *constraint, *context;
 	u32 depth;
 
-	if (env->cur_state->active_rcu_locks)
-		ctx_kind = BPF_DIAG_CONTEXT_RCU;
-	else if (env->cur_state->active_preempt_locks)
-		ctx_kind = BPF_DIAG_CONTEXT_PREEMPT;
-	else if (env->cur_state->active_irq_id)
-		ctx_kind = BPF_DIAG_CONTEXT_IRQ;
-	else if (env->cur_state->active_locks)
-		ctx_kind = BPF_DIAG_CONTEXT_LOCK;
-	else
-		ctx_kind = BPF_DIAG_CONTEXT_NONE;
+	ctx_kind = BPF_DIAG_CONTEXT_NONE;
+	if (env->cur_state->in_sleepable) {
+		if (env->cur_state->active_rcu_locks)
+			ctx_kind = BPF_DIAG_CONTEXT_RCU;
+		else if (env->cur_state->active_preempt_locks)
+			ctx_kind = BPF_DIAG_CONTEXT_PREEMPT;
+		else if (env->cur_state->active_irq_id)
+			ctx_kind = BPF_DIAG_CONTEXT_IRQ;
+		else if (env->cur_state->active_locks)
+			ctx_kind = BPF_DIAG_CONTEXT_LOCK;
+	}
 
 	depth = diag_context_depth(env, ctx_kind);
 	opts = (struct bpf_diag_history_opts) {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index dbb3153fe1b5..1c52e7bd570d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11086,11 +11086,16 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 		}
 
 		if (env->subprog_info[subprog].might_sleep && !in_sleepable_context(env)) {
+			const char *suggestion;
+
 			verbose(env, "sleepable global function %s() called in %s\n",
 				sub_name, non_sleepable_context_description(env));
+			if (in_sleepable(env))
+				suggestion = "Move the call outside the critical section, or use a non-sleepable function.";
+			else
+				suggestion = "Mark the program sleepable if the program type allows it, or use a non-sleepable function.";
 			operation = bpf_diag_fmt(env, "sleepable global function %s()", sub_name);
-			bpf_diag_ctx_forbidden(env, *insn_idx, operation,
-				"Move the call outside the critical section, or use a non-sleepable function.");
+			bpf_diag_ctx_forbidden(env, *insn_idx, operation, suggestion);
 			return -EINVAL;
 		}
 
@@ -12021,14 +12026,16 @@ static inline bool in_sleepable_context(struct bpf_verifier_env *env)
 
 static const char *non_sleepable_context_description(struct bpf_verifier_env *env)
 {
-	if (env->cur_state->active_rcu_locks)
-		return "rcu_read_lock region";
-	if (env->cur_state->active_preempt_locks)
-		return "non-preemptible region";
-	if (env->cur_state->active_irq_id)
-		return "IRQ-disabled region";
-	if (env->cur_state->active_locks)
-		return "lock region";
+	if (in_sleepable(env)) {
+		if (env->cur_state->active_rcu_locks)
+			return "rcu_read_lock region";
+		if (env->cur_state->active_preempt_locks)
+			return "non-preemptible region";
+		if (env->cur_state->active_irq_id)
+			return "IRQ-disabled region";
+		if (env->cur_state->active_locks)
+			return "lock region";
+	}
 	return "non-sleepable prog";
 }
 
@@ -12120,12 +12127,17 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 	}
 
 	if (fn->might_sleep && !in_sleepable_context(env)) {
+		const char *suggestion;
+
 		verbose(env, "sleepable helper %s#%d in %s\n", func_id_name(func_id), func_id,
 			non_sleepable_context_description(env));
+		if (in_sleepable(env))
+			suggestion = "Move the helper call outside the critical section, or use a non-sleepable helper.";
+		else
+			suggestion = "Mark the program sleepable if the program type allows it, or use a non-sleepable helper.";
 		operation = bpf_diag_fmt(env, "sleepable helper %s#%d",
 					 func_id_name(func_id), func_id);
-		bpf_diag_ctx_forbidden(env, insn_idx, operation,
-			"Move the helper call outside the critical section, or use a non-sleepable helper.");
+		bpf_diag_ctx_forbidden(env, insn_idx, operation, suggestion);
 		return -EINVAL;
 	}
 
@@ -14745,11 +14757,20 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	}
 
 	sleepable = bpf_is_kfunc_sleepable(&meta);
-	if (sleepable && !in_sleepable(env)) {
-		verbose(env, "program must be sleepable to call sleepable kfunc %s\n", func_name);
+	if (sleepable && !in_sleepable_context(env)) {
+		const char *suggestion;
+
+		if (in_sleepable(env)) {
+			verbose(env, "kernel func %s is sleepable within %s\n",
+				func_name, non_sleepable_context_description(env));
+			suggestion = "Move the kfunc call outside the critical section, or use a non-sleepable kfunc.";
+		} else {
+			verbose(env, "program must be sleepable to call sleepable kfunc %s\n",
+				func_name);
+			suggestion = "Mark the program sleepable if the program type allows it, or use a non-sleepable kfunc.";
+		}
 		operation = bpf_diag_fmt(env, "sleepable kfunc %s", func_name);
-		bpf_diag_ctx_forbidden(env, insn_idx, operation,
-			"Mark the program sleepable if the program type allows it, or use a non-sleepable kfunc.");
+		bpf_diag_ctx_forbidden(env, insn_idx, operation, suggestion);
 		return -EACCES;
 	}
 
@@ -14863,15 +14884,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 			invalidate_rcu_protected_refs(env);
 	}
 
-	if (sleepable && !in_sleepable_context(env)) {
-		verbose(env, "kernel func %s is sleepable within %s\n",
-			func_name, non_sleepable_context_description(env));
-		operation = bpf_diag_fmt(env, "sleepable kfunc %s", func_name);
-		bpf_diag_ctx_forbidden(env, insn_idx, operation,
-			"Move the kfunc call outside the critical section, or use a non-sleepable kfunc.");
-		return -EACCES;
-	}
-
 	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;
-- 
2.53.0

[PATCH bpf-next v2 5/7] selftests/bpf: Test non-sleepable kfunc context

[Kumar Kartikeya Dwivedi]

A sleepable kfunc call can be rejected because the program itself is
not sleepable or because a sleepable program is inside an active
critical section.

Exercise the two causes separately. Call a sleepable kfunc directly
from a non-sleepable tracing program, then call it from a sleepable
tracing program inside an RCU read-side critical section. Assert both
the legacy verifier messages and the structured reasons.

Link: https://lore.kernel.org/bpf/2e42a1a2bf45f4d2aba7495bdc9f147558055740e2f3c8b9dae255f6c57fc13c@mail.kernel.org/
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/progs/preempt_lock.c        | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/preempt_lock.c b/tools/testing/selftests/bpf/progs/preempt_lock.c
index 81c459435680..955391da326a 100644
--- a/tools/testing/selftests/bpf/progs/preempt_lock.c
+++ b/tools/testing/selftests/bpf/progs/preempt_lock.c
@@ -6,6 +6,8 @@
 #include "bpf_experimental.h"
 
 extern int bpf_copy_from_user_str(void *dst, u32 dst__sz, const void *unsafe_ptr__ign, u64 flags) __weak __ksym;
+extern void bpf_rcu_read_lock(void) __ksym;
+extern void bpf_rcu_read_unlock(void) __ksym;
 
 SEC("?tc")
 __failure __msg("BPF_EXIT instruction in main prog cannot be used inside bpf_preempt_disable-ed region")
@@ -179,6 +181,30 @@ int preempt_sleepable_kfunc(void *ctx)
 	return 0;
 }
 
+SEC("?fentry/" SYS_PREFIX "sys_getpgid")
+__failure __msg("program must be sleepable to call sleepable kfunc bpf_copy_from_user_str")
+__msg("cannot be used in non-sleepable program")
+int non_sleepable_kfunc(void *ctx)
+{
+	u32 data;
+
+	bpf_copy_from_user_str(&data, sizeof(data), NULL, 0);
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("kernel func bpf_copy_from_user_str is sleepable within rcu_read_lock region")
+__msg("cannot be used in RCU read lock region")
+int sleepable_kfunc_in_rcu(void *ctx)
+{
+	u32 data;
+
+	bpf_rcu_read_lock();
+	bpf_copy_from_user_str(&data, sizeof(data), NULL, 0);
+	bpf_rcu_read_unlock();
+	return 0;
+}
+
 int __noinline preempt_global_subprog(void)
 {
 	preempt_balance_subprog();
-- 
2.53.0

[PATCH bpf-next v2 6/7] bpf: Correct Program Structure diagnostic context

[Kumar Kartikeya Dwivedi]

Program Structure reports have two attribution gaps. A missing jump
table is reported at the beginning of its subprogram rather than at
the gotox that needs the table, and the early subprogram-layout checks
run before BTF line information is installed.

Pass the failing gotox instruction into the jump-table lookup.

The BTF validator needs the discovered subprogram boundaries together
with the LD_ABS and tail-call properties collected during the layout
scan. Collect those properties, along with the program's callx marker,
with nested subprogram and instruction loops, then validate BTF before
reporting layout errors. This makes validated source information
available to the jump-boundary and fallthrough reports without changing
either check.

Moving BTF validation ahead of normal instruction validation also lets
CO-RE see a truncated final LD_IMM64. Reject a relocation targeting
that instruction before bpf_core_apply() can inspect or patch its
missing second half.

Link: https://lore.kernel.org/bpf/cf2f420c2b21de440a7dc51b1565c0f06d4b539640ee5c03384e5d77bcfb5686@mail.kernel.org/
Fixes: a8f427835394 ("bpf: Report Program Structure CFG errors")
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/cfg.c       |  6 ++---
 kernel/bpf/check_btf.c | 15 +++++++++---
 kernel/bpf/verifier.c  | 54 +++++++++++++++++++++++++++++-------------
 3 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
index 0de2f634ef67..33b98285e802 100644
--- a/kernel/bpf/cfg.c
+++ b/kernel/bpf/cfg.c
@@ -288,7 +288,7 @@ static struct bpf_iarray *jt_from_map(struct bpf_map *map)
  * combined jump table in jt->items (allocated with kvcalloc)
  */
 static struct bpf_iarray *jt_from_subprog(struct bpf_verifier_env *env,
-					  int subprog_start, int subprog_end)
+					  int insn_idx, int subprog_start, int subprog_end)
 {
 	struct bpf_iarray *jt = NULL;
 	struct bpf_map *map;
@@ -328,7 +328,7 @@ static struct bpf_iarray *jt_from_subprog(struct bpf_verifier_env *env,
 	if (!jt) {
 		verbose(env, "no jump tables found for subprog starting at %u\n", subprog_start);
 		bpf_diag_program_structure(
-			env, subprog_start, "missing jump table",
+			env, insn_idx, "missing jump table",
 			"Make sure subprograms containing gotox instructions are accompanied by jump tables referencing these subprograms.",
 			"No jump table was found for the subprogram that starts at instruction %u.",
 			subprog_start);
@@ -350,7 +350,7 @@ create_jt(int t, struct bpf_verifier_env *env)
 	subprog = bpf_find_containing_subprog(env, t);
 	subprog_start = subprog->start;
 	subprog_end = (subprog + 1)->start;
-	jt = jt_from_subprog(env, subprog_start, subprog_end);
+	jt = jt_from_subprog(env, t, subprog_start, subprog_end);
 	if (IS_ERR(jt))
 		return jt;
 
diff --git a/kernel/bpf/check_btf.c b/kernel/bpf/check_btf.c
index 0e8b3ccc7a5b..81f4dbfbf146 100644
--- a/kernel/bpf/check_btf.c
+++ b/kernel/bpf/check_btf.c
@@ -373,6 +373,8 @@ static int check_core_relo(struct bpf_verifier_env *env,
 	 * relocation record one at a time.
 	 */
 	for (i = 0; i < nr_core_relo; i++) {
+		u32 insn_idx;
+
 		/* future proofing when sizeof(bpf_core_relo) changes */
 		err = bpf_check_uarg_tail_zero(u_core_relo, expected_size, rec_size);
 		if (err) {
@@ -391,15 +393,22 @@ static int check_core_relo(struct bpf_verifier_env *env,
 			break;
 		}
 
-		if (core_relo.insn_off % 8 || core_relo.insn_off / 8 >= prog->len) {
+		insn_idx = core_relo.insn_off / 8;
+		if (core_relo.insn_off % 8 || insn_idx >= prog->len) {
 			verbose(env, "Invalid core_relo[%u].insn_off:%u prog->len:%u\n",
 				i, core_relo.insn_off, prog->len);
 			err = -EINVAL;
 			break;
 		}
+		if (insn_idx == prog->len - 1 &&
+		    prog->insnsi[insn_idx].code == (BPF_LD | BPF_IMM | BPF_DW)) {
+			verbose(env, "Invalid core_relo[%u] targets truncated bpf_ld_imm64 insn\n",
+				i);
+			err = -EINVAL;
+			break;
+		}
 
-		err = bpf_core_apply(&ctx, &core_relo, i,
-				     &prog->insnsi[core_relo.insn_off / 8]);
+		err = bpf_core_apply(&ctx, &core_relo, i, &prog->insnsi[insn_idx]);
 		if (err)
 			break;
 		bpfptr_add(&u_core_relo, rec_size);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1c52e7bd570d..63b32a39a0f7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3098,6 +3098,34 @@ static int add_kfuncs(struct bpf_verifier_env *env)
 	return 0;
 }
 
+static void find_subprog_properties(struct bpf_verifier_env *env)
+{
+	struct bpf_subprog_info *subprog = env->subprog_info;
+	struct bpf_insn *insn = env->prog->insnsi;
+	int cur_subprog;
+
+	for (cur_subprog = 0; cur_subprog < env->subprog_cnt; cur_subprog++) {
+		int i;
+
+		for (i = subprog[cur_subprog].start;
+		     i < subprog[cur_subprog + 1].start; i++) {
+			u8 code = insn[i].code;
+
+			if (code == (BPF_JMP | BPF_CALL) &&
+			    insn[i].src_reg == 0 &&
+			    insn[i].imm == BPF_FUNC_tail_call) {
+				subprog[cur_subprog].has_tail_call = true;
+				subprog[cur_subprog].tail_call_reachable = true;
+			}
+			if (BPF_CLASS(code) == BPF_LD &&
+			    (BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
+				subprog[cur_subprog].has_ld_abs = true;
+			if (bpf_is_callx(&insn[i]))
+				env->has_callx = true;
+		}
+	}
+}
+
 static int check_subprogs(struct bpf_verifier_env *env)
 {
 	int i, subprog_start, subprog_end, off, cur_subprog = 0;
@@ -3111,17 +3139,6 @@ static int check_subprogs(struct bpf_verifier_env *env)
 	for (i = 0; i < insn_cnt; i++) {
 		u8 code = insn[i].code;
 
-		if (code == (BPF_JMP | BPF_CALL) &&
-		    insn[i].src_reg == 0 &&
-		    insn[i].imm == BPF_FUNC_tail_call) {
-			subprog[cur_subprog].has_tail_call = true;
-			subprog[cur_subprog].tail_call_reachable = true;
-		}
-		if (BPF_CLASS(code) == BPF_LD &&
-		    (BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
-			subprog[cur_subprog].has_ld_abs = true;
-		if (bpf_is_callx(&insn[i]))
-			env->has_callx = true;
 		if (BPF_CLASS(code) != BPF_JMP && BPF_CLASS(code) != BPF_JMP32)
 			goto next;
 		if (BPF_OP(code) == BPF_CALL)
@@ -3143,9 +3160,10 @@ static int check_subprogs(struct bpf_verifier_env *env)
 		}
 next:
 		if (i == subprog_end - 1) {
-			/* to avoid fall-through from one subprog into another
+			/*
+			 * To avoid fall-through from one subprog into another,
 			 * the last insn of the subprog should be either exit
-			 * or unconditional jump back or bpf_throw call
+			 * or unconditional jump back or bpf_throw call.
 			 */
 			if (code != (BPF_JMP | BPF_EXIT) &&
 			    code != (BPF_JMP32 | BPF_JA) &&
@@ -22410,17 +22428,19 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
 	if (ret < 0)
 		goto skip_full_check;
 
-	/* Discover all subprograms before validating their layout and BTF. */
+	/* Discover all subprograms and collect the properties needed by BTF validation. */
 	ret = add_subprogs(env);
 	if (ret < 0)
 		goto skip_full_check;
 
-	ret = check_subprogs(env);
+	find_subprog_properties(env);
+
+	/* Validate BTF and apply CO-RE before reporting subprogram layout errors. */
+	ret = bpf_check_btf_info(env, attr, uattr);
 	if (ret < 0)
 		goto skip_full_check;
 
-	/* Validate BTF against the complete subprogram layout and apply CO-RE. */
-	ret = bpf_check_btf_info(env, attr, uattr);
+	ret = check_subprogs(env);
 	if (ret < 0)
 		goto skip_full_check;
 
-- 
2.53.0

[PATCH bpf-next v2 7/7] selftests/bpf: Test Program Structure diagnostic context

[Kumar Kartikeya Dwivedi]

Exercise each corrected Program Structure report. Place a
missing-table gotox after another instruction so its attribution
differs from the subprogram start.

Add malformed subprogram layouts for a branch crossing a subprogram
boundary and a subprogram that falls through its end. Both cases carry
BTF line records and assert that their structured reports include the
corresponding source function and file.

Add a raw CO-RE case that targets a truncated final LD_IMM64 and assert
that it is rejected before relocation is applied.

Link: https://lore.kernel.org/bpf/cf2f420c2b21de440a7dc51b1565c0f06d4b539640ee5c03384e5d77bcfb5686@mail.kernel.org/
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/core_reloc_raw.c | 172 ++++++++++++------
 .../selftests/bpf/progs/verifier_cfg.c        |  40 ++++
 .../selftests/bpf/progs/verifier_gotox.c      |   2 +
 3 files changed, 156 insertions(+), 58 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c b/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c
index a18d3680fb16..035a7fe99937 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc_raw.c
@@ -14,64 +14,44 @@
 
 static char log[16 * 1024];
 
-/* Check that verifier rejects BPF program containing relocation
- * pointing to non-existent BTF type.
- */
-static void test_bad_local_id(void)
+struct test_btf {
+	struct btf_header hdr;
+	__u32 types[15];
+	char strings[128];
+};
+
+static struct test_btf raw_btf = {
+	.hdr = {
+		.magic = BTF_MAGIC,
+		.version = BTF_VERSION,
+		.hdr_len = sizeof(struct btf_header),
+		.type_off = 0,
+		.type_len = sizeof(raw_btf.types),
+		.str_off = offsetof(struct test_btf, strings) -
+			   offsetof(struct test_btf, types),
+		.str_len = sizeof(raw_btf.strings),
+	},
+	.types = {
+		BTF_PTR_ENC(0),					/* [1] void*  */
+		BTF_TYPE_INT_ENC(1, BTF_INT_SIGNED, 0, 32, 4),	/* [2] int    */
+		BTF_FUNC_PROTO_ENC(2, 1),			/* [3] int (*)(void*) */
+		BTF_FUNC_PROTO_ARG_ENC(8, 1),
+		BTF_FUNC_ENC(8, 3)			/* [4] FUNC 'foo' type_id=2   */
+	},
+	.strings = "\0int\0 0\0foo\0"
+};
+
+static int load_test_btf(void)
 {
-	struct test_btf {
-		struct btf_header hdr;
-		__u32 types[15];
-		char strings[128];
-	} raw_btf = {
-		.hdr = {
-			.magic = BTF_MAGIC,
-			.version = BTF_VERSION,
-			.hdr_len = sizeof(struct btf_header),
-			.type_off = 0,
-			.type_len = sizeof(raw_btf.types),
-			.str_off = offsetof(struct test_btf, strings) -
-				   offsetof(struct test_btf, types),
-			.str_len = sizeof(raw_btf.strings),
-		},
-		.types = {
-			BTF_PTR_ENC(0),					/* [1] void*  */
-			BTF_TYPE_INT_ENC(1, BTF_INT_SIGNED, 0, 32, 4),	/* [2] int    */
-			BTF_FUNC_PROTO_ENC(2, 1),			/* [3] int (*)(void*) */
-			BTF_FUNC_PROTO_ARG_ENC(8, 1),
-			BTF_FUNC_ENC(8, 3)			/* [4] FUNC 'foo' type_id=2   */
-		},
-		.strings = "\0int\0 0\0foo\0"
-	};
 	__u32 log_level = 1 | 2 | 4;
 	LIBBPF_OPTS(bpf_btf_load_opts, opts,
 		    .log_buf = log,
 		    .log_size = sizeof(log),
 		    .log_level = log_level,
 	);
-	struct bpf_insn insns[] = {
-		BPF_ALU64_IMM(BPF_MOV, BPF_REG_0, 0),
-		BPF_EXIT_INSN(),
-	};
-	struct bpf_func_info funcs[] = {
-		{
-			.insn_off = 0,
-			.type_id = 4,
-		}
-	};
-	struct bpf_core_relo relos[] = {
-		{
-			.insn_off = 0,		/* patch first instruction (r0 = 0) */
-			.type_id = 100500,	/* !!! this type id does not exist */
-			.access_str_off = 6,	/* offset of "0" */
-			.kind = BPF_CORE_TYPE_ID_LOCAL,
-		}
-	};
-	union bpf_attr attr;
-	int saved_errno;
-	int prog_fd = -1;
-	int btf_fd = -1;
+	int saved_errno, btf_fd;
 
+	log[0] = 0;
 	btf_fd = bpf_btf_load(&raw_btf, sizeof(raw_btf), &opts);
 	saved_errno = errno;
 	if (btf_fd < 0 || env.verbosity > VERBOSE_NORMAL) {
@@ -79,18 +59,30 @@ static void test_bad_local_id(void)
 		printf("%s", log);
 		printf("-------- BTF load log end ----------\n");
 	}
-	if (btf_fd < 0) {
+	if (btf_fd < 0)
 		PRINT_FAIL("bpf_btf_load() failed, errno=%d\n", saved_errno);
-		return;
-	}
+	return btf_fd;
+}
+
+static int load_test_prog(int btf_fd, struct bpf_insn *insns, size_t insn_cnt,
+			  struct bpf_core_relo *relos, size_t relo_cnt)
+{
+	struct bpf_func_info funcs[] = {
+		{
+			.insn_off = 0,
+			.type_id = 4,
+		}
+	};
+	__u32 log_level = 1 | 2 | 4;
+	union bpf_attr attr = {};
+	int prog_fd;
 
 	log[0] = 0;
-	memset(&attr, 0, sizeof(attr));
 	attr.prog_btf_fd = btf_fd;
 	attr.prog_type = BPF_TRACE_RAW_TP;
 	attr.license = (__u64)"GPL";
-	attr.insns = (__u64)&insns;
-	attr.insn_cnt = sizeof(insns) / sizeof(*insns);
+	attr.insns = (__u64)insns;
+	attr.insn_cnt = insn_cnt;
 	attr.log_buf = (__u64)log;
 	attr.log_size = sizeof(log);
 	attr.log_level = log_level;
@@ -98,15 +90,43 @@ static void test_bad_local_id(void)
 	attr.func_info_cnt = sizeof(funcs) / sizeof(*funcs);
 	attr.func_info_rec_size = sizeof(*funcs);
 	attr.core_relos = (__u64)relos;
-	attr.core_relo_cnt = sizeof(relos) / sizeof(*relos);
+	attr.core_relo_cnt = relo_cnt;
 	attr.core_relo_rec_size = sizeof(*relos);
 	prog_fd = sys_bpf_prog_load(&attr, sizeof(attr), 1);
-	saved_errno = errno;
 	if (prog_fd < 0 || env.verbosity > VERBOSE_NORMAL) {
 		printf("-------- program load log start --------\n");
 		printf("%s", log);
 		printf("-------- program load log end ----------\n");
 	}
+	return prog_fd;
+}
+
+/* Check that verifier rejects BPF program containing relocation
+ * pointing to non-existent BTF type.
+ */
+static void test_bad_local_id(void)
+{
+	struct bpf_insn insns[] = {
+		BPF_ALU64_IMM(BPF_MOV, BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	};
+	struct bpf_core_relo relos[] = {
+		{
+			.insn_off = 0,		/* patch first instruction (r0 = 0) */
+			.type_id = 100500,	/* !!! this type id does not exist */
+			.access_str_off = 6,	/* offset of "0" */
+			.kind = BPF_CORE_TYPE_ID_LOCAL,
+		}
+	};
+	int prog_fd = -1;
+	int btf_fd = -1;
+
+	btf_fd = load_test_btf();
+	if (btf_fd < 0)
+		return;
+
+	prog_fd = load_test_prog(btf_fd, insns, ARRAY_SIZE(insns),
+				 relos, ARRAY_SIZE(relos));
 	if (prog_fd >= 0) {
 		PRINT_FAIL("sys_bpf_prog_load() expected to fail\n");
 		goto out;
@@ -118,8 +138,44 @@ static void test_bad_local_id(void)
 	close(btf_fd);
 }
 
+static void test_truncated_ldimm64(void)
+{
+	struct bpf_insn insns[] = {
+		BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, BPF_REG_0, 0, 0, 0),
+	};
+	struct bpf_core_relo relos[] = {
+		{
+			.insn_off = 0,
+			.type_id = 2,
+			.access_str_off = 6,
+			.kind = BPF_CORE_TYPE_ID_LOCAL,
+		}
+	};
+	int prog_fd = -1;
+	int btf_fd = -1;
+
+	btf_fd = load_test_btf();
+	if (btf_fd < 0)
+		return;
+
+	prog_fd = load_test_prog(btf_fd, insns, ARRAY_SIZE(insns),
+				 relos, ARRAY_SIZE(relos));
+	if (prog_fd >= 0) {
+		PRINT_FAIL("sys_bpf_prog_load() expected to fail\n");
+		goto out;
+	}
+	ASSERT_HAS_SUBSTR(log, "core_relo[0] targets truncated bpf_ld_imm64 insn",
+			  "program load log");
+
+out:
+	close(prog_fd);
+	close(btf_fd);
+}
+
 void test_core_reloc_raw(void)
 {
 	if (test__start_subtest("bad_local_id"))
 		test_bad_local_id();
+	if (test__start_subtest("truncated_ldimm64"))
+		test_truncated_ldimm64();
 }
diff --git a/tools/testing/selftests/bpf/progs/verifier_cfg.c b/tools/testing/selftests/bpf/progs/verifier_cfg.c
index 3c3bb03e8217..a8396d4fd5f3 100644
--- a/tools/testing/selftests/bpf/progs/verifier_cfg.c
+++ b/tools/testing/selftests/bpf/progs/verifier_cfg.c
@@ -56,6 +56,46 @@ __naked void out_of_range_jump2(void)
 "	::: __clobber_all);
 }
 
+static __naked __noinline __used int cross_subprog_target(void)
+{
+	asm volatile ("					\
+	r0 = 0;						\
+	exit;						\
+"	::: __clobber_all);
+}
+
+SEC("socket")
+__description("jump across subprogram boundary")
+__failure __msg("jump out of range from insn 1")
+__msg("jump_across_subprog_boundary @ verifier_cfg.c")
+__naked void jump_across_subprog_boundary(void)
+{
+	asm volatile ("					\
+	call cross_subprog_target;			\
+	goto +1;					\
+	exit;						\
+"	::: __clobber_all);
+}
+
+static __naked __noinline __used int fallthrough_subprog(void)
+{
+	asm volatile ("					\
+	r0 = 0;						\
+"	::: __clobber_all);
+}
+
+SEC("socket")
+__description("subprogram fallthrough")
+__failure __msg("last insn is not an exit or jmp")
+__msg("fallthrough_subprog @ verifier_cfg.c")
+__naked void subprog_fallthrough(void)
+{
+	asm volatile ("					\
+	call fallthrough_subprog;			\
+	exit;						\
+"	::: __clobber_all);
+}
+
 SEC("socket")
 __description("invalid DW LDSX instruction in diagnostics")
 __failure __msg("BUG_ldx_99")
diff --git a/tools/testing/selftests/bpf/progs/verifier_gotox.c b/tools/testing/selftests/bpf/progs/verifier_gotox.c
index 5b18c9a27717..a835a4871021 100644
--- a/tools/testing/selftests/bpf/progs/verifier_gotox.c
+++ b/tools/testing/selftests/bpf/progs/verifier_gotox.c
@@ -53,9 +53,11 @@ DEFINE_SIMPLE_JUMP_TABLE_PROG(reserved_field_non_zero_imm, BPF_REG_0, 0, 1, __fa
  */
 SEC("socket")
 __failure __msg("no jump tables found for subprog starting at 0")
+__msg(">>> 1 | (0d) gotox r0")
 __naked void jump_table_no_jump_table(void)
 {
 	asm volatile ("						\
+	r0 = 0;							\
 	.8byte %[gotox_r0];					\
 	r0 = 1;							\
 	exit;							\
-- 
2.53.0

Re: [PATCH bpf-next v2 6/7] bpf: Correct Program Structure diagnostic context

[Alexei Starovoitov]

On Thu Sep 24, 2026 at 9:29 AM UTC, Kumar Kartikeya Dwivedi wrote:
> Program Structure reports have two attribution gaps. A missing jump
> table is reported at the beginning of its subprogram rather than at
> the gotox that needs the table, and the early subprogram-layout checks
> run before BTF line information is installed.
>
> Pass the failing gotox instruction into the jump-table lookup.
>
> The BTF validator needs the discovered subprogram boundaries together
> with the LD_ABS and tail-call properties collected during the layout
> scan. Collect those properties, along with the program's callx marker,
> with nested subprogram and instruction loops, then validate BTF before
> reporting layout errors. This makes validated source information
> available to the jump-boundary and fallthrough reports without changing
> either check.
>
> Moving BTF validation ahead of normal instruction validation also lets
> CO-RE see a truncated final LD_IMM64. Reject a relocation targeting
> that instruction before bpf_core_apply() can inspect or patch its
> missing second half.
>
> Link: https://lore.kernel.org/bpf/cf2f420c2b21de440a7dc51b1565c0f06d4b539640ee5c03384e5d77bcfb5686@mail.kernel.org/
> Fixes: a8f427835394 ("bpf: Report Program Structure CFG errors")
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  kernel/bpf/cfg.c       |  6 ++---
>  kernel/bpf/check_btf.c | 15 +++++++++---
>  kernel/bpf/verifier.c  | 54 +++++++++++++++++++++++++++++-------------
>  3 files changed, 52 insertions(+), 23 deletions(-)
>
> diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
> index 0de2f634ef67..33b98285e802 100644
> --- a/kernel/bpf/cfg.c
> +++ b/kernel/bpf/cfg.c
> @@ -288,7 +288,7 @@ static struct bpf_iarray *jt_from_map(struct bpf_map *map)
>   * combined jump table in jt->items (allocated with kvcalloc)
>   */
>  static struct bpf_iarray *jt_from_subprog(struct bpf_verifier_env *env,
> -					  int subprog_start, int subprog_end)
> +					  int insn_idx, int subprog_start, int subprog_end)
>  {
>  	struct bpf_iarray *jt = NULL;
>  	struct bpf_map *map;
> @@ -328,7 +328,7 @@ static struct bpf_iarray *jt_from_subprog(struct bpf_verifier_env *env,
>  	if (!jt) {
>  		verbose(env, "no jump tables found for subprog starting at %u\n", subprog_start);
>  		bpf_diag_program_structure(
> -			env, subprog_start, "missing jump table",
> +			env, insn_idx, "missing jump table",
>  			"Make sure subprograms containing gotox instructions are accompanied by jump tables referencing these subprograms.",
>  			"No jump table was found for the subprogram that starts at instruction %u.",
>  			subprog_start);
> @@ -350,7 +350,7 @@ create_jt(int t, struct bpf_verifier_env *env)
>  	subprog = bpf_find_containing_subprog(env, t);
>  	subprog_start = subprog->start;
>  	subprog_end = (subprog + 1)->start;
> -	jt = jt_from_subprog(env, subprog_start, subprog_end);
> +	jt = jt_from_subprog(env, t, subprog_start, subprog_end);
>  	if (IS_ERR(jt))
>  		return jt;
>  
> diff --git a/kernel/bpf/check_btf.c b/kernel/bpf/check_btf.c
> index 0e8b3ccc7a5b..81f4dbfbf146 100644
> --- a/kernel/bpf/check_btf.c
> +++ b/kernel/bpf/check_btf.c
> @@ -373,6 +373,8 @@ static int check_core_relo(struct bpf_verifier_env *env,
>  	 * relocation record one at a time.
>  	 */
>  	for (i = 0; i < nr_core_relo; i++) {
> +		u32 insn_idx;
> +
>  		/* future proofing when sizeof(bpf_core_relo) changes */
>  		err = bpf_check_uarg_tail_zero(u_core_relo, expected_size, rec_size);
>  		if (err) {
> @@ -391,15 +393,22 @@ static int check_core_relo(struct bpf_verifier_env *env,
>  			break;
>  		}
>  
> -		if (core_relo.insn_off % 8 || core_relo.insn_off / 8 >= prog->len) {
> +		insn_idx = core_relo.insn_off / 8;
> +		if (core_relo.insn_off % 8 || insn_idx >= prog->len) {
>  			verbose(env, "Invalid core_relo[%u].insn_off:%u prog->len:%u\n",
>  				i, core_relo.insn_off, prog->len);
>  			err = -EINVAL;
>  			break;
>  		}
> +		if (insn_idx == prog->len - 1 &&
> +		    prog->insnsi[insn_idx].code == (BPF_LD | BPF_IMM | BPF_DW)) {
> +			verbose(env, "Invalid core_relo[%u] targets truncated bpf_ld_imm64 insn\n",
> +				i);
> +			err = -EINVAL;
> +			break;
> +		}
>  
> -		err = bpf_core_apply(&ctx, &core_relo, i,
> -				     &prog->insnsi[core_relo.insn_off / 8]);
> +		err = bpf_core_apply(&ctx, &core_relo, i, &prog->insnsi[insn_idx]);
>  		if (err)
>  			break;
>  		bpfptr_add(&u_core_relo, rec_size);
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1c52e7bd570d..63b32a39a0f7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3098,6 +3098,34 @@ static int add_kfuncs(struct bpf_verifier_env *env)
>  	return 0;
>  }
>  
> +static void find_subprog_properties(struct bpf_verifier_env *env)
> +{
> +	struct bpf_subprog_info *subprog = env->subprog_info;
> +	struct bpf_insn *insn = env->prog->insnsi;
> +	int cur_subprog;
> +
> +	for (cur_subprog = 0; cur_subprog < env->subprog_cnt; cur_subprog++) {
> +		int i;
> +
> +		for (i = subprog[cur_subprog].start;
> +		     i < subprog[cur_subprog + 1].start; i++) {
> +			u8 code = insn[i].code;
> +
> +			if (code == (BPF_JMP | BPF_CALL) &&
> +			    insn[i].src_reg == 0 &&
> +			    insn[i].imm == BPF_FUNC_tail_call) {
> +				subprog[cur_subprog].has_tail_call = true;
> +				subprog[cur_subprog].tail_call_reachable = true;
> +			}
> +			if (BPF_CLASS(code) == BPF_LD &&
> +			    (BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
> +				subprog[cur_subprog].has_ld_abs = true;
> +			if (bpf_is_callx(&insn[i]))
> +				env->has_callx = true;
> +		}
> +	}
> +}
> +
>  static int check_subprogs(struct bpf_verifier_env *env)
>  {
>  	int i, subprog_start, subprog_end, off, cur_subprog = 0;
> @@ -3111,17 +3139,6 @@ static int check_subprogs(struct bpf_verifier_env *env)
>  	for (i = 0; i < insn_cnt; i++) {
>  		u8 code = insn[i].code;
>  
> -		if (code == (BPF_JMP | BPF_CALL) &&
> -		    insn[i].src_reg == 0 &&
> -		    insn[i].imm == BPF_FUNC_tail_call) {
> -			subprog[cur_subprog].has_tail_call = true;
> -			subprog[cur_subprog].tail_call_reachable = true;
> -		}
> -		if (BPF_CLASS(code) == BPF_LD &&
> -		    (BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
> -			subprog[cur_subprog].has_ld_abs = true;
> -		if (bpf_is_callx(&insn[i]))
> -			env->has_callx = true;
>  		if (BPF_CLASS(code) != BPF_JMP && BPF_CLASS(code) != BPF_JMP32)
>  			goto next;
>  		if (BPF_OP(code) == BPF_CALL)
> @@ -3143,9 +3160,10 @@ static int check_subprogs(struct bpf_verifier_env *env)
>  		}
>  next:
>  		if (i == subprog_end - 1) {
> -			/* to avoid fall-through from one subprog into another
> +			/*
> +			 * To avoid fall-through from one subprog into another,
>  			 * the last insn of the subprog should be either exit
> -			 * or unconditional jump back or bpf_throw call
> +			 * or unconditional jump back or bpf_throw call.
>  			 */
>  			if (code != (BPF_JMP | BPF_EXIT) &&
>  			    code != (BPF_JMP32 | BPF_JA) &&
> @@ -22410,17 +22428,19 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
>  	if (ret < 0)
>  		goto skip_full_check;
>  
> -	/* Discover all subprograms before validating their layout and BTF. */
> +	/* Discover all subprograms and collect the properties needed by BTF validation. */
>  	ret = add_subprogs(env);
>  	if (ret < 0)
>  		goto skip_full_check;
>  
> -	ret = check_subprogs(env);
> +	find_subprog_properties(env);
> +
> +	/* Validate BTF and apply CO-RE before reporting subprogram layout errors. */
> +	ret = bpf_check_btf_info(env, attr, uattr);
>  	if (ret < 0)
>  		goto skip_full_check;
>  
> -	/* Validate BTF against the complete subprogram layout and apply CO-RE. */
> -	ret = bpf_check_btf_info(env, attr, uattr);
> +	ret = check_subprogs(env);

I think this is undoing your own fix
commit c26e97721b172163

Re: [PATCH bpf-next v2 6/7] bpf: Correct Program Structure diagnostic context

[Kumar Kartikeya Dwivedi]

On Thu Sep 24, 2026 at 11:05 PM CEST, Alexei Starovoitov wrote:
> On Thu Sep 24, 2026 at 9:29 AM UTC, Kumar Kartikeya Dwivedi wrote:
>> Program Structure reports have two attribution gaps. A missing jump
>> table is reported at the beginning of its subprogram rather than at
>> the gotox that needs the table, and the early subprogram-layout checks
>> run before BTF line information is installed.
>>
>> Pass the failing gotox instruction into the jump-table lookup.
>>
>> The BTF validator needs the discovered subprogram boundaries together
>> with the LD_ABS and tail-call properties collected during the layout
>> scan. Collect those properties, along with the program's callx marker,
>> with nested subprogram and instruction loops, then validate BTF before
>> reporting layout errors. This makes validated source information
>> available to the jump-boundary and fallthrough reports without changing
>> either check.
>>
>> Moving BTF validation ahead of normal instruction validation also lets
>> CO-RE see a truncated final LD_IMM64. Reject a relocation targeting
>> that instruction before bpf_core_apply() can inspect or patch its
>> missing second half.
>>
>> Link: https://lore.kernel.org/bpf/cf2f420c2b21de440a7dc51b1565c0f06d4b539640ee5c03384e5d77bcfb5686@mail.kernel.org/
>> Fixes: a8f427835394 ("bpf: Report Program Structure CFG errors")
>> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>> ---
>>  kernel/bpf/cfg.c       |  6 ++---
>>  kernel/bpf/check_btf.c | 15 +++++++++---
>>  kernel/bpf/verifier.c  | 54 +++++++++++++++++++++++++++++-------------
>>  3 files changed, 52 insertions(+), 23 deletions(-)
>>
>> diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
>> index 0de2f634ef67..33b98285e802 100644
>> --- a/kernel/bpf/cfg.c
>> +++ b/kernel/bpf/cfg.c
>> @@ -288,7 +288,7 @@ static struct bpf_iarray *jt_from_map(struct bpf_map *map)
>>   * combined jump table in jt->items (allocated with kvcalloc)
>>   */
>>  static struct bpf_iarray *jt_from_subprog(struct bpf_verifier_env *env,
>> -					  int subprog_start, int subprog_end)
>> +					  int insn_idx, int subprog_start, int subprog_end)
>>  {
>>  	struct bpf_iarray *jt = NULL;
>>  	struct bpf_map *map;
>> @@ -328,7 +328,7 @@ static struct bpf_iarray *jt_from_subprog(struct bpf_verifier_env *env,
>>  	if (!jt) {
>>  		verbose(env, "no jump tables found for subprog starting at %u\n", subprog_start);
>>  		bpf_diag_program_structure(
>> -			env, subprog_start, "missing jump table",
>> +			env, insn_idx, "missing jump table",
>>  			"Make sure subprograms containing gotox instructions are accompanied by jump tables referencing these subprograms.",
>>  			"No jump table was found for the subprogram that starts at instruction %u.",
>>  			subprog_start);
>> @@ -350,7 +350,7 @@ create_jt(int t, struct bpf_verifier_env *env)
>>  	subprog = bpf_find_containing_subprog(env, t);
>>  	subprog_start = subprog->start;
>>  	subprog_end = (subprog + 1)->start;
>> -	jt = jt_from_subprog(env, subprog_start, subprog_end);
>> +	jt = jt_from_subprog(env, t, subprog_start, subprog_end);
>>  	if (IS_ERR(jt))
>>  		return jt;
>>
>> diff --git a/kernel/bpf/check_btf.c b/kernel/bpf/check_btf.c
>> index 0e8b3ccc7a5b..81f4dbfbf146 100644
>> --- a/kernel/bpf/check_btf.c
>> +++ b/kernel/bpf/check_btf.c
>> @@ -373,6 +373,8 @@ static int check_core_relo(struct bpf_verifier_env *env,
>>  	 * relocation record one at a time.
>>  	 */
>>  	for (i = 0; i < nr_core_relo; i++) {
>> +		u32 insn_idx;
>> +
>>  		/* future proofing when sizeof(bpf_core_relo) changes */
>>  		err = bpf_check_uarg_tail_zero(u_core_relo, expected_size, rec_size);
>>  		if (err) {
>> @@ -391,15 +393,22 @@ static int check_core_relo(struct bpf_verifier_env *env,
>>  			break;
>>  		}
>>
>> -		if (core_relo.insn_off % 8 || core_relo.insn_off / 8 >= prog->len) {
>> +		insn_idx = core_relo.insn_off / 8;
>> +		if (core_relo.insn_off % 8 || insn_idx >= prog->len) {
>>  			verbose(env, "Invalid core_relo[%u].insn_off:%u prog->len:%u\n",
>>  				i, core_relo.insn_off, prog->len);
>>  			err = -EINVAL;
>>  			break;
>>  		}
>> +		if (insn_idx == prog->len - 1 &&
>> +		    prog->insnsi[insn_idx].code == (BPF_LD | BPF_IMM | BPF_DW)) {
>> +			verbose(env, "Invalid core_relo[%u] targets truncated bpf_ld_imm64 insn\n",
>> +				i);
>> +			err = -EINVAL;
>> +			break;
>> +		}
>>
>> -		err = bpf_core_apply(&ctx, &core_relo, i,
>> -				     &prog->insnsi[core_relo.insn_off / 8]);
>> +		err = bpf_core_apply(&ctx, &core_relo, i, &prog->insnsi[insn_idx]);
>>  		if (err)
>>  			break;
>>  		bpfptr_add(&u_core_relo, rec_size);
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 1c52e7bd570d..63b32a39a0f7 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -3098,6 +3098,34 @@ static int add_kfuncs(struct bpf_verifier_env *env)
>>  	return 0;
>>  }
>>
>> +static void find_subprog_properties(struct bpf_verifier_env *env)
>> +{
>> +	struct bpf_subprog_info *subprog = env->subprog_info;
>> +	struct bpf_insn *insn = env->prog->insnsi;
>> +	int cur_subprog;
>> +
>> +	for (cur_subprog = 0; cur_subprog < env->subprog_cnt; cur_subprog++) {
>> +		int i;
>> +
>> +		for (i = subprog[cur_subprog].start;
>> +		     i < subprog[cur_subprog + 1].start; i++) {
>> +			u8 code = insn[i].code;
>> +
>> +			if (code == (BPF_JMP | BPF_CALL) &&
>> +			    insn[i].src_reg == 0 &&
>> +			    insn[i].imm == BPF_FUNC_tail_call) {
>> +				subprog[cur_subprog].has_tail_call = true;
>> +				subprog[cur_subprog].tail_call_reachable = true;
>> +			}
>> +			if (BPF_CLASS(code) == BPF_LD &&
>> +			    (BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
>> +				subprog[cur_subprog].has_ld_abs = true;
>> +			if (bpf_is_callx(&insn[i]))
>> +				env->has_callx = true;
>> +		}
>> +	}
>> +}
>> +
>>  static int check_subprogs(struct bpf_verifier_env *env)
>>  {
>>  	int i, subprog_start, subprog_end, off, cur_subprog = 0;
>> @@ -3111,17 +3139,6 @@ static int check_subprogs(struct bpf_verifier_env *env)
>>  	for (i = 0; i < insn_cnt; i++) {
>>  		u8 code = insn[i].code;
>>
>> -		if (code == (BPF_JMP | BPF_CALL) &&
>> -		    insn[i].src_reg == 0 &&
>> -		    insn[i].imm == BPF_FUNC_tail_call) {
>> -			subprog[cur_subprog].has_tail_call = true;
>> -			subprog[cur_subprog].tail_call_reachable = true;
>> -		}
>> -		if (BPF_CLASS(code) == BPF_LD &&
>> -		    (BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
>> -			subprog[cur_subprog].has_ld_abs = true;
>> -		if (bpf_is_callx(&insn[i]))
>> -			env->has_callx = true;
>>  		if (BPF_CLASS(code) != BPF_JMP && BPF_CLASS(code) != BPF_JMP32)
>>  			goto next;
>>  		if (BPF_OP(code) == BPF_CALL)
>> @@ -3143,9 +3160,10 @@ static int check_subprogs(struct bpf_verifier_env *env)
>>  		}
>>  next:
>>  		if (i == subprog_end - 1) {
>> -			/* to avoid fall-through from one subprog into another
>> +			/*
>> +			 * To avoid fall-through from one subprog into another,
>>  			 * the last insn of the subprog should be either exit
>> -			 * or unconditional jump back or bpf_throw call
>> +			 * or unconditional jump back or bpf_throw call.
>>  			 */
>>  			if (code != (BPF_JMP | BPF_EXIT) &&
>>  			    code != (BPF_JMP32 | BPF_JA) &&
>> @@ -22410,17 +22428,19 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
>>  	if (ret < 0)
>>  		goto skip_full_check;
>>
>> -	/* Discover all subprograms before validating their layout and BTF. */
>> +	/* Discover all subprograms and collect the properties needed by BTF validation. */
>>  	ret = add_subprogs(env);
>>  	if (ret < 0)
>>  		goto skip_full_check;
>>
>> -	ret = check_subprogs(env);
>> +	find_subprog_properties(env);
>> +
>> +	/* Validate BTF and apply CO-RE before reporting subprogram layout errors. */
>> +	ret = bpf_check_btf_info(env, attr, uattr);
>>  	if (ret < 0)
>>  		goto skip_full_check;
>>
>> -	/* Validate BTF against the complete subprogram layout and apply CO-RE. */
>> -	ret = bpf_check_btf_info(env, attr, uattr);
>> +	ret = check_subprogs(env);
>
> I think this is undoing your own fix
> commit c26e97721b172163

Only find_subprog_properties() is moving ahead to let us see func/line info for
formatting messages.

I think you missed v3, that was the most recent posting. Already dropped CO-RE
change there.

https://lore.kernel.org/bpf/20260924170646.2366016-7-memxor@gmail.com

Should I resend last 3 commits again (or just patch 6)?