netdev - Re: [PATCH net-next v2 4/5] ipv6: sr: Add seg6local action End.BPF

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] [day] [month] [year] [list]

Message-ID: <CAKSCvkTLFVRP0dxEE34tnG4W5kQKBO-m5710-ezXHqPGFP6_ag@mail.gmail.com>
Date:   Mon, 30 Apr 2018 22:06:18 +0200
From:   Mathieu Xhonneux <m.xhonneux@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        David Lebrun <dlebrun@...gle.com>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH net-next v2 4/5] ipv6: sr: Add seg6local action End.BPF

2018-04-28 2:01 GMT+02:00 Alexei Starovoitov <alexei.starovoitov@...il.com>:
>
> On Fri, Apr 27, 2018 at 10:59:19AM -0400, David Miller wrote:
> > From: Mathieu Xhonneux <m.xhonneux@...il.com>
> > Date: Tue, 24 Apr 2018 18:44:15 +0100
> >
> > > This patch adds the End.BPF action to the LWT seg6local infrastructure.
> > > This action works like any other seg6local End action, meaning that an IPv6
> > > header with SRH is needed, whose DA has to be equal to the SID of the
> > > action. It will also advance the SRH to the next segment, the BPF program
> > > does not have to take care of this.
> >
> > I'd like to see some BPF developers review this change.
> >
> > But on my side I wonder if, instead of validating the whole thing afterwards,
> > we should make the helpers accessible by the eBPF program validate the changes
> > as they are made.
>
> Looking at the code I don't think it's possible to keep it valid all the time
> while building, so seg6_validate_srh() after the program run seems necessary.


Indeed, e.g. to add a TLV in the SRH one needs to call
bpf_lwt_seg6_adjust_srh (to add some room for the TLV), then
bpf_lwt_seg6_store_bytes() (to fill the space with the TLV). Between
those two calls, the SRH is in an invalid state.

>
>
> I think the whole set should be targeting bpf-next tree.
> Please fix kbuild errors, rebase and document new helper in man-page style.
> Things like:
> +       test_btf_haskv.o test_btf_nokv.o test_lwt_seg6local.o
> +>>>>>>> selftests/bpf: test for seg6local End.BPF action
> should be fixed properly.


Oops, I didn't catch this one, thanks. I'll send a v3 towards bpf-next.