From patchwork Wed Sep 16 04:58:36 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jesse Gross X-Patchwork-Id: 518258 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (li376-54.members.linode.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id C94B8140180 for ; Wed, 16 Sep 2015 14:58:45 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 0087F106E7; Tue, 15 Sep 2015 21:58:45 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e4.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id 70EA8106E2 for ; Tue, 15 Sep 2015 21:58:43 -0700 (PDT) Received: from bar2.cudamail.com (unknown [192.168.21.12]) by mx1e4.cudamail.com (Postfix) with ESMTPS id BDC111E03F3 for ; Tue, 15 Sep 2015 22:58:42 -0600 (MDT) X-ASG-Debug-ID: 1442379522-03dc537fe0c1620001-byXFYA Received: from mx1-pf1.cudamail.com ([192.168.24.1]) by bar2.cudamail.com with ESMTP id ePJFHvcHKI19exXs (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 15 Sep 2015 22:58:42 -0600 (MDT) X-Barracuda-Envelope-From: jesse@nicira.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.1 Received: from unknown (HELO mail-pa0-f54.google.com) (209.85.220.54) by mx1-pf1.cudamail.com with ESMTPS (RC4-SHA encrypted); 16 Sep 2015 04:58:41 -0000 Received-SPF: unknown (mx1-pf1.cudamail.com: Multiple SPF records returned) X-Barracuda-RBL-Trusted-Forwarder: 209.85.220.54 Received: by padhk3 with SMTP id hk3so197367163pad.3 for ; Tue, 15 Sep 2015 21:58:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id; bh=aCoeW1Iv+tmsPWXDElHV6/5fgtYo9ixTkY6bwlLkW8Y=; b=c++AqnUDx09oHscgGUayz1a6nfCYcg61Ta18egkoAAp4Kt14V4VEO1HhI8lCmz4VZc GCn2hsH3WthTmbGGSQ6kJ5/fhlCl9rPSscVGTfLlsD5SYWvGp7Ka6rM9rXfoxuaGu8zn /Gsz1ROU3/Gevb+Jp5O7uSovXP/pSLdMupIG7ccJuyc3M6gDgXZ/v2Q1BKiKdYI7Wiez 9Qg4bU0YUFn3HS6ivS84grnCIEzhKpXAZTIG4wxfKIXDn7IpPNPcFggq++h7rdFJLImL PYIPOUPlqoT4/Jdcu66dnxAaem4f8TIVrY2qRpn7UbKTti0efU7WboDUv6xlCAO8AQnq br3w== X-Gm-Message-State: ALoCoQmRutwT9sVkYn/+DTb6y8QGnlUKJ/eUAyvOnyzULCQvqA1hsFAq/uXkkLSvJw5b1sDWwhxJ X-Received: by 10.68.114.196 with SMTP id ji4mr55828084pbb.46.1442379520518; Tue, 15 Sep 2015 21:58:40 -0700 (PDT) Received: from ubuntu.localdomain (c-71-202-123-143.hsd1.ca.comcast.net. [71.202.123.143]) by smtp.gmail.com with ESMTPSA id fm5sm25071709pbb.60.2015.09.15.21.58.39 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 15 Sep 2015 21:58:39 -0700 (PDT) X-CudaMail-Envelope-Sender: jesse@nicira.com X-Barracuda-Apparent-Source-IP: 71.202.123.143 From: Jesse Gross To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-E1-914118986 X-CudaMail-DTE: 091515 X-CudaMail-Originating-IP: 209.85.220.54 Date: Tue, 15 Sep 2015 21:58:36 -0700 X-ASG-Orig-Subj: [##CM-E1-914118986##][PATCH] datapath: Backport "openvswitch: Fix mask generation for nested attributes." Message-Id: <1442379516-88970-1-git-send-email-jesse@nicira.com> X-Mailer: git-send-email 2.1.4 X-Barracuda-Connect: UNKNOWN[192.168.24.1] X-Barracuda-Start-Time: 1442379522 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Subject: [ovs-dev] [PATCH] datapath: Backport "openvswitch: Fix mask generation for nested attributes." X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" Upstream commit: openvswitch: Fix mask generation for nested attributes. Masks were added to OVS flows in a way that was backwards compatible with userspace programs that did not generate masks. As a result, it is possible that we may receive flows that do not have a mask and we need to synthesize one. Generating a mask requires iterating over attributes and descending into nested attributes. For each level we need to know the size to generate the correct mask. We do this with a linked table of attribute types. Although the logic to handle these nested attributes was there in concept, there are a number of bugs in practice. Examples include incomplete links between tables, variable length attributes being treated as nested and missing sanity checks. Signed-off-by: Jesse Gross Acked-by: Pravin B Shelar Signed-off-by: David S. Miller Upstream: 982b5270 ("openvswitch: Fix mask generation for nested attributes.") Signed-off-by: Jesse Gross Acked-by: Pravin B Shelar --- datapath/flow_netlink.c | 82 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 59 insertions(+), 23 deletions(-) diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c index 6280246..d835a00 100644 --- a/datapath/flow_netlink.c +++ b/datapath/flow_netlink.c @@ -57,6 +57,7 @@ struct ovs_len_tbl { }; #define OVS_ATTR_NESTED -1 +#define OVS_ATTR_VARIABLE -2 static void update_range(struct sw_flow_match *match, size_t offset, size_t size, bool is_mask) @@ -300,6 +301,10 @@ size_t ovs_key_attr_size(void) + nla_total_size(28); /* OVS_KEY_ATTR_ND */ } +static const struct ovs_len_tbl ovs_vxlan_ext_key_lens[OVS_VXLAN_EXT_MAX + 1] = { + [OVS_VXLAN_EXT_GBP] = { .len = sizeof(u32) }, +}; + static const struct ovs_len_tbl ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1] = { [OVS_TUNNEL_KEY_ATTR_ID] = { .len = sizeof(u64) }, [OVS_TUNNEL_KEY_ATTR_IPV4_SRC] = { .len = sizeof(u32) }, @@ -311,8 +316,9 @@ static const struct ovs_len_tbl ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1] [OVS_TUNNEL_KEY_ATTR_TP_SRC] = { .len = sizeof(u16) }, [OVS_TUNNEL_KEY_ATTR_TP_DST] = { .len = sizeof(u16) }, [OVS_TUNNEL_KEY_ATTR_OAM] = { .len = 0 }, - [OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS] = { .len = OVS_ATTR_NESTED }, - [OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS] = { .len = OVS_ATTR_NESTED }, + [OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS] = { .len = OVS_ATTR_VARIABLE }, + [OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS] = { .len = OVS_ATTR_NESTED, + .next = ovs_vxlan_ext_key_lens }, }; /* The size of the argument for each %OVS_KEY_ATTR_* Netlink attribute. */ @@ -341,6 +347,13 @@ static const struct ovs_len_tbl ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = { [OVS_KEY_ATTR_MPLS] = { .len = sizeof(struct ovs_key_mpls) }, }; +static bool check_attr_len(unsigned int attr_len, unsigned int expected_len) +{ + return expected_len == attr_len || + expected_len == OVS_ATTR_NESTED || + expected_len == OVS_ATTR_VARIABLE; +} + static bool is_all_zero(const u8 *fp, size_t size) { int i; @@ -380,7 +393,7 @@ static int __parse_flow_nlattrs(const struct nlattr *attr, } expected_len = ovs_key_lens[type].len; - if (nla_len(nla) != expected_len && expected_len != OVS_ATTR_NESTED) { + if (!check_attr_len(nla_len(nla), expected_len)) { OVS_NLERR(log, "Key %d has unexpected len %d expected %d", type, nla_len(nla), expected_len); return -EINVAL; @@ -465,29 +478,50 @@ static int genev_tun_opt_from_nlattr(const struct nlattr *a, return 0; } -static const struct nla_policy vxlan_opt_policy[OVS_VXLAN_EXT_MAX + 1] = { - [OVS_VXLAN_EXT_GBP] = { .type = NLA_U32 }, -}; - -static int vxlan_tun_opt_from_nlattr(const struct nlattr *a, +static int vxlan_tun_opt_from_nlattr(const struct nlattr *attr, struct sw_flow_match *match, bool is_mask, bool log) { - struct nlattr *tb[OVS_VXLAN_EXT_MAX+1]; + struct nlattr *a; + int rem; unsigned long opt_key_offset; struct ovs_vxlan_opts opts; - int err; BUILD_BUG_ON(sizeof(opts) > sizeof(match->key->tun_opts)); - err = nla_parse_nested(tb, OVS_VXLAN_EXT_MAX, a, vxlan_opt_policy); - if (err < 0) - return err; - memset(&opts, 0, sizeof(opts)); + nla_for_each_nested(a, attr, rem) { + int type = nla_type(a); - if (tb[OVS_VXLAN_EXT_GBP]) - opts.gbp = nla_get_u32(tb[OVS_VXLAN_EXT_GBP]); + if (type > OVS_VXLAN_EXT_MAX) { + OVS_NLERR(log, "VXLAN extension %d out of range max %d", + type, OVS_VXLAN_EXT_MAX); + return -EINVAL; + } + + if (!check_attr_len(nla_len(a), + ovs_vxlan_ext_key_lens[type].len)) { + OVS_NLERR(log, "VXLAN extension %d has unexpected len %d expected %d", + type, nla_len(a), + ovs_vxlan_ext_key_lens[type].len); + return -EINVAL; + } + + switch (type) { + case OVS_VXLAN_EXT_GBP: + opts.gbp = nla_get_u32(a); + break; + default: + OVS_NLERR(log, "Unknown VXLAN extension attribute %d", + type); + return -EINVAL; + } + } + if (rem) { + OVS_NLERR(log, "VXLAN extension message has %d unknown bytes.", + rem); + return -EINVAL; + } if (!is_mask) SW_FLOW_KEY_PUT(match, tun_opts_len, sizeof(opts), false); @@ -520,8 +554,8 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr, return -EINVAL; } - if (ovs_tunnel_key_lens[type].len != nla_len(a) && - ovs_tunnel_key_lens[type].len != OVS_ATTR_NESTED) { + if (!check_attr_len(nla_len(a), + ovs_tunnel_key_lens[type].len)) { OVS_NLERR(log, "Tunnel attr %d has unexpected len %d expected %d", type, nla_len(a), ovs_tunnel_key_lens[type].len); return -EINVAL; @@ -1011,10 +1045,13 @@ static void nlattr_set(struct nlattr *attr, u8 val, /* The nlattr stream should already have been validated */ nla_for_each_nested(nla, attr, rem) { - if (tbl && tbl[nla_type(nla)].len == OVS_ATTR_NESTED) - nlattr_set(nla, val, tbl[nla_type(nla)].next); - else + if (tbl[nla_type(nla)].len == OVS_ATTR_NESTED) { + if (tbl[nla_type(nla)].next) + tbl = tbl[nla_type(nla)].next; + nlattr_set(nla, val, tbl); + } else { memset(nla_data(nla), val, nla_len(nla)); + } } } @@ -1841,8 +1878,7 @@ static int validate_set(const struct nlattr *a, key_len /= 2; if (key_type > OVS_KEY_ATTR_MAX || - (ovs_key_lens[key_type].len != key_len && - ovs_key_lens[key_type].len != OVS_ATTR_NESTED)) + !check_attr_len(key_len, ovs_key_lens[key_type].len)) return -EINVAL; if (masked && !validate_masked(nla_data(ovs_key), key_len))