Re: [PATCH bpf-next v4 12/16] bpf: Report Execution Context Safety errors

On Thu, 2026-08-13 at 01:33 +0200, Kumar Kartikeya Dwivedi wrote:

...

diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c

...

+void bpf_diag_ctx(struct bpf_verifier_env *env, enum bpf_diag_ctx_report report, u32 insn_idx,
+		  const char *operation, enum bpf_diag_context_kind ctx_kind, const char *context,
+		  const char *suggestion)

Nit: expose diag_ctx_{forbidden,active,underflow} and drop this function and enum?

+{
+	switch (report) {
+	case BPF_DIAG_CTX_FORBIDDEN:
+		diag_ctx_forbidden(env, insn_idx, operation, ctx_kind, context,
+				   diag_context_constraint(ctx_kind), suggestion);
+		return;
+	case BPF_DIAG_CTX_ACTIVE:
+		diag_ctx_active(env, insn_idx, operation, ctx_kind, context, suggestion);
+		return;
+	case BPF_DIAG_CTX_UNDERFLOW:
+		diag_ctx_underflow(env, insn_idx, operation, ctx_kind, suggestion);
+		return;
+	}
+}

...

> @@ -2005,23 +2174,6 @@ static void diag_print_ref_event(struct bpf_verifier_env *env,
>  			       event->ref.ref_id);
>  }
>  
> -static const char *diag_context_name(enum bpf_diag_context_kind kind)
> -{
> -	switch (kind) {
> -	case BPF_DIAG_CONTEXT_RCU:
> -		return "RCU read lock region";
> -	case BPF_DIAG_CONTEXT_PREEMPT:
> -		return "non-preemptible region";
> -	case BPF_DIAG_CONTEXT_IRQ:
> -		return "IRQ-disabled region";
> -	case BPF_DIAG_CONTEXT_LOCK:
> -		return "lock region";
> -	case BPF_DIAG_CONTEXT_NONE:
> -	default:
> -		return "context";
> -	}
> -}

Nit: the move is churn.

...

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 7e28f4d9f5c4..71893c36ecce 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

...

> @@ -1104,9 +1106,7 @@ static int unmark_stack_slot_irq_flag(struct bpf_verifier_env *env, struct bpf_r
>  	st = &slot->spilled_ptr;
>  
>  	if (st->irq.kfunc_class != kfunc_class) {
> -		const char *fmt = "This IRQ flag was saved by %s IRQ kfuncs, but the restore call "
> -				  "belongs to the %s IRQ kfunc family. Save and restore operations "
> -				  "must use the same family.";
> +		const char *fmt = "This IRQ flag was saved by %s IRQ kfuncs, but the restore call belongs to the %s IRQ kfunc family. Save and restore operations must use the same family.";

Churn.

>  		const char *flag_kfunc = st->irq.kfunc_class == IRQ_NATIVE_KFUNC ? "native" :
>  										   "lock";
>  		const char *used_kfunc = kfunc_class == IRQ_NATIVE_KFUNC ? "native" : "lock";
> @@ -1115,10 +1115,10 @@ static int unmark_stack_slot_irq_flag(struct bpf_verifier_env *env, struct bpf_r
>  		verbose(env, "irq flag acquired by %s kfuncs cannot be restored with %s kfuncs\n",
>  			flag_kfunc, used_kfunc);
>  		reason = bpf_diag_fmt(env, fmt, flag_kfunc, used_kfunc);
> -		bpf_diag_irq(env, env->insn_idx, "IRQ flag restore mismatch", reason,
> -			     "Restore the flag with the matching IRQ restore kfunc for the save "
> -			     "operation that created it.",
> -			     bpf_diag_irq_depth(env->cur_state));
> +		bpf_diag_irq(
> +			env, env->insn_idx, "IRQ flag restore mismatch", reason,
> +			"Restore the flag with the matching IRQ restore kfunc for the save operation that created it.",
> +			bpf_diag_irq_depth(env->cur_state));

Churn.

>  		return -EINVAL;
>  	}
>  
> @@ -1136,11 +1136,11 @@ static int unmark_stack_slot_irq_flag(struct bpf_verifier_env *env, struct bpf_r
>  
>  		verbose(env, "cannot restore irq state out of order, expected id=%d acquired at insn_idx=%d\n",
>  			env->cur_state->active_irq_id, insn_idx);
> -		bpf_diag_irq(env, env->insn_idx, "IRQ flag restore out of order",
> -			     "IRQ-disabled regions must be restored in last-in, first-out order, "
> -			     "but this restore does not match the currently active IRQ flag.",
> -			     "Restore nested IRQ flags in the reverse order they were saved.",
> -			     bpf_diag_irq_depth(env->cur_state));
> +		bpf_diag_irq(
> +			env, env->insn_idx, "IRQ flag restore out of order",
> +			"IRQ-disabled regions must be restored in last-in, first-out order, but this restore does not match the currently active IRQ flag.",
> +			"Restore nested IRQ flags in the reverse order they were saved.",
> +			bpf_diag_irq_depth(env->cur_state));
>  		return err;

Churn.

>  	}
>  

...

> @@ -10551,6 +10578,36 @@ static const char *non_sleepable_context_description(struct bpf_verifier_env *en
>  	return "non-sleepable prog";
>  }
>  
> +static enum bpf_diag_context_kind non_sleepable_context_kind(struct bpf_verifier_env *env)
> +{
> +	if (env->cur_state->active_rcu_locks)
> +		return BPF_DIAG_CONTEXT_RCU;
> +	if (env->cur_state->active_preempt_locks)
> +		return BPF_DIAG_CONTEXT_PREEMPT;
> +	if (env->cur_state->active_irq_id)
> +		return BPF_DIAG_CONTEXT_IRQ;
> +	if (env->cur_state->active_locks)
> +		return BPF_DIAG_CONTEXT_LOCK;
> +	return BPF_DIAG_CONTEXT_NONE;
> +}
> +
> +static const char *non_sleepable_context_diag_description(struct bpf_verifier_env *env)

Drop the BPF_DIAG_CONTEXT_RCU & Co and use `const char *`?

> +{
> +	switch (non_sleepable_context_kind(env)) {
> +	case BPF_DIAG_CONTEXT_RCU:
> +		return "RCU read lock region";
> +	case BPF_DIAG_CONTEXT_PREEMPT:
> +		return "non-preemptible region";
> +	case BPF_DIAG_CONTEXT_IRQ:
> +		return "IRQ-disabled region";
> +	case BPF_DIAG_CONTEXT_LOCK:
> +		return "lock region";
> +	case BPF_DIAG_CONTEXT_NONE:
> +	default:
> +		return "non-sleepable program";
> +	}
> +}
> +
>  static int release_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>  		       bool convert_rcu, bool release_dynptr)
>  {

...

> @@ -11919,12 +11983,10 @@ static int process_irq_flag(struct bpf_verifier_env *env, struct bpf_reg_state *
>  		if (!is_irq_flag_reg_valid_uninit(env, reg)) {
>  			verbose(env, "expected uninitialized irq flag as %s\n",
>  				reg_arg_name(env, argno));
> -			bpf_diag_res(env, env->insn_idx, "IRQ flag is already initialized",
> -				     "Saving IRQ state requires an uninitialized stack slot for "
> -				     "the IRQ flag, but this slot already contains tracked IRQ "
> -				     "flag state.",
> -				     "Use a fresh stack slot for this save operation, or restore "
> -				     "the existing IRQ flag before reusing the slot.");
> +			bpf_diag_res(
> +				env, env->insn_idx, "IRQ flag is already initialized",
> +				"Saving IRQ state requires an uninitialized stack slot for the IRQ flag, but this slot already contains tracked IRQ flag state.",
> +				"Use a fresh stack slot for this save operation, or restore the existing IRQ flag before reusing the slot.");

Churn.

>  			return -EINVAL;
>  		}
>  
> @@ -11941,11 +12003,10 @@ static int process_irq_flag(struct bpf_verifier_env *env, struct bpf_reg_state *
>  		if (err) {
>  			verbose(env, "expected an initialized irq flag as %s\n",
>  				reg_arg_name(env, argno));
> -			bpf_diag_res(env, env->insn_idx, "uninitialized IRQ flag restore",
> -				     "Restoring IRQ state requires a stack slot that was "
> -				     "initialized by a matching IRQ save operation on this path.",
> -				     "Pass the same stack slot that was previously initialized by "
> -				     "the matching IRQ save kfunc.");
> +			bpf_diag_res(
> +				env, env->insn_idx, "uninitialized IRQ flag restore",
> +				"Restoring IRQ state requires a stack slot that was initialized by a matching IRQ save operation on this path.",
> +				"Pass the same stack slot that was previously initialized by the matching IRQ save kfunc.");
>  			return err;

Churn.

...