summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIlya Maximets <i.maximets@ovn.org>2020-06-03 16:58:16 +0200
committerIlya Maximets <i.maximets@ovn.org>2020-06-10 22:40:26 +0200
commit0e4f1a450340885b8800c7ae06e6170d4337f0c3 (patch)
treeb9357161e5750047cecb2f3aef5a422233ee7ef2
parent6ad6ed23ac518711f123f57d6a5b1dbec068f2c8 (diff)
downloadopenvswitch-0e4f1a450340885b8800c7ae06e6170d4337f0c3.tar.gz
ovs-rcu: Avoid flushing callbacks during postponing.
ovsrcu_flush_cbset() call during ovsrcu_postpone() could cause use after free in case the caller sets new pointer only after postponing free for the old one: ------------------ ------------------ ------------------- Thread 1 Thread 2 RCU Thread ------------------ ------------------ ------------------- pointer = A ovsrcu_quiesce(): thread->seqno = 30 global_seqno = 31 quiesced read pointer A postpone(free(A)): flush cbset pop flushed_cbsets ovsrcu_synchronize: target_seqno = 31 ovsrcu_quiesce(): thread->seqno = 31 global_seqno = 32 quiesced read pointer A use pointer A ovsrcu_quiesce(): thread->seqno = 32 global_seqno = 33 quiesced read pointer A pointer = B ovsrcu_quiesce(): thread->seqno = 33 global_seqno = 34 quiesced target_seqno exceeded by all threads call cbs to free A use pointer A (use after free) ----------------------------------------------------------- Fix that by using dynamically re-allocated array without flushing to the global flushed_cbsets until writer enters quiescent state. Fixes: 0f2ea84841e1 ("ovs-rcu: New library.") Reported-by: Linhaifeng <haifeng.lin@huawei.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-June/371265.html Acked-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
-rw-r--r--AUTHORS.rst1
-rw-r--r--lib/ovs-rcu.c17
2 files changed, 13 insertions, 5 deletions
diff --git a/AUTHORS.rst b/AUTHORS.rst
index ea4912f20..91e44f76b 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -442,6 +442,7 @@ Krishna Miriyala krishna@nicira.com
Krishna Mohan Elluru elluru.kri.mohan@hpe.com
László Sürü laszlo.suru@ericsson.com
Len Gao leng@vmware.com
+Linhaifeng haifeng.lin@huawei.com
Logan Rosen logatronico@gmail.com
Luca Falavigna dktrkranz@debian.org
Luiz Henrique Ozaki luiz.ozaki@gmail.com
diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
index 0614d988c..6d9adbe41 100644
--- a/lib/ovs-rcu.c
+++ b/lib/ovs-rcu.c
@@ -29,6 +29,8 @@
VLOG_DEFINE_THIS_MODULE(ovs_rcu);
+#define MIN_CBS 16
+
struct ovsrcu_cb {
void (*function)(void *aux);
void *aux;
@@ -36,7 +38,8 @@ struct ovsrcu_cb {
struct ovsrcu_cbset {
struct ovs_list list_node;
- struct ovsrcu_cb cbs[16];
+ struct ovsrcu_cb *cbs;
+ size_t n_allocated;
int n_cbs;
};
@@ -262,16 +265,19 @@ ovsrcu_postpone__(void (*function)(void *aux), void *aux)
cbset = perthread->cbset;
if (!cbset) {
cbset = perthread->cbset = xmalloc(sizeof *perthread->cbset);
+ cbset->cbs = xmalloc(MIN_CBS * sizeof *cbset->cbs);
+ cbset->n_allocated = MIN_CBS;
cbset->n_cbs = 0;
}
+ if (cbset->n_cbs == cbset->n_allocated) {
+ cbset->cbs = x2nrealloc(cbset->cbs, &cbset->n_allocated,
+ sizeof *cbset->cbs);
+ }
+
cb = &cbset->cbs[cbset->n_cbs++];
cb->function = function;
cb->aux = aux;
-
- if (cbset->n_cbs >= ARRAY_SIZE(cbset->cbs)) {
- ovsrcu_flush_cbset(perthread);
- }
}
static bool
@@ -293,6 +299,7 @@ ovsrcu_call_postponed(void)
for (cb = cbset->cbs; cb < &cbset->cbs[cbset->n_cbs]; cb++) {
cb->function(cb->aux);
}
+ free(cbset->cbs);
free(cbset);
}