From 95619d8c3011254a65cdc72d83d0f3638167c264 Mon Sep 17 00:00:00 2001 From: Joe Stringer Date: Wed, 11 Nov 2015 11:39:50 -0800 Subject: [PATCH] ofproto-dpif: Validate ct_* field masks. When inserting rules that match on connection tracking fields, datapath support must be checked before allowing or denying the rule insertion. Previously we only disallowed flows that had non-zero values for the ct_* field, but allowed non-zero masks. This meant that, eg: ct_state=-trk,... Would be allowed, while ct_state=+trk,... Would be disallowed, due to lack of datapath support. Fix this by performing the check on masks instead of the flows. Reported-by: Ravindra Kenchappa Signed-off-by: Joe Stringer Acked-by: Jarno Rajahalme --- ofproto/ofproto-dpif.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 37c5d5da0..8d6865e26 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4015,30 +4015,26 @@ rule_dealloc(struct rule *rule_) static enum ofperr rule_check(struct rule *rule) { + struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->ofproto); + const struct odp_support *support; uint16_t ct_state, ct_zone; ovs_u128 ct_label; uint32_t ct_mark; - ct_state = MINIFLOW_GET_U16(rule->cr.match.flow, ct_state); - ct_zone = MINIFLOW_GET_U16(rule->cr.match.flow, ct_zone); - ct_mark = MINIFLOW_GET_U32(rule->cr.match.flow, ct_mark); - ct_label = MINIFLOW_GET_U128(rule->cr.match.flow, ct_label); + support = &ofproto_dpif_get_support(ofproto)->odp; + ct_state = MINIFLOW_GET_U16(&rule->cr.match.mask->masks, ct_state); + ct_zone = MINIFLOW_GET_U16(&rule->cr.match.mask->masks, ct_zone); + ct_mark = MINIFLOW_GET_U32(&rule->cr.match.mask->masks, ct_mark); + ct_label = MINIFLOW_GET_U128(&rule->cr.match.mask->masks, ct_label); - if (ct_state || ct_zone || ct_mark - || !ovs_u128_is_zero(&ct_label)) { - struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->ofproto); - const struct odp_support *support = &ofproto_dpif_get_support(ofproto)->odp; - - if ((ct_state && !support->ct_state) - || (ct_zone && !support->ct_zone) - || (ct_mark && !support->ct_mark) - || (!ovs_u128_is_zero(&ct_label) && !support->ct_label)) { - return OFPERR_OFPBMC_BAD_FIELD; - } - if (ct_state & CS_UNSUPPORTED_MASK) { - return OFPERR_OFPBMC_BAD_MASK; - } + if ((ct_state && !support->ct_state) + || (ct_state & CS_UNSUPPORTED_MASK) + || (ct_zone && !support->ct_zone) + || (ct_mark && !support->ct_mark) + || (!ovs_u128_is_zero(&ct_label) && !support->ct_label)) { + return OFPERR_OFPBMC_BAD_MASK; } + return 0; } -- 2.20.1