netdev - call for bpf progs. Re: [PATCHv2 bpf-next 5/5] selftests: bpf: add test for sk

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: <20200326210719.den5isqxntnoqhmv@ast-mbp>
Date:   Thu, 26 Mar 2020 14:07:19 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Lorenz Bauer <lmb@...udflare.com>
Cc:     Yonghong Song <yhs@...com>, Joe Stringer <joe@...d.net.nz>,
        bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Martin Lau <kafai@...com>
Subject: call for bpf progs. Re: [PATCHv2 bpf-next 5/5] selftests: bpf: add
 test for sk_assign

On Thu, Mar 26, 2020 at 10:13:31AM +0000, Lorenz Bauer wrote:
> > > +
> > > +     if (ipv4) {
> > > +             if (tuple->ipv4.dport != bpf_htons(4321))
> > > +                     return TC_ACT_OK;
> > > +
> > > +             ln.ipv4.daddr = bpf_htonl(0x7f000001);
> > > +             ln.ipv4.dport = bpf_htons(1234);
> > > +
> > > +             sk = bpf_skc_lookup_tcp(skb, &ln, sizeof(ln.ipv4),
> > > +                                     BPF_F_CURRENT_NETNS, 0);
> > > +     } else {
> > > +             if (tuple->ipv6.dport != bpf_htons(4321))
> > > +                     return TC_ACT_OK;
> > > +
> > > +             /* Upper parts of daddr are already zero. */
> > > +             ln.ipv6.daddr[3] = bpf_htonl(0x1);
> > > +             ln.ipv6.dport = bpf_htons(1234);
> > > +
> > > +             sk = bpf_skc_lookup_tcp(skb, &ln, sizeof(ln.ipv6),
> > > +                                     BPF_F_CURRENT_NETNS, 0);
> > > +     }
> > > +
> > > +     /* We can't do a single skc_lookup_tcp here, because then the compiler
> > > +      * will likely spill tuple_len to the stack. This makes it lose all
> > > +      * bounds information in the verifier, which then rejects the call as
> > > +      * unsafe.
> > > +      */
> >
> > This is a known issue. For scalars, only constant is restored properly
> > in verifier at this moment. I did some hacking before to enable any
> > scalars. The fear is this will make pruning performs worse. More
> > study is needed here.
> 
> Of topic, but: this is actually one of the most challenging issues for
> us when writing
> BPF. It forces us to have very deep call graphs to hopefully avoid clang
> spilling the constants. Please let me know if I can help in any way.

Thanks for bringing this up.
Yonghong, please correct me if I'm wrong.
I think you've experimented with tracking spilled constants. The first issue
came with spilling of 4 byte constant. The verifier tracks 8 byte slots and
lots of places assume that slot granularity. It's not clear yet how to refactor
the verifier. Ideas, help are greatly appreciated.
The second concern was pruning, but iirc the experiments were inconclusive.
selftests/bpf only has old fb progs. Hence, I think, the step zero is for
everyone to contribute their bpf programs written in C. If we have both
cilium and cloudflare progs as selftests it will help a lot to guide such long
lasting verifier decisions.