[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3551c6f1-61d4-6f2d-885f-9f84131179e9@iogearbox.net>
Date: Wed, 22 Aug 2018 09:22:57 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Petar Penkov <ppenkov@...gle.com>,
Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: 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 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.
Thanks,
Daniel
Powered by blists - more mailing lists