From aee532378943ad5990b99857f832d532fe702237 Mon Sep 17 00:00:00 2001 From: Joe Stringer Date: Fri, 31 Oct 2014 14:05:46 -0700 Subject: [PATCH] odp-util: Fix segfault in MPLS attribute parsing. Just because the ethertype is MPLS, this doesn't mean that the datapath understands and provides OVS_KEY_ATTR_MPLS attributes for the flow. Previously we would check the size of the OVS_KEY_ATTR_MPLS attribute before checking whether the attribute is present. This would cause a segfault in nl_attr_get_size(), usually triggered from a handler thread. This patch brings the MPLS parsing code more in line with the rest of the parse_l2_5_onward() function, by only processing MPLS if the attribute is present. Reported-by: Pravin B Shelar Signed-off-by: Joe Stringer Acked-by: Ben Pfaff --- lib/odp-util.c | 57 +++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/lib/odp-util.c b/lib/odp-util.c index 6c5ec3c23..8b33ec8f1 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -2969,47 +2969,42 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], enum ovs_key_attr expected_bit = 0xff; if (eth_type_mpls(src_flow->dl_type)) { - size_t size = nl_attr_get_size(attrs[OVS_KEY_ATTR_MPLS]); - const ovs_be32 *mpls_lse = nl_attr_get(attrs[OVS_KEY_ATTR_MPLS]); - int n = size / sizeof(ovs_be32); - int i; - - if (!size || size % sizeof(ovs_be32)) { - return ODP_FIT_ERROR; - } - - if (!is_mask) { + if (!is_mask || present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_MPLS)) { expected_attrs |= (UINT64_C(1) << OVS_KEY_ATTR_MPLS); + } + if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_MPLS)) { + size_t size = nl_attr_get_size(attrs[OVS_KEY_ATTR_MPLS]); + const ovs_be32 *mpls_lse = nl_attr_get(attrs[OVS_KEY_ATTR_MPLS]); + int n = size / sizeof(ovs_be32); + int i; - if (!(present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_MPLS))) { - return ODP_FIT_TOO_LITTLE; + if (!size || size % sizeof(ovs_be32)) { + return ODP_FIT_ERROR; } - } else if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_MPLS)) { if (flow->mpls_lse[0] && flow->dl_type != htons(0xffff)) { return ODP_FIT_ERROR; } - expected_attrs |= (UINT64_C(1) << OVS_KEY_ATTR_MPLS); - } - for (i = 0; i < n && i < FLOW_MAX_MPLS_LABELS; i++) { - flow->mpls_lse[i] = mpls_lse[i]; - } - if (n > FLOW_MAX_MPLS_LABELS) { - return ODP_FIT_TOO_MUCH; - } + for (i = 0; i < n && i < FLOW_MAX_MPLS_LABELS; i++) { + flow->mpls_lse[i] = mpls_lse[i]; + } + if (n > FLOW_MAX_MPLS_LABELS) { + return ODP_FIT_TOO_MUCH; + } - if (!is_mask) { - /* BOS may be set only in the innermost label. */ - for (i = 0; i < n - 1; i++) { - if (flow->mpls_lse[i] & htonl(MPLS_BOS_MASK)) { - return ODP_FIT_ERROR; + if (!is_mask) { + /* BOS may be set only in the innermost label. */ + for (i = 0; i < n - 1; i++) { + if (flow->mpls_lse[i] & htonl(MPLS_BOS_MASK)) { + return ODP_FIT_ERROR; + } } - } - /* BOS must be set in the innermost label. */ - if (n < FLOW_MAX_MPLS_LABELS - && !(flow->mpls_lse[n - 1] & htonl(MPLS_BOS_MASK))) { - return ODP_FIT_TOO_LITTLE; + /* BOS must be set in the innermost label. */ + if (n < FLOW_MAX_MPLS_LABELS + && !(flow->mpls_lse[n - 1] & htonl(MPLS_BOS_MASK))) { + return ODP_FIT_TOO_LITTLE; + } } } -- 2.20.1