On Wed, Apr 25, 2018 at 12:29:03PM -0700, Yonghong Song wrote: > The special property of return values for helpers bpf_get_stack > and bpf_probe_read_str are captured in verifier. > Both helpers return a negative error code or > a length, which is equal to or smaller than the buffer > size argument. This additional information in the > verifier can avoid the condition such as "retval > bufsize" > in the bpf program. For example, for the code blow, > usize = bpf_get_stack(ctx, raw_data, max_len, BPF_F_USER_STACK); > if (usize < 0 || usize > max_len) > return 0; > The verifier may have the following errors: > 52: (85) call bpf_get_stack#65 > R0=map_value(id=0,off=0,ks=4,vs=1600,imm=0) R1_w=ctx(id=0,off=0,imm=0) > R2_w=map_value(id=0,off=0,ks=4,vs=1600,imm=0) R3_w=inv800 R4_w=inv256 > R6=ctx(id=0,off=0,imm=0) R7=map_value(id=0,off=0,ks=4,vs=1600,imm=0) > R9_w=inv800 R10=fp0,call_-1 > 53: (bf) r8 = r0 > 54: (bf) r1 = r8 > 55: (67) r1 <<= 32 > 56: (bf) r2 = r1 > 57: (77) r2 >>= 32 > 58: (25) if r2 > 0x31f goto pc+33 > R0=inv(id=0) R1=inv(id=0,smax_value=9223372032559808512, > umax_value=18446744069414584320, > var_off=(0x0; 0xffffffff00000000)) > R2=inv(id=0,umax_value=799,var_off=(0x0; 0x3ff)) > R6=ctx(id=0,off=0,imm=0) R7=map_value(id=0,off=0,ks=4,vs=1600,imm=0) > R8=inv(id=0) R9=inv800 R10=fp0,call_-1 > 59: (1f) r9 -= r8 > 60: (c7) r1 s>>= 32 > 61: (bf) r2 = r7 > 62: (0f) r2 += r1 > math between map_value pointer and register with unbounded > min value is not allowed > The failure is due to llvm compiler optimization where register "r2", > which is a copy of "r1", is tested for condition while later on "r1" > is used for map_ptr operation. The verifier is not able to track such > inst sequence effectively. > > Without the "usize > max_len" condition, there is no llvm optimization > and the below generated code passed verifier: > 52: (85) call bpf_get_stack#65 > R0=map_value(id=0,off=0,ks=4,vs=1600,imm=0) R1_w=ctx(id=0,off=0,imm=0) > R2_w=map_value(id=0,off=0,ks=4,vs=1600,imm=0) R3_w=inv800 R4_w=inv256 > R6=ctx(id=0,off=0,imm=0) R7=map_value(id=0,off=0,ks=4,vs=1600,imm=0) > R9_w=inv800 R10=fp0,call_-1 > 53: (b7) r1 = 0 > 54: (bf) r8 = r0 > 55: (67) r8 <<= 32 > 56: (c7) r8 s>>= 32 > 57: (6d) if r1 s> r8 goto pc+24 > R0=inv(id=0,umax_value=800,var_off=(0x0; 0x3ff)) > R1=inv0 R6=ctx(id=0,off=0,imm=0) > R7=map_value(id=0,off=0,ks=4,vs=1600,imm=0) > R8=inv(id=0,umax_value=800,var_off=(0x0; 0x3ff)) R9=inv800 > R10=fp0,call_-1 > 58: (bf) r2 = r7 > 59: (0f) r2 += r8 > 60: (1f) r9 -= r8 > 61: (bf) r1 = r6 > > Signed-off-by: Yonghong Song <y...@fb.com>
Acked-by: Alexei Starovoitov <a...@kernel.org>