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
This commit is contained in:
Willy Tarreau 2022-12-08 17:47:59 +01:00
parent b634987fed
commit 9192d20f02
3 changed files with 28 additions and 14 deletions

View File

@ -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

View File

@ -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 */

View File

@ -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))) {