From: Eli Britstein <[email protected]>
A group has a hash_map that contains the array of its buckets.
Revalidator threads access it to check if some bucket is alive, while
the main thread may modify the group, re-allocating this array. This race
condition may lead to invalid memory access.
Introduce a rw-lock to take a write-lock while modifying a group and a
read-lock for revalidators.
Fixes: 0a8f6beb54ab ("ofproto-dpif: Fix dp_hash mapping after select group
modification.")
Signed-off-by: Eli Britstein <[email protected]>
Acked-by: Roi Dayan <[email protected]>
---
ofproto/ofproto-dpif-xlate.c | 3 +++
ofproto/ofproto-dpif.c | 9 +++++++++
ofproto/ofproto-dpif.h | 1 +
3 files changed, 13 insertions(+)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2c8197fb7307..945506bdf86a 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4998,13 +4998,16 @@ pick_dp_hash_select_group(struct xlate_ctx *ctx, struct
group_dpif *group)
* are quasi-randomly spread over the hash values, this maintains
* a distribution according to bucket weights even when some buckets
* are non-live. */
+ ovs_rwlock_rdlock(&group->hash_map_rwlock);
for (int i = 0; i <= hash_mask; i++) {
struct ofputil_bucket *b =
group->hash_map[(dp_hash + i) & hash_mask];
if (bucket_is_alive(ctx, group, b, 0)) {
+ ovs_rwlock_unlock(&group->hash_map_rwlock);
return b;
}
}
+ ovs_rwlock_unlock(&group->hash_map_rwlock);
return NULL;
}
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ed9e44ce2b03..e8b38d1e5355 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5301,6 +5301,7 @@ group_dpif_credit_stats(struct group_dpif *group,
static bool
group_setup_dp_hash_table(struct group_dpif *group, size_t max_hash)
+ OVS_REQ_WRLOCK(group->hash_map_rwlock)
{
struct ofputil_bucket *bucket;
uint32_t n_buckets = group->up.n_buckets;
@@ -5388,6 +5389,7 @@ group_setup_dp_hash_table(struct group_dpif *group,
size_t max_hash)
static void
group_set_selection_method(struct group_dpif *group)
+ OVS_REQ_WRLOCK(group->hash_map_rwlock)
{
const struct ofputil_group_props *props = &group->up.props;
const char *selection_method = props->selection_method;
@@ -5456,11 +5458,14 @@ group_construct(struct ofgroup *group_)
ovs_mutex_init_adaptive(&group->stats_mutex);
ovs_mutex_lock(&group->stats_mutex);
group_construct_stats(group);
+ ovs_rwlock_init(&group->hash_map_rwlock);
+ ovs_rwlock_wrlock(&group->hash_map_rwlock);
group->hash_map = NULL;
if (group->up.type == OFPGT11_SELECT) {
VLOG_DBG("Constructing select group %"PRIu32, group->up.group_id);
group_set_selection_method(group);
}
+ ovs_rwlock_unlock(&group->hash_map_rwlock);
ovs_mutex_unlock(&group->stats_mutex);
return 0;
}
@@ -5469,7 +5474,9 @@ static void
group_destruct(struct ofgroup *group_)
{
struct group_dpif *group = group_dpif_cast(group_);
+
ovs_mutex_destroy(&group->stats_mutex);
+ ovs_rwlock_destroy(&group->hash_map_rwlock);
if (group->hash_map) {
free(group->hash_map);
group->hash_map = NULL;
@@ -5481,6 +5488,7 @@ group_modify(struct ofgroup *group_)
{
struct group_dpif *group = group_dpif_cast(group_);
+ ovs_rwlock_wrlock(&group->hash_map_rwlock);
if (group->hash_map) {
free(group->hash_map);
group->hash_map = NULL;
@@ -5489,6 +5497,7 @@ group_modify(struct ofgroup *group_)
VLOG_DBG("Modifying select group %"PRIu32, group->up.group_id);
group_set_selection_method(group);
}
+ ovs_rwlock_unlock(&group->hash_map_rwlock);
}
static enum ofperr
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index f8d3df5ab5a6..3a0f4e0a29ed 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -142,6 +142,7 @@ struct group_dpif {
enum ovs_hash_alg hash_alg; /* dp_hash algorithm to be applied. */
uint32_t hash_basis; /* Basis for dp_hash. */
uint32_t hash_mask; /* Used to mask dp_hash (2^N - 1).*/
+ struct ovs_rwlock hash_map_rwlock;
struct ofputil_bucket **hash_map; /* Map hash values to buckets. */
};
--
2.47.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
