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 <je...@nicira.com>
---
datapath/datapath.c | 11 ++++++++---
datapath/flow.c | 12 ++++--------
datapath/flow.h | 5 +++--
3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index eb07a89..8c9b7f7 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,7 +1279,9 @@ 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);
+ ovs_flow_key_mask(&masked_key, &key, &mask);
+ error = validate_and_copy_actions(a[OVS_FLOW_ATTR_ACTIONS],
+ &masked_key, 0, &acts);
if (error)
goto err_kfree;
} else if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW) {
@@ -1324,6 +1326,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 +1346,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 be69186..752c8d6 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 a31dab0..1a3764e 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 */
--
1.8.1.2
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
