diff mbox series

[ovs-dev,v1,8/8] ovs-rcu: Add blocking RCU mode

Message ID 54575393370fa6076ba6ce7201f36bbfecbe9039.1619564350.git.grive@u256.net
State Superseded
Headers show
Series RCU: Add blocking mode for debugging | expand

Commit Message

Gaetan Rivet April 27, 2021, 11:03 p.m. UTC
Add the configure option --enable-rcu-blocking, that modifies the RCU
library. When enabled, quiescing from other threads will block, waiting
on the RCU thread to execute the postponed jobs.

This mode forces the deferred memory reclamation to happen
deterministically, reducing the latency of the deferral and forcing memory
to be freed any time a thread goes through a quiescent state.

Some use-after-free that were hidden by deferred memory reclamation may
become observable as a result. Previously the RCU mechanism would make them
harder to detect.

UAF detection tools should then be used in conjunction with this
compilation flag, e.g. (assuming llvm installed):

  ./configure --enable-rcu-blocking --enable-asan
  make

  # Verify the tool works: should trigger a UAF
  ./tests/ovstest test-rcu-uaf quiesce
  ./tests/ovstest test-rcu-uaf try-quiesce
  ./tests/ovstest test-rcu-uaf quiesce-start-end

  # The testsuite can be used as well
  make check TESTSUITEFLAGS='-k rcu'

Signed-off-by: Gaetan Rivet <grive@u256.net>
---
 .ci/linux-build.sh                   |  4 ++
 .github/workflows/build-and-test.yml |  7 +++
 NEWS                                 |  1 +
 acinclude.m4                         | 15 +++++
 configure.ac                         |  1 +
 lib/ovs-rcu.c                        | 82 ++++++++++++++++++++++++++++
 tests/atlocal.in                     |  1 +
 tests/library.at                     |  3 +
 8 files changed, 114 insertions(+)
diff mbox series

Patch

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 3c58637b4..e4cbe2024 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -235,6 +235,10 @@  if [ "$ASAN" ]; then
     CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -O1"
 fi
 
+if [ "$RCU_BLOCK" ]; then
+    EXTRA_OPTS="$EXTRA_OPTS --enable-rcu-blocking"
+fi
+
 save_OPTS="${OPTS} $*"
 OPTS="${EXTRA_OPTS} ${save_OPTS}"
 
diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml
index ce98a9f98..655923325 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -23,6 +23,7 @@  jobs:
       M32:         ${{ matrix.m32 }}
       OPTS:        ${{ matrix.opts }}
       TESTSUITE:   ${{ matrix.testsuite }}
+      RCU_BLOCK:   ${{ matrix.rcu_blocking }}
 
     name: linux ${{ join(matrix.*, ' ') }}
     runs-on: ubuntu-18.04
@@ -109,6 +110,12 @@  jobs:
           - compiler:     gcc
             deb_package:  deb
 
+          - compiler:     clang
+            testsuite:    test
+            kernel:       3.16
+            asan:         asan
+            rcu_blocking: rcu-blocking
+
     steps:
     - name: checkout
       uses: actions/checkout@v2
diff --git a/NEWS b/NEWS
index 57e1f041b..83fcfe1d0 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,7 @@  Post-v2.15.0
        in ovsdb on startup.
      * New command 'record-hostname-if-not-set' to update hostname in ovsdb.
    - New --enable-asan configure option enables AddressSanitizer.
+   - New --enable-rcu-blocking configure option to debug RCU usage.
 
 
 v2.15.0 - 15 Feb 2021
diff --git a/acinclude.m4 b/acinclude.m4
index 615e7f962..b01264373 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -1386,6 +1386,21 @@  AC_DEFUN([OVS_ENABLE_SPARSE],
      [], [enable_sparse=no])
    AM_CONDITIONAL([ENABLE_SPARSE_BY_DEFAULT], [test $enable_sparse = yes])])
 
+dnl OVS_ENABLE_RCU_BLOCKING
+AC_DEFUN([OVS_ENABLE_RCU_BLOCKING],
+  [AC_ARG_ENABLE(
+    [rcu-blocking],
+    [AC_HELP_STRING([--enable-rcu-blocking],
+                    [Enable the blocking RCU mode])],
+    [RCU_BLOCKING=yes], [RCU_BLOCKING=no])
+   AC_SUBST([RCU_BLOCKING])
+   AC_CONFIG_COMMANDS_PRE([
+     if test "$RCU_BLOCKING" = "yes"; then
+         OVS_CFLAGS="$OVS_CFLAGS -DOVS_RCU_BLOCKING=1"
+     fi
+   ])
+  ])
+
 dnl OVS_CTAGS_IDENTIFIERS
 dnl
 dnl ctags ignores symbols with extras identifiers. This is a list of
diff --git a/configure.ac b/configure.ac
index eec5a9d1b..de11ff777 100644
--- a/configure.ac
+++ b/configure.ac
@@ -184,6 +184,7 @@  OVS_CHECK_CC_OPTION([-mavx512f], [CFLAGS="$CFLAGS -DHAVE_AVX512F"])
 OVS_ENABLE_WERROR
 OVS_ENABLE_ASAN
 OVS_ENABLE_SPARSE
+OVS_ENABLE_RCU_BLOCKING
 OVS_CTAGS_IDENTIFIERS
 OVS_CHECK_DPCLS_AUTOVALIDATOR
 OVS_CHECK_BINUTILS_AVX512
diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
index 1866bd308..cd8414973 100644
--- a/lib/ovs-rcu.c
+++ b/lib/ovs-rcu.c
@@ -71,6 +71,79 @@  static void ovsrcu_unregister__(struct ovsrcu_perthread *);
 static bool ovsrcu_call_postponed(void);
 static void *ovsrcu_postpone_thread(void *arg OVS_UNUSED);
 
+#ifdef OVS_RCU_BLOCKING
+
+static struct seq *postpone_wait;
+DEFINE_STATIC_PER_THREAD_DATA(bool, need_wait, false);
+DEFINE_STATIC_PER_THREAD_DATA(uint64_t, quiescent_seqno, 0);
+
+static void
+ovsrcu_postpone_end(void)
+{
+    if (single_threaded()) {
+        return;
+    }
+    seq_change(postpone_wait);
+}
+
+static bool
+ovsrcu_do_not_block(void)
+{
+    /* Do not wait on the postpone thread if it has been cleared for exit. */
+    return single_threaded() ||
+           (strncmp(get_subprogram_name(), "urcu", strlen("urcu")) == 0) ||
+           latch_is_set(&postpone_exit);
+}
+
+static void
+ovsrcu_postpone_wait_enable(bool seq_locked)
+{
+    if (ovsrcu_do_not_block()) {
+        return;
+    }
+
+    *quiescent_seqno_get() = seq_locked ?
+                             seq_read_protected(postpone_wait) :
+                             seq_read(postpone_wait);
+    *need_wait_get() = true;
+}
+
+static void
+ovsrcu_postpone_wait(void)
+{
+    struct ovsrcu_perthread *perthread;
+
+    if (ovsrcu_do_not_block() || !*need_wait_get()) {
+        return;
+    }
+
+    *need_wait_get() = false;
+
+    /* Unregistering the potential current perthread ensures
+     * this thread is seen as quiescent.
+     *
+     * It is safe to call, as any path taken to reach this function will
+     * have flushed this thread cbset already.
+     */
+    perthread = pthread_getspecific(perthread_key);
+    if (perthread) {
+        ovs_assert(perthread->cbset == NULL);
+        pthread_setspecific(perthread_key, NULL);
+        ovsrcu_unregister__(perthread);
+    }
+
+    seq_wait(postpone_wait, *quiescent_seqno_get());
+    poll_block();
+}
+
+#else
+
+#define ovsrcu_postpone_end() do { } while (0)
+#define ovsrcu_postpone_wait_enable(b) do { } while (0)
+#define ovsrcu_postpone_wait() do { } while (0)
+
+#endif /* OVS_RCU_BLOCKING */
+
 static struct ovsrcu_perthread *
 ovsrcu_perthread_get(void)
 {
@@ -122,6 +195,7 @@  ovsrcu_quiesced(void)
             ovsthread_once_done(&once);
         }
     }
+    ovsrcu_postpone_wait();
 }
 
 /* Indicates the beginning of a quiescent state.  See "Details" near the top of
@@ -154,6 +228,7 @@  ovsrcu_quiesce(void)
     perthread = ovsrcu_perthread_get();
     perthread->seqno = seq_read(global_seqno);
     if (perthread->cbset) {
+        ovsrcu_postpone_wait_enable(false);
         ovsrcu_flush_cbset(perthread);
     }
     seq_change(global_seqno);
@@ -172,6 +247,7 @@  ovsrcu_try_quiesce(void)
     if (!seq_try_lock()) {
         perthread->seqno = seq_read_protected(global_seqno);
         if (perthread->cbset) {
+            ovsrcu_postpone_wait_enable(true);
             ovsrcu_flush_cbset__(perthread, true);
         }
         seq_change_protected(global_seqno);
@@ -349,6 +425,8 @@  ovsrcu_call_postponed(void)
         free(cbset);
     }
 
+    ovsrcu_postpone_end();
+
     return true;
 }
 
@@ -397,6 +475,7 @@  static void
 ovsrcu_unregister__(struct ovsrcu_perthread *perthread)
 {
     if (perthread->cbset) {
+        ovsrcu_postpone_wait_enable(false);
         ovsrcu_flush_cbset(perthread);
     }
 
@@ -432,6 +511,9 @@  ovsrcu_init_module(void)
 {
     static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
     if (ovsthread_once_start(&once)) {
+#ifdef OVS_RCU_BLOCKING
+        postpone_wait = seq_create();
+#endif
         global_seqno = seq_create();
         xpthread_key_create(&perthread_key, ovsrcu_thread_exit_cb);
         fatal_signal_add_hook(ovsrcu_cancel_thread_exit_cb, NULL, NULL, true);
diff --git a/tests/atlocal.in b/tests/atlocal.in
index f61e752bf..c677049c9 100644
--- a/tests/atlocal.in
+++ b/tests/atlocal.in
@@ -4,6 +4,7 @@  OPENSSL_SUPPORTS_SNI='@OPENSSL_SUPPORTS_SNI@'
 HAVE_UNBOUND='@HAVE_UNBOUND@'
 EGREP='@EGREP@'
 PYTHON3='@PYTHON3@'
+RCU_BLOCKING='@RCU_BLOCKING@'
 
 # PYTHONCOERCECLOCALE=0 disables the Unicode compatibility warning on
 # stderr that breaks almost any Python3 test (PEP 0538)
diff --git a/tests/library.at b/tests/library.at
index 4a549f77e..cae612eea 100644
--- a/tests/library.at
+++ b/tests/library.at
@@ -264,6 +264,7 @@  AT_CLEANUP
 AT_SETUP([rcu quiesce use-after-free detection])
 AT_SKIP_IF([test "$IS_WIN32" = "yes"])
 AT_SKIP_IF([test "$ASAN_ENABLED" = "no"])
+AT_SKIP_IF([test "$RCU_BLOCKING" = "no"])
 # SIGABRT + 128
 exit_status=134
 AT_KEYWORDS([rcu asan])
@@ -275,6 +276,7 @@  AT_CLEANUP
 AT_SETUP([rcu try-quiesce use-after-free detection])
 AT_SKIP_IF([test "$IS_WIN32" = "yes"])
 AT_SKIP_IF([test "$ASAN_ENABLED" = "no"])
+AT_SKIP_IF([test "$RCU_BLOCKING" = "no"])
 # SIGABRT + 128
 exit_status=134
 AT_KEYWORDS([rcu asan])
@@ -286,6 +288,7 @@  AT_CLEANUP
 AT_SETUP([rcu quiesce-start-end use-after-free detection])
 AT_SKIP_IF([test "$IS_WIN32" = "yes"])
 AT_SKIP_IF([test "$ASAN_ENABLED" = "no"])
+AT_SKIP_IF([test "$RCU_BLOCKING" = "no"])
 AT_KEYWORDS([rcu asan])
 # SIGABRT + 128
 exit_status=134