From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH net-next 1/2] net_sched: don't do precise pkt_len computation for untrusted packets Date: Wed, 20 Mar 2013 14:19:28 +0800 Message-ID: <514954F0.2040604@redhat.com> References: <1363333305-54398-1-git-send-email-jasowang@redhat.com> <20130317.121026.733329595817911776.davem@davemloft.net> <51482F14.9030407@redhat.com> <1363695040.21184.32.camel@edumazet-glaptop> <1363697906.21184.53.camel@edumazet-glaptop> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: David Miller , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, mst@redhat.com, edumazet@google.com, Daniel Borkmann To: Eric Dumazet Return-path: In-Reply-To: <1363697906.21184.53.camel@edumazet-glaptop> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 03/19/2013 08:58 PM, Eric Dumazet wrote: > On Tue, 2013-03-19 at 05:10 -0700, Eric Dumazet wrote: >> On Tue, 2013-03-19 at 17:25 +0800, Jason Wang wrote: >> >>> I believe before doing header check for untrusted packets, the only >>> thing we can trust is skb->len and that's we've used before >>> 1def9238d4aa2. But after that, we're trying to use unchecked or >>> meaningless value (e.g gso_segs were reset to zero in >>> tun/macvtap/packet), and guest then can utilize this to result a very >>> huge (-1U) pkt_len by filling evil value in the header. Can all kinds of >>> packet scheduler survive this kinds of possible DOS? >> I would use the flow dissector to fix the transport header from all >> DODGY providers. >> >> Daniel Borkmann is working on a patch serie adding nhoff into flow_keys, >> and adding __skb_get_poff(const struct sk_buff *skb), for a BPF >> extension we talked about in Copenhagen / Netfilter Workshop. >> >> You could then set the transport header offset to the right value. >> >> (and drop evil packets before they go further in the stack) >> >> if (gso_packet(skb)) { >> u32 poff = __skb_get_poff(skb); >> >> if (!poff) { >> drop_evil_packet(skb); >> } else { >> skb_set_transport_header(skb, poff); >> ... >> } >> } > > Oh well, no need to use __skb_get_poff() but plain skb_flow_dissect() > (once patched to include thoff in struct flow_keys) > > struct flow_keys keys; > > if (!skb_flow_dissect(skb, &keys)) > goto drop; > > if ((gso_type & (SKB_GSO_TCPV4|SKB_GSO_TCPV6)) && > keys.ip_proto != IP_PROTO_TCP) > goto drop; > > skb_set_transport_header(skb, keys.thoff); I was consider a just skb_reset_transport_header() here. Consider the transport header maybe checked and reset during header check for packets of gso or partial checksum. And bypass precise pkt len computation. Some problems with skb_flow_dissect(): - it can only recognizes a subset of all ethernet protocols. The may blocks guest who may want to use other protocol such as IPX. - almost no check in the validity of the L4 protocol header which may be used by qdisc_pkt_len_init(), which may still give a chance to evil guest to use - gso_segs were untouched (still zero) Another method is doing header check here which needs more work. > > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html