netdev - Re: [bpf-next RFC 0/3] Introduce eBPF flow dissector

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: <CAG4SDVXU8kT_ZX17GkZb8BVBEHGRSR9vBe+jE_EavkBzjOeECQ@mail.gmail.com>
Date:   Wed, 22 Aug 2018 17:10:03 -0700
From:   Petar Penkov <ppenkov@...gle.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Petar Penkov <peterpenkov96@...il.com>,
        Networking <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        simon.horman@...ronome.com, Willem de Bruijn <willemb@...gle.com>
Subject: Re: [bpf-next RFC 0/3] Introduce eBPF flow dissector

On Wed, Aug 22, 2018 at 12:28 AM, Daniel Borkmann <daniel@...earbox.net> wrote:
> "On 08/22/2018 09:22 AM, Daniel Borkmann wrote:
>> On 08/22/2018 02:19 AM, Petar Penkov wrote:
>>> On Mon, Aug 20, 2018 at 1:52 PM, Alexei Starovoitov
>>> <alexei.starovoitov@...il.com> wrote:
>>>> On Thu, Aug 16, 2018 at 09:44:20AM -0700, Petar Penkov wrote:
>>>>> From: Petar Penkov <ppenkov@...gle.com>
>> [...]
>>>>> 3/ The BPF program cannot use direct packet access everywhere because it
>>>>> uses an offset, initially supplied by the flow dissector.  Because the
>>>>> initial value of this non-constant offset comes from outside of the
>>>>> program, the verifier does not know what its value is, and it cannot verify
>>>>> that it is within packet bounds. Therefore, direct packet access programs
>>>>> get rejected.
>>>>
>>>> this part doesn't seem to match the code.
>>>> direct packet access is allowed and usable even for fragmented skbs.
>>>> in such case only linear part of skb is in "direct access".
>>>
>>> I am not sure I understand. What I meant was that I use bpf_skb_load_bytes
>>> rather than direct packet access because the offset at which I read headers,
>>> nhoff, depends on an initial value that cannot be statically verified - namely
>>> what __skb_flow_dissect provides. Is there an alternative approach I should
>>> be taking here, and/or am I misunderstanding direct access?
>>
>> You can still use direct packet access with it, the only thing you would
>> need to make sure is that the initial offset is bounded (e.g. test if
>> larger than some const and then drop the packet, or '& <const>') so that
>> the verifier can make sure the alu op won't cause overflow, then you can
>> add this to pkt_data, and later on open an access range with the usual test
>> like pkt_data' + <const> > pkt_end.
>
> And for non-linear data, you could use the bpf_skb_pull_data() helper as
> we have in tc/BPF case 36bbef52c7eb ("bpf: direct packet write and access
> for helpers for clsact progs") to pull it into linear area and make it
> accessible for direct packet access.
>
>> Thanks,
>> Daniel

Thanks for the clarification! With direct packet access the flow
dissector in patch 2
is as fast as the in-kernel flow dissector when tested with the test in patch 3.

To bound the initial offset and use direct access I check if the
initial offset is larger
than 1500. This is sufficient for the verifier but I was wondering if there is a
better constant to use.

Thanks once again for your feedback,
Petar