[DTrace-devel] BPF verifier question

[Kris Van Hees]

On Tue, Aug 25, 2020 at 10:35:43PM -0700, Eugene Loh wrote:
> I am getting a BPF verifier error I do not understand.  Is the verifier 
> wrong?
> 
> Consider:
> 
>      BEGIN { x = curcpu; }
> 
> I can run "dtrace -Se".  The germane output is:
> 
>      lddw %r1, [%fp-8]
>      lddw %r1, [%r1+8]
>      mov  %r2, 289==0x121=DIF_VAR_CURCPU
>      call dt_get_bvar       ! curcpu
> 
>      mov  %r8, %r0
>      mov  %r2, %r8
>      mov  %r1, 0
>      call dt_set_gvar       ! x
> 
> That is, we look up curcpu and store it in x.
> 
> But when we try loading the program, the BPF verifier complains. It says 
> the return value from dt_get_bvar() is
> 
>      R0=map_value_or_null(id=2,off=0,ks=4,vs=24,imm=0)
> 
> which gets copied to %r8 and then to %r2 and into dt_set_gvar(). Lemme 
> see, bpf/set_gvar.c has:
> 
>      dt_set_gvar(uint32_t id, uint64_t val)
>      {
>          bpf_map_update_elem(&gvars, &id, &val, BPF_ANY);
>      }
> 
> and the BPF verifier says:
> 
>      instruction                          state
> 
>                                           R1_w=invP0
> R2_w=map_value_or_null
>                                           R10=fp0
> 
>      *(u32 *)(r10 -4) = r1                fp-8=0000????
>      *(u64 *)(r10 -16) = r2 fp-16_w=map_value_or_null
>      r4 = 0                               R4_w=invP0
>      r3 = r10-16                          R3_w=fp-16
>      r2 = r10-4                           R2_w=fp-4
>      r1 = 0xffff8f8f7ebc7a00              R1_w=map_ptr
>      call bpf_map_update_elem#2
> 
>      BPF: invalid indirect read from stack off -16+0 size 8
> 
> Why?  I would think that r3==fp-16 points to some location that can be 
> dereferenced (we just set its contents to a map_value_or_null), and so 
> all must be good.  Who is right?  The verifier or me?  (I'm not insulted 
> if I'm wrong;  I'd just like to understand.)  In the language of the 
> dt_set_gvar() C source code, I'm passing in a map_value_or_null.  I put 
> it somewhere (r10-16) and pass that address to bpf_map_update_elem().  
> The 8 bytes at r10-16 are addressable.  So, I should be good.

The problem is that dt_get_bvar() is retunring a map_value_or_null which
is a signal to the verifier that we have not made sure that the value is
in fact not NULL (which would indicate that the map value was no found in
the map).  THe BPF verifier requires that whenever we retrieve a map value
from a map, we do a if (val == NULL) ... test and then the type will change
to map_value rather than map_value_or_null.