Re: [PATCH bpf-next v6 1/9] bpf: Upgrade scalar to PTR_TO_ARENA on arena pointer addition

Emil Tsalapatis
2026-04-12T16:49:49Z

On Sat Apr 11, 2026 at 9:58 PM EDT, Alexei Starovoitov wrote:
> On Sat, Apr 11, 2026 at 6:19 PM Emil Tsalapatis <emil@etsalapatis.com> wrote:
>>
>> The compiler sometimes stores the result of a PTR_TO_ARENA and SCALAR
>> operation into the scalar register rather than the pointer register.
>> Handle this case by upgrading the destination scalar register to
>> PTR_TO_ARENA, matching the existing handling when the destination is
>> already PTR_TO_ARENA.
>>
>> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> Acked-by: Song Liu <song@kernel.org>
>> Fixes: 6082b6c328b5 ("bpf: Recognize addr_space_cast instruction in the verifier.")
>> ---
>> kernel/bpf/verifier.c | 73 ++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 59 insertions(+), 14 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 9c1135d373e2..1aa60199fa2e 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -16640,6 +16640,59 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
>> return 0;
>> }
>>
>> +/* At least one source instruction register is a PTR_TO_ARENA. */
>> +static int adjust_ptr_to_arena_vals(struct bpf_verifier_env *env,
>> + struct bpf_insn *insn, struct bpf_reg_state *dst_reg,
>> + struct bpf_reg_state *src_reg)
>> +{
>> + struct bpf_insn_aux_data *aux = cur_aux(env);
>> + u8 opcode = BPF_OP(insn->code);
>> +
>> + /*
>> + * If it's an instruction with an imm operand, we know it's valid
>> + * because we checked in the caller if the destination is
>> + * an arena.
>> + */
>> + if (!src_reg)
>> + goto valid;
>> +
>> + /* Ensure both operands is either PTR_TO_ARENA or SCALAR_VALUE. */
>> +
>> + if (dst_reg->type != PTR_TO_ARENA && dst_reg->type != SCALAR_VALUE)
>> + goto error;
>> +
>> + if (src_reg->type != PTR_TO_ARENA && src_reg->type != SCALAR_VALUE)
>> + goto error;
>> +
>> + /* If dst_reg wasn't a PTR_TO_ARENA, it is now. */
>> + if (dst_reg->type != PTR_TO_ARENA)
>> + *dst_reg = *src_reg;
>> +
>> +valid:
>> + dst_reg->subreg_def = env->insn_idx + 1;
>> +
>> + if (BPF_CLASS(insn->code) == BPF_ALU64)
>> + /*
>> + * 32-bit operations zero upper bits automatically.
>> + * 64-bit operations need to be converted to 32.
>> + */
>> + aux->needs_zext = true;
>> +
>> + /* Any arithmetic operations are allowed on arena pointers */
>> + return 0;
>> +
>> +error:
>> + verbose(env, "R%d %s R%d: Invalid operation between "
>> + "bpf_reg_state types %s and %s\n",
>> + insn->dst_reg,
>> + bpf_alu_string[opcode >> 4],
>> + insn->src_reg,
>> + reg_type_str(env, dst_reg->type),
>> + reg_type_str(env, src_reg->type));
>
> You went the wrong direction here.
> The current verifier allows arena += ptr.
> So be it.
> The resulting arena pointer is still safe.
> It's probably meaningless, but the verifier shouldn't judge.
> It's job to prevent crashes and not logical errors.
> So revert to previous code and drop != scalar check
> and fix the actual error, since two reg_type_str() are wrong.
>

Ack, this version is doing too much. I'll drop the check next
version. This also removes the verbose() call so it resolves the
reg_type_str() issue. Since for v6 the only Sashiko comments
left are debatable and/or nits, next version will be a fix of
this patch, adjustments to the patch 2 tests to reflect the
updated patch 1 semantics, and removing an unnecessary header
include from patch 5.

> pw-bot: cr