Re: [PATCH bpf-next v4 05/16] bpf: Track verifier register diagnostic events

[Eduard Zingerman <eddyz87@gmail.com>]

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

Claude ensures me that BPF_ATOMIC | BPF_FETCH lacks the mod_end() call
for when the stack memory is modified. Seem to be true.

...

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

...

> @@ -30,15 +30,23 @@
>  #define BPF_DIAG_CONTEXT_CNT (1 + BPF_DIAG_CONTEXT * 2)
>  #define BPF_DIAG_SOURCE_LANE_WIDTH 88
>  #define BPF_DIAG_TAB_WIDTH 8
> -#define BPF_DIAG_REG_DESC_LEN 512
> -#define BPF_DIAG_REG_TMP_LEN 192

Churn

...

> @@ -49,6 +57,13 @@ struct bpf_diag_history_event {
>  		struct {
>  			bool cond_true;
>  		} branch;
> +		struct {
> +			struct bpf_diag_mod_target target;
> +			struct bpf_diag_mod_target origin;

Silly question, should we keep the 'src'/'dst' terminology here?
In order to be in line with instruction set.

> +			struct bpf_diag_reg_snapshot old, new;
> +			u8 reason;
> +			bool origin_valid;
> +		} mod;
>  	};
>  };

...

> @@ -375,6 +401,34 @@ static void diag_print_wrapped_prefixed(struct bpf_verifier_env *env, const char
>  	}
>  }
>  
> +static void bpf_diag_format_btf_type(char *buf, size_t size, const struct btf *btf, u32 type_id)
> +{

Nit: single caller, might as well inline.

> +	size_t len;
> +	int ret;
> +
> +	buf[0] = '\0';
> +	ret = btf_type_snprintf_show_name(btf, type_id, buf, size);
> +	if (ret < 0 || !buf[0]) {
> +		scnprintf(buf, size, "BTF type ID %u", type_id);
> +		return;
> +	}
> +
> +	len = strlen(buf);
> +	if (len && buf[len - 1] == '{')
> +		buf[len - 1] = '\0';
> +}
> +
> +const char *bpf_diag_fmt_btf_type(struct bpf_verifier_env *env, const struct btf *btf, u32 type_id)
> +{
> +	char *buf = bpf_diag_fmt_buf(env, BPF_DIAG_FMT_BUF_SIZE);
> +
> +	if (!buf)
> +		return "";
> +
> +	bpf_diag_format_btf_type(buf, BPF_DIAG_FMT_BUF_SIZE, btf, type_id);
> +	return buf;
> +}

...

> +static bool diag_snapshot_eq(const struct bpf_diag_reg_snapshot *old,
> +			     const struct bpf_diag_reg_snapshot *new)
> +{

Nit: memcmp?

> +	return old->type == new->type && old->map_ptr == new->map_ptr && old->btf == new->btf &&
> +	       old->btf_id == new->btf_id && old->var_off.value == new->var_off.value &&
> +	       old->var_off.mask == new->var_off.mask && old->r64.base == new->r64.base &&
> +	       old->r64.size == new->r64.size;
> +}

...

> +static struct bpf_func_state *diag_func_state(struct bpf_verifier_env *env, u32 frameno)
> +{
> +	struct bpf_verifier_state *vstate = env->cur_state;
> +	int frame;
> +
> +	for (frame = 0; frame <= vstate->curframe; frame++) {
> +		if (vstate->frame[frame]->frameno == frameno)
> +			return vstate->frame[frame];
> +	}

Isn't this just `return frameno <= vstate->curframe ? vstate->frame[frameno] : NULL`?

> +	return NULL;
> +}
> +

...

> diff --git a/kernel/bpf/diagnostics.h b/kernel/bpf/diagnostics.h

...

--- 8< --------------------------------------

> +enum bpf_diag_mod_target_kind { ...
> +struct bpf_diag_mod_target { ...
> +static inline struct bpf_diag_mod_target bpf_diag_reg_target(u32 frameno, u8 regno) ...
> +static inline struct bpf_diag_mod_target bpf_diag_stack_arg_target(u32 frameno, u8 slot) ...
> +static inline struct bpf_diag_mod_target bpf_diag_stack_slot_target(u32 frameno, u16 spi) ...
> +static inline struct bpf_diag_mod_target bpf_diag_stack_range_target(u32 frameno, s16 min_off, ...

-------------------------------------- >8 ---

Move this to diagnostics.c? (and drop prefixes).

...

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c

...

> @@ -8902,13 +8936,18 @@ static int check_func_proto(const struct bpf_func_proto *fn, struct bpf_call_arg
>   */
>  static void clear_all_pkt_pointers(struct bpf_verifier_env *env)
>  {
> +	struct bpf_stack_state *stack;
>  	struct bpf_func_state *state;
>  	struct bpf_reg_state *reg;
>  
> -	bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({
> -		if (reg_is_pkt_pointer_any(reg) || reg_is_dynptr_slice_pkt(reg))
> -			mark_reg_invalid(env, reg);
> -	}));
> +	bpf_for_each_reg_in_vstate_mask(
> +		env->cur_state, state, reg, stack, 1 << STACK_SPILL, ({
> +			if (reg_is_pkt_pointer_any(reg) || reg_is_dynptr_slice_pkt(reg)) {
> +				bpf_diag_record_scrub(env, reg, BPF_DIAG_MOD_PKT_DATA_CHANGE);
> +				mark_reg_invalid(env, reg);
> +			}
> +		}))
> +		;
>  }

Nit: bpf_for_each_reg_in_vstate_mask conversion is not necessary.