[ovs-dev] [PATCH] ofproto:fix use-after-free
guohongzhi
guohongzhi1 at huawei.com
Fri Mar 6 13:05:55 UTC 2020
ASAN report use-after-free when destroy ofproto_rule, the rule->ofproto
has freed in ofproto_destroy.
Add ref_count for ofproto to avoid use-after-free when destroy
ofproto_rule adn group.
Signed-off-by: guohongzhi <guohongzhi1 at huawei.com>
---
ofproto/ofproto-dpif.c | 1 +
ofproto/ofproto-provider.h | 3 +++
ofproto/ofproto.c | 31 +++++++++++++++++++++++++++++--
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 7515352..258972d 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1494,6 +1494,7 @@ construct(struct ofproto *ofproto_)
ofproto->ams_seq = seq_create();
ofproto->ams_seqno = seq_read(ofproto->ams_seq);
+ ovs_refcount_init(&ofproto_->ref_count);
SHASH_FOR_EACH_SAFE (node, next, &init_ofp_ports) {
struct iface_hint *iface_hint = node->data;
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 7907d4b..c3e6527 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -139,6 +139,7 @@ struct ofproto {
/* Variable length mf_field mapping. Stores all configured variable length
* meta-flow fields (struct mf_field) in a switch. */
struct vl_mff_map vl_mff_map;
+ struct ovs_refcount ref_count;
};
void ofproto_init_tables(struct ofproto *, int n_tables);
@@ -442,6 +443,8 @@ struct rule {
void ofproto_rule_ref(struct rule *);
bool ofproto_rule_try_ref(struct rule *);
void ofproto_rule_unref(struct rule *);
+void ofproto_ref(struct ofproto *ofproto);
+void ofproto_unref(struct ofproto *ofproto);
static inline const struct rule_actions * rule_get_actions(const struct rule *);
static inline bool rule_is_table_miss(const struct rule *);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 7b84784..dc9caa0 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -554,6 +554,10 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
ovs_mutex_unlock(&ofproto_mutex);
ofproto_destroy__(ofproto);
return error;
+ } else {
+ /* ofproto construct succeed, ref its self
+ * inorder to unref when call ofproto destroy*/
+ ofproto_ref(ofproto);
}
/* Check that hidden tables, if any, are at the end. */
@@ -1673,8 +1677,9 @@ ofproto_destroy(struct ofproto *p, bool del)
p->connmgr = NULL;
ovs_mutex_unlock(&ofproto_mutex);
- /* Destroying rules is deferred, must have 'ofproto' around for them. */
- ovsrcu_postpone(ofproto_destroy_defer__, p);
+ /* Inorder to keep the code logical as before, we ref for ofproto when ofproto create
+ * so, we should also unref ofproto here*/
+ ofproto_unref(p);
}
/* Destroys the datapath with the respective 'name' and 'type'. With the Linux
@@ -2852,6 +2857,22 @@ update_mtu_ofproto(struct ofproto *p)
}
}
+void
+ofproto_ref(struct ofproto *ofproto)
+{
+ if (ofproto) {
+ ovs_refcount_ref(&ofproto->ref_count);
+ }
+}
+
+void
+ofproto_unref(struct ofproto *ofproto)
+{
+ if (ofproto && ovs_refcount_unref_relaxed(&ofproto->ref_count) == 1) {
+ ovsrcu_postpone(ofproto_destroy_defer__, ofproto);
+ }
+}
+
static void
ofproto_rule_destroy__(struct rule *rule)
OVS_NO_THREAD_SAFETY_ANALYSIS
@@ -2860,6 +2881,7 @@ ofproto_rule_destroy__(struct rule *rule)
rule_actions_destroy(rule_get_actions(rule));
ovs_mutex_destroy(&rule->mutex);
rule->ofproto->ofproto_class->rule_dealloc(rule);
+ ofproto_unref(rule->ofproto);
}
static void
@@ -3000,6 +3022,7 @@ group_destroy_cb(struct ofgroup *group)
ofputil_bucket_list_destroy(CONST_CAST(struct ovs_list *,
&group->buckets));
group->ofproto->ofproto_class->group_dealloc(group);
+ ofproto_unref(group->ofproto);
}
void
@@ -5176,6 +5199,7 @@ ofproto_rule_create(struct ofproto *ofproto, struct cls_rule *cr,
/* Initialize base state. */
*CONST_CAST(struct ofproto **, &rule->ofproto) = ofproto;
+ ovs_refcount_ref(ofproto);
cls_rule_move(CONST_CAST(struct cls_rule *, &rule->cr), cr);
ovs_refcount_init(&rule->ref_count);
@@ -7271,6 +7295,9 @@ init_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm,
ofputil_bucket_list_destroy(CONST_CAST(struct ovs_list *,
&(*ofgroup)->buckets));
ofproto->ofproto_class->group_dealloc(*ofgroup);
+ } else {
+ /* group construct succeed, ref ofproto */
+ ofproto_ref(ofproto);
}
return error;
}
--
2.21.0.windows.1
More information about the dev
mailing list