Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Memory leak related to cloning pipes #1647

Merged
merged 2 commits into from
Sep 12, 2017
Merged

Memory leak related to cloning pipes #1647

merged 2 commits into from
Sep 12, 2017

Commits on Sep 12, 2017

  1. cfg-tree: memleak for filter clone 

    Lets consider the following config snippet:

filter f_warn  { level(warn, err, crit)  };
log {
  filter(f_warn);
  filter(f_warn);
};

There is a named filter and two filter references here. These filters
are instantiated with in different phases. The named filters are
instantiated during config parse, and added to CfgTree->objects
hashtable which has a general destructor.

The instantiation of the referenced filters happen during config
compile. In the cfg_tree_compile_single, when we hit the first filter
reference, we reuse the pipe that is instantiated during config parse:
the pipe element stored in CfgTree->objects. We utilize a flag to do
this only once. Now when we compile the the second filter reference,
we see that the pipe in CfgTree->objects is already used, so we clone
the pipe instead. Here a completely new unname pipe is created. So
that these could be freed they are added to a pointer array:
CfgTree->initialized_pipes, and during free, we call log_pipe_unref
for all these elements. The conflict comes from that both the pipes above
are added to the initialized_pipes array, but for the first one we
have two destructors: one for the CfgTree->objects hashtable, and one
for the CfgTree->initialized_pipes, but only one for the cloned pipe.
To avoid double free, the original implementation unconditionally
log_pipe_ref-fed all pipes that are added to CfgTree->initialized_pipes.
But this resulted in the cloned pipes had reference count 2 but only
one destructor, leaking memory.

As the config graph is recreated in every start, these leaks accumulate
in consecutive reloads.

This patch considers that initialized_pipes is intended to be used for
destructing all pipe objects. This means log_pipe_ref is missing for
pipes in objects in CfgTree->objects. So this patch branches, and
calls an extra log_pipe_ref for those, but not for the cloned pipes.

Reported in: syslog-ng#1317

Signed-off-by: Antal Nemes <antal.nemes@balabit.com>
    @furiel
    furiel committed Sep 12, 2017
    Configuration menu
    Copy the full SHA
    74be85e View commit details
    Browse the repository at this point in the history

  2. rewrite: memory leak during clone 

    The clone function accidentally refs self->replacement template. But
it is also ref-fed inside the constructor. So this extra ref is not
needed, and also causes memory leak.

As the clone is part of configuration load, this leak accumulates during reloads.

Signed-off-by: Antal Nemes <antal.nemes@balabit.com>
    @furiel
    furiel committed Sep 12, 2017
    Configuration menu
    Copy the full SHA
    3c8d58d View commit details
    Browse the repository at this point in the history