If the loop that registers the underlying tables detected an invalid table spec (missing colon), it would free the partially constructed pipemap or unionmap but not unregister any underlying tables that were registered during previous iterations of the loop. This would cause their refcount increments to leak. Fix the bug by checking every underlying table spec before registering any of them.
I believe the effects of the bug are very minor; users are unlikely to experience any notable negative consequences without this fix. The leak is one-time, and the returned surrogate dict is likely to render their config useless anyway. But I think it is still worth fixing: * The bug might raise alarms in static analysis tools or memory leak checkers like Valgrind. * The bug prevents the affected underlying tables from being properly closed before Postfix exits, which might matter for some table implementations. * This change makes it easier for potential contributors like me to understand the code. --- src/util/dict_pipe.c | 17 ++++++++++++----- src/util/dict_union.c | 17 ++++++++++++----- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/util/dict_pipe.c b/src/util/dict_pipe.c index 8ce0faad7..f26474307 100644 --- a/src/util/dict_pipe.c +++ b/src/util/dict_pipe.c @@ -151,13 +151,10 @@ DICT *dict_pipe_open(const char *name, int open_flags, int dict_flags) DICT_TYPE_PIPE)); /* - * The least-trusted table in the pipeline determines the over-all trust - * level. The first table determines the pattern-matching flags. + * Check all underlying table specs before registering any of them to avoid + * leaking refcounts if one of them is bad. */ - DICT_OWNER_AGGREGATE_INIT(aggr_owner); for (cpp = argv->argv; (dict_type_name = *cpp) != 0; cpp++) { - if (msg_verbose) - msg_info("%s: %s", myname, dict_type_name); if (strchr(dict_type_name, ':') == 0) DICT_PIPE_RETURN(dict_surrogate(DICT_TYPE_PIPE, name, open_flags, dict_flags, @@ -165,6 +162,16 @@ DICT *dict_pipe_open(const char *name, int open_flags, int dict_flags) "need \"%s:{type:name...}\"", DICT_TYPE_PIPE, name, DICT_TYPE_PIPE)); + } + + /* + * The least-trusted table in the pipeline determines the over-all trust + * level. The first table determines the pattern-matching flags. + */ + DICT_OWNER_AGGREGATE_INIT(aggr_owner); + for (cpp = argv->argv; (dict_type_name = *cpp) != 0; cpp++) { + if (msg_verbose) + msg_info("%s: %s", myname, dict_type_name); if ((dict = dict_handle(dict_type_name)) == 0) dict = dict_open(dict_type_name, open_flags, dict_flags); dict_register(dict_type_name, dict); diff --git a/src/util/dict_union.c b/src/util/dict_union.c index 80df03b61..84f2b69b5 100644 --- a/src/util/dict_union.c +++ b/src/util/dict_union.c @@ -164,13 +164,10 @@ DICT *dict_union_open(const char *name, int open_flags, int dict_flags) DICT_TYPE_UNION)); /* - * The least-trusted table in the set determines the over-all trust - * level. The first table determines the pattern-matching flags. + * Check all underlying table specs before registering any of them to avoid + * leaking refcounts if one of them is bad. */ - DICT_OWNER_AGGREGATE_INIT(aggr_owner); for (cpp = argv->argv; (dict_type_name = *cpp) != 0; cpp++) { - if (msg_verbose) - msg_info("%s: %s", myname, dict_type_name); if (strchr(dict_type_name, ':') == 0) DICT_UNION_RETURN(dict_surrogate(DICT_TYPE_UNION, name, open_flags, dict_flags, @@ -178,6 +175,16 @@ DICT *dict_union_open(const char *name, int open_flags, int dict_flags) "need \"%s:{type:name...}\"", DICT_TYPE_UNION, name, DICT_TYPE_UNION)); + } + + /* + * The least-trusted table in the set determines the over-all trust + * level. The first table determines the pattern-matching flags. + */ + DICT_OWNER_AGGREGATE_INIT(aggr_owner); + for (cpp = argv->argv; (dict_type_name = *cpp) != 0; cpp++) { + if (msg_verbose) + msg_info("%s: %s", myname, dict_type_name); if ((dict = dict_handle(dict_type_name)) == 0) dict = dict_open(dict_type_name, open_flags, dict_flags); dict_register(dict_type_name, dict); -- 2.49.0 _______________________________________________ Postfix-devel mailing list -- postfix-devel@postfix.org To unsubscribe send an email to postfix-devel-le...@postfix.org