BUG/MINOR: log: smp_rgs array issues with inherited global log directives

When a log directive is defined in the global section, each time we use
"log global" in a proxy section, the global log directives are duplicated
for the current proxy. This works by creating a new proxy logger struct
and duplicating every members for each global one.

However, smp_rgs logger member is a special pointer member that is
allocated when "range" is used on a log directive. Currently, we simply
copy the array pointer (from the global one), instead of creating our own
copy. Because of that, range log sampling may not work properly in some
situations prior to 3f1284560 ("MINOR: log: remove the unused curr_idx in
struct smp_log_range") when used in global log directives, for instance:

  global
    log 127.0.0.1:5114 format raw sample 1-2,3:4 local0 info # should receive 75% of all proxy logs
    log 127.0.0.1:5115 format raw sample 4:4 local0 info     # should receive 25% of all proxy logs

  listen proxy1
    log global

  listen proxy2
    log global

May not work as expected, because curr_idx was stored within smp_rgs array
member prior to 3f1284560, and due to this bug, it happens to be shared
between every log directive inherited from a "global" one. The result is
that curr_idx counter will not behave properly because the index will be
increased globally instead of per-log directive, and it could even suffer
from concurrent thread accesses under load since we don't own the global
log directive's lock when manipulating it.

Another issue that was revealed because of this bug is that the smp_rgs
array allocated during config parsing is never freed in free_logger(),
resulting in small memory leak during clean exit.

To fix these issues all at once, let's properly duplicate smp_rgs logger
struct member in dup_logger() like we already do for other special members
so that every log directive have its own sms_rgs copy, and then
systematically free it in free_logger().

While this bug affects all stable versions (including 2.4), it's probably
best to not backport this beyond 2.6 because of 211ea252d
("BUG/MINOR: logs: fix logsrv leaks on clean exit") prerequisite that
first appears in 2.6.

[ada: for versions prior to 2.9, 969e212
 ("MINOR: log: add dup_logsrv() helper function") and 76acde91
 ("BUG/MINOR: log: keep the ref in dup_logger()") must be backported
 first.
 Note: Some ctx adjustments should be performed because 'logger' struct
 used to be named 'logsrv' in the past and 2.9 introduced logger target
 struct member. Thus it's probably easier to manually apply 76acde91 and
 the current bugfix by hand directly on top of 969e212.
]

diff --git a/src/log.c b/src/log.c
index 5ae0ec0ea2bb611a65ee421667eba0e4a86c24a5..3d66e7a8c07416964b3b2958983bac60936b899e 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -1423,6 +1423,7 @@ struct logger *dup_logger(struct logger *def)
 
        /* default values */
        cpy->conf.file = NULL;
+       cpy->lb.smp_rgs = NULL;
        LIST_INIT(&cpy->list);
 
        /* special members */
@@ -1433,6 +1434,13 @@ struct logger *dup_logger(struct logger *def)
                if (!cpy->conf.file)
                        goto error;
        }
+       if (def->lb.smp_rgs) {
+               cpy->lb.smp_rgs = malloc(sizeof(*cpy->lb.smp_rgs) * def->lb.smp_rgs_sz);
+               if (!cpy->lb.smp_rgs)
+                       goto error;
+               memcpy(cpy->lb.smp_rgs, def->lb.smp_rgs,
+                      sizeof(*cpy->lb.smp_rgs) * def->lb.smp_rgs_sz);
+       }
 
        /* inherit from original reference if set */
        cpy->ref = (def->ref) ? def->ref : def;
@@ -1456,6 +1464,7 @@ void free_logger(struct logger *logger)
        BUG_ON(LIST_INLIST(&logger->list));
        ha_free(&logger->conf.file);
        deinit_log_target(&logger->target);
+       free(logger->lb.smp_rgs);
        free(logger);
 }