We should update rcu pointer first then use ovsrcu_postpone to free otherwise maybe cause use-after-free. e.g.,reader indicates momentary quiescent and access old pointer after writer postpone free old pointer and before setting new pointer.
Signed-off-by: Linhaifeng <haifeng....@huawei.com> --- lib/classifier.c | 4 ++-- lib/ovs-rcu.h | 4 ++-- lib/pvector.c | 15 ++++++++------- ofproto/ofproto-dpif-mirror.c | 4 ++-- ofproto/ofproto-dpif-upcall.c | 3 +-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/classifier.c b/lib/classifier.c index f2c3497c2..6bff76e07 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -249,11 +249,11 @@ cls_rule_set_conjunctions(struct cls_rule *cr, unsigned int old_n = old ? old->n : 0; if (old_n != n || (n && memcmp(old_conj, conj, n * sizeof *conj))) { + ovsrcu_set(&match->conj_set, + cls_conjunction_set_alloc(match, conj, n)); if (old) { ovsrcu_postpone(free, old); } - ovsrcu_set(&match->conj_set, - cls_conjunction_set_alloc(match, conj, n)); } } diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h index ecc4c9201..98c238aea 100644 --- a/lib/ovs-rcu.h +++ b/lib/ovs-rcu.h @@ -118,10 +118,10 @@ * void * change_flow(struct flow *new_flow) * { + * struct flow *old_flow = ovsrcu_get_protected(struct flow *, &flowp) * ovs_mutex_lock(&mutex); - * ovsrcu_postpone(free, - * ovsrcu_get_protected(struct flow *, &flowp)); * ovsrcu_set(&flowp, new_flow); + * ovsrcu_postpone(free, old_flow); * ovs_mutex_unlock(&mutex); * } * diff --git a/lib/pvector.c b/lib/pvector.c index cc527fdc4..aa8c6cb24 100644 --- a/lib/pvector.c +++ b/lib/pvector.c @@ -67,10 +67,11 @@ pvector_init(struct pvector *pvec) void pvector_destroy(struct pvector *pvec) { + struct pvector_impl *old = pvector_impl_get(pvec); free(pvec->temp); pvec->temp = NULL; - ovsrcu_postpone(free, pvector_impl_get(pvec)); ovsrcu_set(&pvec->impl, NULL); /* Poison. */ + ovsrcu_postpone(free, old); } /* Iterators for callers that need the 'index' afterward. */ @@ -205,11 +206,11 @@ pvector_change_priority(struct pvector *pvec, void *ptr, int priority) /* Make the modified pvector available for iteration. */ void pvector_publish__(struct pvector *pvec) { - struct pvector_impl *temp = pvec->temp; - + struct pvector_impl *new = pvec->temp; + struct pvector_impl *old = ovsrcu_get_protected(struct pvector_impl *, + &pvec->impl); pvec->temp = NULL; - pvector_impl_sort(temp); /* Also removes gaps. */ - ovsrcu_postpone(free, ovsrcu_get_protected(struct pvector_impl *, - &pvec->impl)); - ovsrcu_set(&pvec->impl, temp); + pvector_impl_sort(new); /* Also removes gaps. */ + ovsrcu_set(&pvec->impl, new); + ovsrcu_postpone(free, old); } diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c index 343b75f0e..343100c08 100644 --- a/ofproto/ofproto-dpif-mirror.c +++ b/ofproto/ofproto-dpif-mirror.c @@ -276,9 +276,9 @@ mirror_set(struct mbridge *mbridge, void *aux, const char *name, hmapx_destroy(&dsts_map); if (vlans || src_vlans) { + unsigned long *new_vlans = vlan_bitmap_clone(src_vlans); + ovsrcu_set(&mirror->vlans, new_vlans); ovsrcu_postpone(free, vlans); - vlans = vlan_bitmap_clone(src_vlans); - ovsrcu_set(&mirror->vlans, vlans); } mirror->out = out; diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 5e08ef10d..be6dafb78 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1658,11 +1658,10 @@ ukey_set_actions(struct udpif_key *ukey, const struct ofpbuf *actions) struct ofpbuf *old_actions = ovsrcu_get_protected(struct ofpbuf *, &ukey->actions); + ovsrcu_set(&ukey->actions, ofpbuf_clone(actions)); if (old_actions) { ovsrcu_postpone(ofpbuf_delete, old_actions); } - - ovsrcu_set(&ukey->actions, ofpbuf_clone(actions)); } static struct udpif_key * -- 2.21.0.windows.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev