On 2018年01月19日 22:39, Willem de Bruijn wrote:
On Fri, Jan 19, 2018 at 3:19 AM, Jason Wang <jasow...@redhat.com> wrote:
On 2018年01月19日 08:53, Willem de Bruijn wrote:
And what you propose here is just a very small subset of the
necessary checking, more comes at gso header checking. So even if we
care
performance, it only help for some specific case.
It also fixed the bug that Eric sent a separate patch for, as that did
not dissect as a valid TCP packet, either.
I may miss something but how did this patch protects an evil thoff?
Actually, it blocked that specific reproducer because the ip protocol
did not match.
I see.
I think that __skb_flow_dissect_tcp should return a boolean, causing
dissection return FLOW_DISSECT_RET_OUT_BAD if the tcph is bad.
That would be needed to really catch it with flow dissection at the
source.
Just sanitize transport to offset_hint (0) in the case of tun.
That is the current approach in skb_probe_transport_header, but it
opens us up to parsing of garbage headers later in the stack when
unconditionally reading tcp_hdrlen. The qdisc_pkt_len_init case is one
such example. Seems better to leave transport header unset in such
cases and qualify all access to the headers if DODGY.
It looks to
me flow dissector will return FLOW_DISSECT_RET_OUT_BAD too if it can't
recognize the protocol. We can't differ the real failure from unrecognized
protocol. (or change the return from bool to int).
Unrecognized protocol should imply failure. virtio_net_hdr_to_skb accepts
a whitelist of protocols, all of which the flow dissector can verify.
Yes and only for gso packets.
Another argument for early validation: we cannot currently differentiate
tunnel headers inserted by the tun/pf_packet/xen user from those
generated by the stack if both are present.
The kernel currently does not define tunnel VIRTIO_NET_HDR_GSO
types, so should drop packets with tunnel headers. Tunnel gso handlers
indeed do drop these if skb->encapsulation is not set.
But if a packet travels through a tunnel device and the bit is set, at
segmentation time we cannot distinguish trusted stack headers from
possibly malformed user supplied ones.
Right.
Thanks
