Re: [PATCH bpf-next v7 1/9] bpf: mark instructions accessing program stack

["Alexis Lothoré" <alexis.lothore@bootlin.com>]

On Mon Aug 24, 2026 at 12:33 AM CEST, Kumar Kartikeya Dwivedi wrote:
> On Sat Aug 22, 2026 at 1:24 AM CEST, bot+bpf-ci wrote:
>>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>>> index 004b06785521..412a13a664fa 100644
>>> --- a/include/linux/bpf_verifier.h
>>> +++ b/include/linux/bpf_verifier.h
>>> @@ -725,6 +725,8 @@ struct bpf_insn_aux_data {
>>>  	u16 const_reg_map_mask;
>>>  	u16 const_reg_subprog_mask;
>>>  	u32 const_reg_vals[10];
>>> +	/* instruction can access non-stack memory */
>>> +	bool non_stack_access;
>>>  };
>>
>> [ ... ]
>>
>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>> index e036ae20bf6b..040af75d9550 100644
>>> --- a/kernel/bpf/verifier.c
>>> +++ b/kernel/bpf/verifier.c
>>> @@ -3272,6 +3272,11 @@ static void mark_indirect_target(struct bpf_verifier_env *env, int idx)
>>>  	env->insn_aux_data[idx].indirect_target = true;
>>>  }
>>>
>>> +static void mark_non_stack_access(struct bpf_verifier_env *env, int idx)
>>> +{
>>> +	env->insn_aux_data[idx].non_stack_access = true;
>>> +}
>>> +
>>
>> [ ... ]
>>
>>> @@ -6654,6 +6659,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
>>>  				clear_scalar_id(&regs[value_regno]);
>>>  		}
>>>  	}
>>> +
>>> +	if (!err && reg->type != PTR_TO_STACK)
>>> +		mark_non_stack_access(env, insn_idx);
>>> +
>>>  	return err;
>>>  }
>>
>> Does this test read the correct register type? For a load instruction where
>> dst_reg == src_reg (e.g., rX = *(u64 *)(rX + off)), check_mem_access()
>> receives reg = &regs[src_reg] and value_regno = dst_reg, so reg points to
>> the same slot that gets overwritten by the load.
>>
>> When the pointer is PTR_TO_STACK, check_stack_read() runs:
>>
>> check_stack_read_fixed_off() {
>>     state->regs[dst_regno] = *reg;  /* or mark_reg_stack_read() */
>> }
>>
>> By the time execution reaches the new test, reg->type is the loaded value's
>> type (typically SCALAR_VALUE), not PTR_TO_STACK. The condition evaluates to
>> true and marks a stack-only access as non-stack.
>>
>> Both check_load_mem() and check_store_reg() snapshot the pointer type before
>> calling check_mem_access():
>>
>>     /* verifier.c check_load_mem */
>>     src_reg_type = regs[insn->src_reg].type;
>>     err = check_mem_access(env, env->insn_idx, ...);
>>
>>     /* verifier.c check_store_reg */
>>     dst_reg_type = regs[insn->dst_reg].type;
>>     err = check_mem_access(env, env->insn_idx, ...);
>>
>> Could check_mem_access() snapshot reg->type at entry and test that snapshot
>> instead?
>
> This one also looks like a real bug, we should probably save the type before the
> call as suggested and done elsewhere.

It took me a bit of time to fully grasp it, but indeed, that's an actual
issue, possibly over-instrumenting programs. I failed to reproduce it
with any existing selftest, and also failed to make clang output a
faulty program just from C, but playing with a bit of BPF assembly, I
managed to observe it:

  SEC("tcx/ingress")
  int ldx_self_alias_on_stack(struct __sk_buff *skb)
  {
  	struct kasan_test_val val;
  	__u64 addr;
  
  	addr = (__u64)&val;
  	asm volatile(
  	        "r1 = %0\n"
  	        "r1 = *(u64 *)(r1 + 0)\n"
  	        :
  	        : "r"(addr)
  	        : "r1", "memory");
  
  	return 0;
  }

This r1 = *(u64 *)(r1) ends up being wrongly instrumented, despite
accessing only stack. I'll then add this reg->type saving to preserve
and pass the initial state to the check.

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com