From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ed Swierk Subject: [PATCH v2] openvswitch: Trim off padding before L3+ netfilter processing Date: Thu, 21 Dec 2017 07:17:17 -0800 Message-ID: <1513869437-20059-1-git-send-email-eswierk@skyportsystems.com> References: <1513095439-128864-1-git-send-email-eswierk@skyportsystems.com> Cc: Ed Swierk , Benjamin Warren , Keith Holleman To: ovs-dev@openvswitch.org, netdev@vger.kernel.org, Pravin Shelar Return-path: Received: from mail-pf0-f196.google.com ([209.85.192.196]:35074 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751143AbdLUPRe (ORCPT ); Thu, 21 Dec 2017 10:17:34 -0500 Received: by mail-pf0-f196.google.com with SMTP id j124so14068776pfc.2 for ; Thu, 21 Dec 2017 07:17:34 -0800 (PST) Sender: netdev-owner@vger.kernel.org List-ID: IPv4 and IPv6 packets may arrive with lower-layer padding that is not included in the L3 length. For example, a short IPv4 packet may have up to 6 bytes of padding following the IP payload when received on an Ethernet device. In the normal IPv4 receive path, ip_rcv() trims the packet to ip_hdr->tot_len before invoking netfilter hooks (including conntrack and nat). In the IPv6 receive path, ip6_rcv() does the same using ipv6_hdr->payload_len. Similarly in the br_netfilter receive path, br_validate_ipv4() and br_validate_ipv6() trim the packet to the L3 length before invoking NF_INET_PRE_ROUTING hooks. In the OVS conntrack receive path, ovs_ct_execute() pulls the skb to the L3 header but does not trim it to the L3 length before calling nf_conntrack_in(NF_INET_PRE_ROUTING). When nf_conntrack_proto_tcp encounters a packet with lower-layer padding, nf_checksum() fails and logs "nf_ct_tcp: bad TCP checksum". While extra zero bytes don't affect the checksum, the length in the IP pseudoheader does. That length is based on skb->len, and without trimming, it doesn't match the length the sender used when computing the checksum. The assumption throughout nf_conntrack and nf_nat is that skb->len reflects the length of the L3 header and payload, so there is no need to refer back to ip_hdr->tot_len or ipv6_hdr->payload_len. This change brings OVS into line with other netfilter users, trimming IPv4 and IPv6 packets prior to L3+ netfilter processing. Signed-off-by: Ed Swierk --- v2: - Trim packet in nat receive path as well as conntrack - Free skb on error --- net/openvswitch/conntrack.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index b27c5c6..1bdc78f 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -703,6 +703,33 @@ static bool skb_nfct_cached(struct net *net, return ct_executed; } +/* Trim the skb to the L3 length. Assumes the skb is already pulled to + * the L3 header. The skb is freed on error. + */ +static int skb_trim_l3(struct sk_buff *skb) +{ + unsigned int nh_len; + int err; + + switch (skb->protocol) { + case htons(ETH_P_IP): + nh_len = ntohs(ip_hdr(skb)->tot_len); + break; + case htons(ETH_P_IPV6): + nh_len = ntohs(ipv6_hdr(skb)->payload_len) + + sizeof(struct ipv6hdr); + break; + default: + nh_len = skb->len; + } + + err = pskb_trim_rcsum(skb, nh_len); + if (err) + kfree_skb(skb); + + return err; +} + #ifdef CONFIG_NF_NAT_NEEDED /* Modelled after nf_nat_ipv[46]_fn(). * range is only used for new, uninitialized NAT state. @@ -715,8 +742,12 @@ static int ovs_ct_nat_execute(struct sk_buff *skb, struct nf_conn *ct, { int hooknum, nh_off, err = NF_ACCEPT; + /* The nat module expects to be working at L3. */ nh_off = skb_network_offset(skb); skb_pull_rcsum(skb, nh_off); + err = skb_trim_l3(skb); + if (err) + return err; /* See HOOK2MANIP(). */ if (maniptype == NF_NAT_MANIP_SRC) @@ -1111,6 +1142,9 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb, /* The conntrack module expects to be working at L3. */ nh_ofs = skb_network_offset(skb); skb_pull_rcsum(skb, nh_ofs); + err = skb_trim_l3(skb); + if (err) + return err; if (key->ip.frag != OVS_FRAG_TYPE_NONE) { err = handle_fragments(net, key, info->zone.id, skb); -- 1.9.1