On 19/08/17 00:37, Alexei Starovoitov wrote:
> that '14: safe' above is not correct.
>
> Disabling liveness as:
> @@ -3282,7 +3288,7 @@ static bool regsafe(struct bpf_reg_state *rold,
> struct bpf_reg_state *rcur,
> bool varlen_map_access, struct idpair *idmap)
> {
> - if (!(rold->live & REG_LIVE_READ))
> + if (0 && !(rold->live & REG_LIVE_READ))
>
> makes the test work properly and proper verifier output is:
> from 9 to 11: R0=map_value(id=0,off=0,ks=8,vs=48,imm=0)
> R1=inv(id=0,smax_value=10) R2=inv11 R10=fp0
> 11: (64) (u32) r1 <<= (u32) 2
> 12: (0f) r0 += r1
> 13: (05) goto pc+0
> 14: (7a) *(u64 *)(r0 +0) = 4
>
> R0=map_value(id=0,off=0,ks=8,vs=48,umax_value=17179869180,var_off=(0x0;
> 0x3fffffffc)) R1=inv(id=0,umax_value=17179869180,var_off=(0x0; 0x3fffffffc))
> R2=inv11 R10=fp0
> R0 unbounded memory access, make sure to bounds check any array access into a
> map
>
> I don't yet understand the underlying reason. R0 should have been
> marked as LIVE_READ by ST_MEM...
> Please help debug it further.
>
Having added a bunch of debugging, I found out that indeed R0 _had_ been
marked as LIVE_READ. The problem was that env->varlen_map_value_access
wasn't set, because the access was at a constant offset (imm=0), but then
when we compare register states we just say "oh yeah, it's a map_value,
we don't need to look at the var_off".
This probably results from my unifying PTR_TO_MAP_VALUE with
PTR_TO_MAP_VALUE_ADJ; before that the old and new R0 would have different
reg->type so wouldn't match.
I'm tempted to just rip out env->varlen_map_value_access and always check
the whole thing, because honestly I don't know what it was meant to do
originally or how it can ever do any useful pruning. While drastic, it
does cause your test case to pass.
I'm not quite sure why your test passed when you disabled liveness, though;
that I can't explain.
-Ed
_______________________________________________
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev
