Re: Re: [PATCH bpf-next 1/2] bpf: Reject scalar addition from untrusted memory

[Nuoqi Gui]

> -----Original Messages-----
> From: "Emil Tsalapatis" <emil@xxxxxxxxxxxxxxx>
> Send time:Wednesday, 10/06/2026 02:21:49
> To: "Eduard Zingerman" <eddyz87@xxxxxxxxx>, "Nuoqi Gui" <gnq25@xxxxxxxxxxxxxxxxxxxxx>, "Alexei Starovoitov" <ast@xxxxxxxxxx>, "Daniel Borkmann" <daniel@xxxxxxxxxxxxx>, "Andrii Nakryiko" <andrii@xxxxxxxxxx>
> Cc: "Kumar Kartikeya Dwivedi" <memxor@xxxxxxxxx>, "John Fastabend" <john.fastabend@xxxxxxxxx>, "Martin KaFai Lau" <martin.lau@xxxxxxxxx>, "Song Liu" <song@xxxxxxxxxx>, "Yonghong Song" <yonghong.song@xxxxxxxxx>, "Jiri Olsa" <jolsa@xxxxxxxxxx>, "Shuah Khan" <shuah@xxxxxxxxxx>, bpf@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-kselftest@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH bpf-next 1/2] bpf: Reject scalar addition from untrusted memory
> 
> On Tue Jun 9, 2026 at 1:01 PM EDT, Eduard Zingerman wrote:
> > On Tue, 2026-06-09 at 22:55 +0800, Nuoqi Gui wrote:
> >> scalar += rdonly_untrusted_mem reaches adjust_ptr_min_max_vals() with the
> >> pointer as the source register. The untrusted PTR_TO_MEM case returns there
> >> without updating the scalar destination, leaving stale verifier state.
> >> 
> >> Reject that addition before the early return. Pointer += scalar remains
> >> handled by the existing untrusted-memory rule.
> >> 
> >> Fixes: f2362a57aeff ("bpf: allow void* cast using bpf_rdonly_cast()")
> >> Signed-off-by: Nuoqi Gui <gnq25@xxxxxxxxxxxxxxxxxxxxx>
> >> ---
> >>  kernel/bpf/verifier.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >> 
> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >> index c8d980fdd709..c6b350f9585a 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -14823,6 +14823,14 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
> >>  				 * This is legal, but we have to reverse our
> >>  				 * src/dest handling in computing the range
> >>  				 */
> >> +				if (opcode == BPF_ADD &&
> >> +				    base_type(src_reg->type) == PTR_TO_MEM &&
> >> +				    (src_reg->type & PTR_UNTRUSTED)) {
> >> +					verbose(env, "R%d tried to add from %s to scalar\n",
> >> +						insn->dst_reg,
> >> +						reg_type_str(env, src_reg->type));
> >> +					return -EACCES;
> >> +				}
> >>  				err = mark_chain_precision(env, insn->dst_reg);
> >>  				if (err)
> >>  					return err;
> >
> > Should the fix be like this:
> >
> >   diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >   index 7d27ba396d32..9c85dd680a46 100644
> >   --- a/kernel/bpf/verifier.c
> >   +++ b/kernel/bpf/verifier.c
> >   @@ -13593,8 +13593,10 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> >            * Accesses to untrusted PTR_TO_MEM are done through probe
> >            * instructions, hence no need to track offsets.
> >            */
> >   -       if (base_type(ptr_reg->type) == PTR_TO_MEM && (ptr_reg->type & PTR_UNTRUSTED))
> >   +       if (base_type(ptr_reg->type) == PTR_TO_MEM && (ptr_reg->type & PTR_UNTRUSTED)) {
> >   +               *dst_reg = *ptr_reg;
> >                   return 0;
> >   +       }
> >    
> >           switch (base_type(ptr_reg->type)) {
> >           case PTR_TO_CTX:
> >
> > Instead?
> 
> Seconded, AFAICT there is no reason to fail verification since we just
> the pointer's value to the scalar. Whether the operation makes sense is
> up to the programmer to decide.

Thanks.

Agreed, rejecting scalar += pointer is not the right semantic fix. The bug is
that the untrusted PTR_TO_MEM early return leaves dst state stale.

I'll send v2 moving the fix into adjust_ptr_min_max_vals().