• Yonghong Song's avatar
    bpf, testing: Workaround a verifier failure for test_progs · b7a0d65d
    Yonghong Song authored
    With latest llvm compiler, running test_progs will have the following
    verifier failure for test_sysctl_loop1.o:
    
      libbpf: load bpf program failed: Permission denied
      libbpf: -- BEGIN DUMP LOG ---
      libbpf:
      invalid indirect read from stack var_off (0x0; 0xff)+196 size 7
      ...
      libbpf: -- END LOG --
      libbpf: failed to load program 'cgroup/sysctl'
      libbpf: failed to load object 'test_sysctl_loop1.o'
    
    The related bytecode looks as below:
    
      0000000000000308 LBB0_8:
          97:       r4 = r10
          98:       r4 += -288
          99:       r4 += r7
         100:       w8 &= 255
         101:       r1 = r10
         102:       r1 += -488
         103:       r1 += r8
         104:       r2 = 7
         105:       r3 = 0
         106:       call 106
         107:       w1 = w0
         108:       w1 += -1
         109:       if w1 > 6 goto -24 <LBB0_5>
         110:       w0 += w8
         111:       r7 += 8
         112:       w8 = w0
         113:       if r7 != 224 goto -17 <LBB0_8>
    
    And source code:
    
         for (i = 0; i < ARRAY_SIZE(tcp_mem); ++i) {
                 ret = bpf_strtoul(value + off, MAX_ULONG_STR_LEN, 0,
                                   tcp_mem + i);
                 if (ret <= 0 || ret > MAX_ULONG_STR_LEN)
                         return 0;
                 off += ret & MAX_ULONG_STR_LEN;
         }
    
    Current verifier is not able to conclude that register w0 before '+'
    at insn 110 has a range of 1 to 7 and thinks it is from 0 - 255. This
    leads to more conservative range for w8 at insn 112, and later verifier
    complaint.
    
    Let us workaround this issue until we found a compiler and/or verifier
    solution. The workaround in this patch is to make variable 'ret' volatile,
    which will force a reload and then '&' operation to ensure better value
    range. With this patch, I got the below byte code for the loop:
    
      0000000000000328 LBB0_9:
         101:       r4 = r10
         102:       r4 += -288
         103:       r4 += r7
         104:       w8 &= 255
         105:       r1 = r10
         106:       r1 += -488
         107:       r1 += r8
         108:       r2 = 7
         109:       r3 = 0
         110:       call 106
         111:       *(u32 *)(r10 - 64) = r0
         112:       r1 = *(u32 *)(r10 - 64)
         113:       if w1 s< 1 goto -28 <LBB0_5>
         114:       r1 = *(u32 *)(r10 - 64)
         115:       if w1 s> 7 goto -30 <LBB0_5>
         116:       r1 = *(u32 *)(r10 - 64)
         117:       w1 &= 7
         118:       w1 += w8
         119:       r7 += 8
         120:       w8 = w1
         121:       if r7 != 224 goto -21 <LBB0_9>
    
    Insn 117 did the '&' operation and we got more precise value range
    for 'w8' at insn 120. The test is happy then:
    
      #3/17 test_sysctl_loop1.o:OK
    Signed-off-by: default avatarYonghong Song <yhs@fb.com>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Acked-by: default avatarSong Liu <songliubraving@fb.com>
    Link: https://lore.kernel.org/bpf/20191107170045.2503480-1-yhs@fb.com
    b7a0d65d
test_sysctl_loop1.c 1.79 KB