netdev - Re: [PATCH net-next] bpf: fix verifier issue at check_packet_ptr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170203034636.GA68213@ast-mbp.thefacebook.com>
Date:   Thu, 2 Feb 2017 19:46:38 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     William Tu <u9012063@...il.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] bpf: fix verifier issue at check_packet_ptr_add

On Thu, Feb 02, 2017 at 07:26:44PM -0800, William Tu wrote:
> Thanks. below is my program. The verifier fails at line 272, when
> writing to ICMP header.
> -----
> ; ebpf_packetEnd = ((void*)(long)skb->data_end);
>      206:       r2 = *(u32 *)(r6 + 4)
> ; ebpf_packetStart = ((void*)(long)skb->data);
>      207:       r1 = *(u32 *)(r6 + 0)
> ...
> r10 is "struct hd" at local stack
> r1 is skb->data
> ...
> ; if (hd.icmp.ebpf_valid) {
>      261:       r4 = *(u64 *)(r10 - 40)
>      262:       if r4 == 0 goto 29
> ; if (ebpf_packetEnd < ebpf_packetStart + BYTES(ebpf_packetOffsetInBits + 32)) {
>      263:       r4 = r0
>      264:       r4 += 32
>      265:       r4 >>= 3
>      266:       r5 = r1
>      267:       r5 += r4
>      268:       if r5 > r2 goto 23
> ; write_byte(ebpf_packetStart, BYTES(ebpf_packetOffsetInBits) + 0,
> (ebpf_byte) << 0);
>      269:       r2 = r0
>      270:       r2 >>= 3
>      271:       r4 = r1
>      272:       r4 += r2
>      273:       r2 = *(u64 *)(r10 - 80)
>      274:       *(u8 *)(r4 + 0) = r2
> 
> verifier log
> ========
> from 208 to 260: R0=inv,min_value=0,max_value=0 R1=pkt(id=0,off=0,r=0)
> R2=pkt_end R3=fp-12 R6=ctx R7=imm0,min_value=0,max_value=0
> R8=inv,min_value=0,max_value=0 R9=inv R10=fp
> 260: (bf) r0 = r7
> 261: (79) r4 = *(u64 *)(r10 -40)
> 262: (15) if r4 == 0x0 goto pc+29
>  R0=imm0,min_value=0,max_value=0 R1=pkt(id=0,off=0,r=0) R2=pkt_end
> R3=fp-12 R4=inv R6=ctx R7=imm0,min_value=0,max_value=0
> R8=inv,min_value=0,max_value=0 R9=inv R10=fp
> 263: (bf) r4 = r0
> 264: (07) r4 += 32
> 265: (77) r4 >>= 3
> 266: (bf) r5 = r1
> 267: (0f) r5 += r4
> 268: (2d) if r5 > r2 goto pc+23
>  R0=imm0,min_value=0,max_value=0 R1=pkt(id=0,off=0,r=4) R2=pkt_end
> R3=fp-12 R4=imm4,min_value=4,max_value=4 R5=pkt(id=0,off=4,r=4) R6=ctx
> R7=imm0,min_value=0,max_value=0 R8=inv,min_value=0,max_value=0 R9=inv
> R10=fp
> 269: (bf) r2 = r0
> 270: (77) r2 >>= 3
> 271: (bf) r4 = r1
> 272: (0f) r4 += r2
> 273: (79) r2 = *(u64 *)(r10 -80)
> 274: (73) *(u8 *)(r4 +0) = r2
> 
> ---
> The full C source code, llvm-objdump, and verifier log.
> https://gist.github.com/williamtu/abaeb11563872508d47cfabed23ac9ea
> https://gist.github.com/williamtu/3e308a14c5795c82d516f934be0560cc
> https://gist.github.com/williamtu/1ae888cea019d065eab3057f74d38905#file-gistfile1-txt

thanks for sharing.
the C program looks auto-generated? Just curious what did you
use to do it?

The line 272 is r4 += r2
where R4=imm4 and R2=pkt_end
Right now verifier doesn't accept any arithmetic with pkt_end,
since it expects the programs to have cannonical form of
if (ptr > pkt_end)
  goto fail;

Even if we add it, I'm not sure what 'pkt_end + 4' suppose to do.
It's a pointer after the valid packet range.

I'm the most puzzled with the diff:
-               if (imm <= 0) {
+               if (imm < 0) {
how is it making the program to pass verifier?

PS
gentle reminder to avoid top posting.