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

["Emil Tsalapatis" <emil@etsalapatis.com>]

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