MEDIUM: pools: better check for size rounding overflow on registration

Certain object sizes cannot be controlled at declaration time because
the resulting object size may be slightly extended (tag, caller),
aligned and rounded up, or even doubled depending on pool settings
(e.g. if backup is used).

This patch addresses this by enlarging the type in the pool registration
to 64-bit so that no info is lost from the declaration, and extra checks
for overflows can be performed during registration after various rounding
steps. This allows to catch issues such as these ones and to report a
suitable error:

  global
      tune.http.logurilen 2147483647

  frontend
      capture request header name len 2147483647
      http-request capture src len 2147483647
      tcp-request content capture src len 2147483647

diff --git a/include/haproxy/pool-t.h b/include/haproxy/pool-t.h
index 54c1aebfae14e2d838a370e8ecc3343abf9e2184..3a1ea84680bee55eb8651ddb5680377e75049fac 100644 (file)
--- a/include/haproxy/pool-t.h
+++ b/include/haproxy/pool-t.h
@@ -72,8 +72,8 @@ struct pool_registration {
        struct list list;    /* link element */
        const char *name;    /* name of the pool */
        const char *file;    /* where the pool is declared */
+       ullong size;         /* expected object size */
        unsigned int line;   /* line in the file where the pool is declared, 0 if none */
-       unsigned int size;   /* expected object size */
        unsigned int flags;  /* MEM_F_* */
        unsigned int type_align;  /* type-imposed alignment; 0=unspecified */
        unsigned int align;  /* expected alignment; 0=unspecified */

diff --git a/include/haproxy/pool.h b/include/haproxy/pool.h
index a1e7be70985a914d399e546fb89c4f59b9ef589c..e05b95c6d0888ae8de502a07563d5079b417284a 100644 (file)
--- a/include/haproxy/pool.h
+++ b/include/haproxy/pool.h
@@ -183,7 +183,7 @@ unsigned long long pool_total_allocated(void);
 unsigned long long pool_total_used(void);
 void pool_flush(struct pool_head *pool);
 void pool_gc(struct pool_head *pool_ctx);
-struct pool_head *create_pool_with_loc(const char *name, unsigned int size, unsigned int align,
+struct pool_head *create_pool_with_loc(const char *name, ullong size, unsigned int align,
                                        unsigned int flags, const char *file, unsigned int line);
 struct pool_head *create_pool_from_reg(const char *name, struct pool_registration *reg);
 void create_pool_callback(struct pool_head **ptr, char *name, struct pool_registration *reg);

diff --git a/src/pool.c b/src/pool.c
index 5108a81e6f4e8a504c5a26e9e7a584cb18a541cb..b76bd83683ce0410bec680a788194a0801058c64 100644 (file)
--- a/src/pool.c
+++ b/src/pool.c
@@ -302,7 +302,7 @@ static int mem_should_fail(const struct pool_head *pool)
  * registration struct. Use create_pool() instead which does it for free.
  * The alignment will be stored as-is in the registration.
  */
-struct pool_head *create_pool_with_loc(const char *name, unsigned int size,
+struct pool_head *create_pool_with_loc(const char *name, ullong size,
                                       unsigned int align, unsigned int flags,
                                       const char *file, unsigned int line)
 {
@@ -335,7 +335,8 @@ struct pool_head *create_pool_from_reg(const char *name, struct pool_registratio
 {
        unsigned int extra_mark, extra_caller, extra;
        unsigned int flags = reg->flags;
-       unsigned int size = reg->size;
+       ullong reg_size = reg->size; // copy of the originally requested size
+       ullong size = reg_size;
        unsigned int alignment = reg->align;
        struct pool_head *pool = NULL;
        struct pool_head *entry;
@@ -374,6 +375,9 @@ struct pool_head *create_pool_from_reg(const char *name, struct pool_registratio
        extra_caller = (pool_debugging & POOL_DBG_CALLER) ? POOL_EXTRA_CALLER : 0;
        extra = extra_mark + extra_caller;
 
+       if (size > 0xFFFFFFFFULL || (size + extra) > 0xFFFFFFFFULL || (uint)(size + extra) < (uint)reg_size)
+               goto ovf;
+
        if (!(pool_debugging & POOL_DBG_NO_CACHE)) {
                /* we'll store two lists there, we need the room for this. Let's
                 * make sure it's always OK even when including the extra word
@@ -392,7 +396,7 @@ struct pool_head *create_pool_from_reg(const char *name, struct pool_registratio
         */
        if (!(flags & MEM_F_EXACT)) {
                align = (pool_debugging & POOL_DBG_TAG) ? sizeof(void *) : 16;
-               size  = ((size + align - 1) & -align);
+               size  = ((size + align - 1) & -(ullong)align);
        }
 
        if (pool_debugging & POOL_DBG_BACKUP) {
@@ -402,6 +406,9 @@ struct pool_head *create_pool_from_reg(const char *name, struct pool_registratio
                extra += size;
        }
 
+       if (size > 0xFFFFFFFFULL || (size + extra) > 0xFFFFFFFFULL || (uint)(size + extra) < (uint)reg_size)
+               goto ovf;
+
        /* TODO: thread: we do not lock pool list for now because all pools are
         * created during HAProxy startup (so before threads creation) */
        start = &pools;
@@ -496,6 +503,11 @@ struct pool_head *create_pool_from_reg(const char *name, struct pool_registratio
 
  fail:
        return pool;
+ ovf:
+       ha_alert("Failed to create pool '%s' of size '%llu': overflow detected due to too large "
+                "a configured size and/or configured pool options. Aborting.\n",
+                name, reg_size);
+       return NULL;
 }
 
 /* Tries to allocate an object for the pool <pool> using the system's allocator
@@ -1428,7 +1440,7 @@ int dump_pools_info(struct appctx *appctx)
                        struct pool_registration *reg;
 
                        list_for_each_entry(reg, &ctx->pool_info[i].entry->regs, list) {
-                               chunk_appendf(&trash, "      >  %-12s: size=%u flags=%#x align=%u", reg->name, reg->size, reg->flags, reg->align);
+                               chunk_appendf(&trash, "      >  %-12s: size=%llu flags=%#x align=%u", reg->name, reg->size, reg->flags, reg->align);
                                if (reg->file && reg->line)
                                        chunk_appendf(&trash, " [%s:%u]", reg->file, reg->line);
                                chunk_appendf(&trash, "\n");
@@ -1651,7 +1663,7 @@ void create_pool_callback(struct pool_head **ptr, char *name, struct pool_regist
 {
        *ptr = create_pool_from_reg(name, reg);
        if (!*ptr) {
-               ha_alert("Failed to allocate pool '%s' of size %u : %s. Aborting.\n",
+               ha_alert("Failed to allocate pool '%s' of size %llu : %s. Aborting.\n",
                         name, reg->size, strerror(errno));
                exit(1);
        }