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>

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 3f7eee54f4572b6e2efac4c2f92f6c1bd6d65312..7a3b12610807df4f6335e08026b90c960ca1135c 100644 (file)
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -563,6 +563,7 @@ Krishna Miriyala                miriyalak@vmware.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 ebc8120f0fd33812c02f7c9a7e3bc6fb47d59c95..cde1e925ba9481ee281564591a23023b14f37a14 100644 (file)
--- a/lib/ovs-rcu.c
+++ b/lib/ovs-rcu.c
@@ -30,6 +30,8 @@
 
 VLOG_DEFINE_THIS_MODULE(ovs_rcu);
 
+#define MIN_CBS 16
+
 struct ovsrcu_cb {
     void (*function)(void *aux);
     void *aux;
@@ -37,7 +39,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;
 };
 
@@ -310,16 +313,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
@@ -341,6 +347,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);
     }