MINOR: pools: make DEBUG_UAF a runtime setting

Since the massive pools cleanup that happened in 2.6, the pools
architecture was made quite more hierarchical and many alternate code
blocks could be moved to runtime flags set by -dM. One of them had not
been converted by then, DEBUG_UAF. It's not much more difficult actually,
since it only acts on a pair of functions indirection on the slow path
(OS-level allocator) and a default setting for the cache activation.

This patch adds the "uaf" setting to the options permitted in -dM so
that it now becomes possible to set or unset UAF at boot time without
recompiling. This is particularly convenient, because every 3 months on
average, developers ask a user to recompile haproxy with DEBUG_UAF to
understand a bug. Now it will not be needed anymore, instead the user
will only have to disable pools and enable uaf using -dMuaf. Note that
-dMuaf only disables previously enabled pools, but it remains possible
to re-enable caching by specifying the cache after, like -dMuaf,cache.
A few tests with this mode show that it can be an interesting combination
which catches significantly less UAF but will do so with much less
overhead, so it might be compatible with some high-traffic deployments.

The change is very small and isolated. It could be helpful to backport
this at least to 2.7 once confirmed not to cause build issues on exotic
systems, and even to 2.6 a bit later as this has proven to be useful
over time, and could be even more if it did not require a rebuild. If
a backport is desired, the following patches are needed as well:

  CLEANUP: pools: move the write before free to the uaf-only function
  CLEANUP: pool: only include pool-os from pool.c not pool.h
  REORG: pool: move all the OS specific code to pool-os.h
  CLEANUP: pools: get rid of CONFIG_HAP_POOLS
  DEBUG: pool: show a few examples in -dMhelp

diff --git a/doc/internals/api/pools.txt b/doc/internals/api/pools.txt
index 315d5c4b237e5c6685e7fd7e65773e739d1f1b80..4023dc316812316c96628ccf3e47e267546fcdfd 100644 (file)
--- a/doc/internals/api/pools.txt
+++ b/doc/internals/api/pools.txt
@@ -75,9 +75,10 @@ The pools architecture is selected at build time. The main options are:
     accesses. Released objects are instantly freed using munmap() so that any
     immediate subsequent access to the memory area crashes the process if the
     area had not been reallocated yet. This mode can be enabled at build time
-    by setting DEBUG_UAF. It tends to consume a lot of memory and not to scale
-    at all with concurrent calls, that tends to make the system stall. The
-    watchdog may even trigger on some slow allocations.
+    by setting DEBUG_UAF, or at run time by disabling pools and enabling UAF
+    with "-dMuaf". It tends to consume a lot of memory and not to scale at all
+    with concurrent calls, that tends to make the system stall. The watchdog
+    may even trigger on some slow allocations.
 
 There are no more provisions for running with a shared pool but no thread-local
 cache: the shared pool's main goal is to compensate for the expensive calls to
@@ -511,7 +512,9 @@ DEBUG_UAF
         through mmap() and munmap(). The memory usage significantly inflates
         and the performance degrades, but this allows to detect a lot of
         use-after-free conditions by crashing the program at the first abnormal
-        access. This should not be used in production.
+        access. This should not be used in production. It corresponds to
+        boot-time options "-dMuaf". Caching is disabled but may be re-enabled
+        using "-dMcache".
 
 DEBUG_POOL_INTEGRITY
         When enabled, objects picked from the cache are checked for corruption

diff --git a/include/haproxy/pool-t.h b/include/haproxy/pool-t.h
index 460173c6fb6743726405e086757f466e9b457313..523bbaf9af0dc9f6f16fde8e8cea7f1b7db9feba 100644 (file)
--- a/include/haproxy/pool-t.h
+++ b/include/haproxy/pool-t.h
@@ -50,6 +50,7 @@
 #define POOL_DBG_CALLER     0x00000040  // trace last caller's location
 #define POOL_DBG_TAG        0x00000080  // place a tag at the end of the area
 #define POOL_DBG_POISON     0x00000100  // poison memory area on pool_alloc()
+#define POOL_DBG_UAF        0x00000200  // enable use-after-free protection
 
 
 /* This is the head of a thread-local cache */

diff --git a/src/pool.c b/src/pool.c
index ba5fe5165278855f58efd73333ddedee78b7c380..e225d2144dc9d356732dbdb1d36ba6581543f0cc 100644 (file)
--- a/src/pool.c
+++ b/src/pool.c
@@ -60,6 +60,9 @@ uint pool_debugging __read_mostly =               /* set of POOL_DBG_* flags */
 #endif
 #if defined(DEBUG_MEMORY_POOLS)
        POOL_DBG_TAG        |
+#endif
+#if defined(DEBUG_UAF)
+       POOL_DBG_UAF        |
 #endif
        0;
 
@@ -79,6 +82,7 @@ static const struct {
        { POOL_DBG_CALLER,     "caller",     "no-caller",    "save caller information in cache" },
        { POOL_DBG_TAG,        "tag",        "no-tag",       "add tag at end of allocated objects" },
        { POOL_DBG_POISON,     "poison",     "no-poison",    "poison newly allocated objects" },
+       { POOL_DBG_UAF,        "uaf",        "no-uaf",       "enable use-after-free checks (slow)" },
        { 0 /* end */ }
 };
 
@@ -336,11 +340,11 @@ void *pool_get_from_os(struct pool_head *pool)
 {
        if (!pool->limit || pool->allocated < pool->limit) {
                void *ptr;
-#ifdef DEBUG_UAF
-               ptr = pool_alloc_area_uaf(pool->alloc_sz);
-#else
-               ptr = pool_alloc_area(pool->alloc_sz);
-#endif
+
+               if (pool_debugging & POOL_DBG_UAF)
+                       ptr = pool_alloc_area_uaf(pool->alloc_sz);
+               else
+                       ptr = pool_alloc_area(pool->alloc_sz);
                if (ptr) {
                        _HA_ATOMIC_INC(&pool->allocated);
                        return ptr;
@@ -357,11 +361,10 @@ void *pool_get_from_os(struct pool_head *pool)
  */
 void pool_put_to_os(struct pool_head *pool, void *ptr)
 {
-#ifdef DEBUG_UAF
-       pool_free_area_uaf(ptr, pool->alloc_sz);
-#else
-       pool_free_area(ptr, pool->alloc_sz);
-#endif
+       if (pool_debugging & POOL_DBG_UAF)
+               pool_free_area_uaf(ptr, pool->alloc_sz);
+       else
+               pool_free_area(ptr, pool->alloc_sz);
        _HA_ATOMIC_DEC(&pool->allocated);
 }
 
@@ -1061,6 +1064,8 @@ int pool_parse_debugging(const char *str, char **err)
                                  "  Detect out-of-bound corruptions: -dMno-merge,tag\n"
                                  "  Detect post-free cache corruptions: -dMno-merge,cold-first,integrity,caller\n"
                                  "  Detect all cache corruptions: -dMno-merge,cold-first,integrity,tag,caller\n"
+                                 "  Detect UAF (disables cache, very slow): -dMuaf\n"
+                                 "  Detect post-cache UAF: -dMuaf,cache,no-merge,cold-first,integrity,tag,caller\n"
                                  "  Detect post-free cache corruptions: -dMno-merge,cold-first,integrity,caller\n",
                                  *err);
                        return -1;
@@ -1069,6 +1074,11 @@ int pool_parse_debugging(const char *str, char **err)
                for (v = 0; dbg_options[v].flg; v++) {
                        if (isteq(feat, ist(dbg_options[v].set))) {
                                new_dbg |= dbg_options[v].flg;
+                               /* UAF implicitly disables caching, but it's
+                                * still possible to forcefully re-enable it.
+                                */
+                               if (dbg_options[v].flg == POOL_DBG_UAF)
+                                       new_dbg |= POOL_DBG_NO_CACHE;
                                break;
                        }
                        else if (isteq(feat, ist(dbg_options[v].clr))) {