From: Jesse Gross Date: Tue, 16 Jul 2013 04:30:59 +0000 (-0700) Subject: datapath: Use masked flow when validating actions. X-Git-Tag: sliver-openvswitch-2.0.90-1~36^2~21 X-Git-Url: http://git.onelab.eu/?p=sliver-openvswitch.git;a=commitdiff_plain;h=529db6351a720e3525059370a7ebd2420582395f datapath: Use masked flow when validating actions. It is important to validate flow actions to ensure that they do not try to write off the end of a packet. The mechanism to do this is to ensure that a flow is precise enough to describe valid vs. invalid packets and only allowing actions on valid flows. The introduction of megaflows broke this by using a narrow base flow but a potentially wide match. This meant that while the original flow was properly validated, later packets might not conform to that flow and could be truncated. This switches to using the masked flow instead, effectively requiring that all possible matching packets be valid in order for a flow's actions to be accepted. This change only affects the flow setup path - executed packets have always used the flow extracted from the packet and therefore were properly validated. Signed-off-by: Jesse Gross --- diff --git a/datapath/datapath.c b/datapath/datapath.c index eb07a8909..1de50c19b 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -1251,7 +1251,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info) { struct nlattr **a = info->attrs; struct ovs_header *ovs_header = info->userhdr; - struct sw_flow_key key; + struct sw_flow_key key, masked_key; struct sw_flow *flow = NULL; struct sw_flow_mask mask; struct sk_buff *reply; @@ -1279,9 +1279,13 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info) if (IS_ERR(acts)) goto error; - error = validate_and_copy_actions(a[OVS_FLOW_ATTR_ACTIONS], &key, 0, &acts); - if (error) + ovs_flow_key_mask(&masked_key, &key, &mask); + error = validate_and_copy_actions(a[OVS_FLOW_ATTR_ACTIONS], + &masked_key, 0, &acts); + if (error) { + OVS_NLERR("Flow actions may not be safe on all matching packets.\n"); goto err_kfree; + } } else if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW) { error = -EINVAL; goto error; @@ -1324,6 +1328,9 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info) } clear_stats(flow); + flow->key = masked_key; + flow->unmasked_key = key; + /* Make sure mask is unique in the system */ mask_p = ovs_sw_flow_mask_find(table, &mask); if (!mask_p) { @@ -1341,7 +1348,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info) rcu_assign_pointer(flow->sf_acts, acts); /* Put flow in bucket. */ - ovs_flow_insert(table, flow, &key, match.range.end); + ovs_flow_insert(table, flow); reply = ovs_flow_cmd_build_info(flow, dp, info->snd_portid, info->snd_seq, OVS_FLOW_CMD_NEW); diff --git a/datapath/flow.c b/datapath/flow.c index be69186db..752c8d6b3 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -349,9 +349,8 @@ static bool icmp6hdr_ok(struct sk_buff *skb) sizeof(struct icmp6hdr)); } -static void flow_key_mask(struct sw_flow_key *dst, - const struct sw_flow_key *src, - const struct sw_flow_mask *mask) +void ovs_flow_key_mask(struct sw_flow_key *dst, const struct sw_flow_key *src, + const struct sw_flow_mask *mask) { u8 *m = (u8 *)&mask->key + mask->range.start; u8 *s = (u8 *)src + mask->range.start; @@ -1045,7 +1044,7 @@ static struct sw_flow *ovs_masked_flow_lookup(struct flow_table *table, u32 hash; struct sw_flow_key masked_key; - flow_key_mask(&masked_key, flow_key, mask); + ovs_flow_key_mask(&masked_key, flow_key, mask); hash = ovs_flow_hash(&masked_key, key_start, key_len); head = find_bucket(table, hash); hlist_for_each_entry_rcu(flow, head, hash_node[table->node_ver]) { @@ -1071,11 +1070,8 @@ struct sw_flow *ovs_flow_lookup(struct flow_table *tbl, } -void ovs_flow_insert(struct flow_table *table, struct sw_flow *flow, - const struct sw_flow_key *key, int key_len) +void ovs_flow_insert(struct flow_table *table, struct sw_flow *flow) { - flow->unmasked_key = *key; - flow_key_mask(&flow->key, &flow->unmasked_key, ovsl_dereference(flow->mask)); flow->hash = ovs_flow_hash(&flow->key, ovsl_dereference(flow->mask)->range.start, ovsl_dereference(flow->mask)->range.end); diff --git a/datapath/flow.h b/datapath/flow.h index a31dab0d4..1a3764e8c 100644 --- a/datapath/flow.h +++ b/datapath/flow.h @@ -216,9 +216,8 @@ void ovs_flow_tbl_destroy(struct flow_table *table, bool deferred); struct flow_table *ovs_flow_tbl_alloc(int new_size); struct flow_table *ovs_flow_tbl_expand(struct flow_table *table); struct flow_table *ovs_flow_tbl_rehash(struct flow_table *table); -void ovs_flow_insert(struct flow_table *table, struct sw_flow *flow, - const struct sw_flow_key *key, int key_len); +void ovs_flow_insert(struct flow_table *table, struct sw_flow *flow); void ovs_flow_remove(struct flow_table *table, struct sw_flow *flow); struct sw_flow *ovs_flow_dump_next(struct flow_table *table, u32 *bucket, u32 *idx); @@ -258,4 +257,6 @@ void ovs_sw_flow_mask_del_ref(struct sw_flow_mask *, bool deferred); void ovs_sw_flow_mask_insert(struct flow_table *, struct sw_flow_mask *); struct sw_flow_mask *ovs_sw_flow_mask_find(const struct flow_table *, const struct sw_flow_mask *); +void ovs_flow_key_mask(struct sw_flow_key *dst, const struct sw_flow_key *src, + const struct sw_flow_mask *mask); #endif /* flow.h */