diff mbox series

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

Message ID 28b891f132ade312c53f022fd2e277fbd07ed6fd.1621517561.git.grive@u256.net
State New
Headers show
Series RCU: Add blocking mode for debugging | expand

Commit Message

Gaetan Rivet May 20, 2021, 1:35 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                        | 87 ++++++++++++++++++++++++++++
 lib/ovs-rcu.h                        | 31 ++++++++++
 tests/atlocal.in                     |  1 +
 tests/library.at                     |  3 +
 9 files changed, 150 insertions(+)
diff mbox series

Patch

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 19600a668..c9e04c090 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 e2350c6d9..56f6f42fc 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 79e18b85b..daafbe150 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,7 @@  Post-v2.15.0
      * OVS validated with DPDK 20.11.1. It is recommended to use this version
        until further releases.
    - New configure option '--enable-asan' enables AddressSanitizer.
+   - New configure option '--enable-rcu-blocking' 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..295a56778 100644
--- a/lib/ovs-rcu.c
+++ b/lib/ovs-rcu.c
@@ -71,6 +71,84 @@  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);
+    }
+
+    /* Flush pollable events to ensure only the RCU wait will
+     * wake the thread. */
+    poll_immediate_wake();
+    poll_block();
+
+    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 +200,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 +233,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 +252,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 +430,8 @@  ovsrcu_call_postponed(void)
         free(cbset);
     }
 
+    ovsrcu_postpone_end();
+
     return true;
 }
 
@@ -397,6 +480,7 @@  static void
 ovsrcu_unregister__(struct ovsrcu_perthread *perthread)
 {
     if (perthread->cbset) {
+        ovsrcu_postpone_wait_enable(false);
         ovsrcu_flush_cbset(perthread);
     }
 
@@ -432,6 +516,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/lib/ovs-rcu.h b/lib/ovs-rcu.h
index ecc4c9201..0b668d132 100644
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -155,6 +155,37 @@ 
  *         port_delete(id);
  *     }
  *
+ * Debugging
+ * ---------
+ *
+ * Deferred release of resources can prevent memory error detection.
+ * For example, keeping references to objects protected by RCU across quiescing
+ * periods creates a race condition.  The reference can either remain available
+ * or be released, depending on how the RCU thread is scheduled.  It can make
+ * tools such as an Address Sanitizer (ASAN) less effective.
+ *
+ * A special mode of operation is proposed making the RCU blocking.  In this
+ * mode, a user thread having postponed callbacks will wait for their
+ * execution at the end of their quiescent period.  If a reference was
+ * scheduled for release at some point, this mode will ensure that the release
+ * is executed deterministically.
+ *
+ *     1| void foo()
+ *     2| {
+ *     3|     char *var = xmalloc(1);
+ *     4|
+ *     5|     var[0] = 'a';
+ *     6|     ovsrcu_postpone(free, var);
+ *     7|     ovsrcu_quiesce();
+ *     8|     var[0] = 'b';
+ *     9| }
+ *
+ * If this mode is enabled, the user thread will stop line 7, until the RCU
+ * thread executed 'free(var)'.  If ASAN is in use, it will immediately
+ * detect the use-after-free line 8.
+ *
+ * This mode is enabled during configure by '--enable-rcu-blocking'. It should
+ * be used in conjunction with '--enable-asan'.
  */
 
 #include "compiler.h"
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