mirror of
https://git.FreeBSD.org/src.git
synced 2026-06-02 11:24:32 +00:00
MAC/do: Fix releasing a nonexistent reference on configuration parsing error
On parsing error, parse_and_set_conf(), introduced with the recent
"executable paths" feature, has been calling drop_conf() on the
being-built configuration. However, that configuration structure is
allocated through alloc_conf(), which does not grab a reference.
Calling drop_conf() on it, which releases a reference, is thus
erroneous, and causes the underlying counter to saturate, translating
into a memory leak.
To fix this bug, make alloc_conf() grab a reference on the newly-created
'struct conf', and rename it to new_conf() to be more in line with what
it does. Keep set_conf() as is, i.e., grabbing an additional reference
on behalf of the jail that is going to hold the configuration.
Consequently, make sure that callers of alloc_conf() unconditionally
drop the reference acquired by the latter before returning (i.e., even
if set_conf() has been called).
While here, since hold_conf() is always used to obtain additional
references on a configuration (new_conf() does not use it, instead
directly setting the use count), add an assertion that it is never used
on a configuration that has no references at all (which indicates that
the configuration has been destroyed).
These changes generally simplify the lifecycle of configurations,
reducing the probability of re-introducing reference mismatches (at the
expense of slightly more reference counting operations, but performance
does not matter here).
Reviewed by: bapt
Fixes: 9818224174 ("MAC/do: Executable paths feature (GSoC 2025's final state)")
MFC after: 1 month
Sponsored by: The FreeBSD Foundation
Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38
This commit is contained in:
@@ -366,14 +366,14 @@ init_exec_paths(struct exec_paths *const exec_paths)
|
||||
}
|
||||
|
||||
static struct conf *
|
||||
alloc_conf(void)
|
||||
new_conf(void)
|
||||
{
|
||||
struct conf *const conf = malloc(sizeof(*conf), M_MAC_DO, M_WAITOK |
|
||||
M_ZERO);
|
||||
|
||||
init_rules(&conf->rules);
|
||||
init_exec_paths(&conf->exec_paths);
|
||||
conf->use_count = 0;
|
||||
refcount_init(&conf->use_count, 1);
|
||||
|
||||
return (conf);
|
||||
}
|
||||
@@ -1163,7 +1163,10 @@ error:
|
||||
static void
|
||||
hold_conf(struct conf *const conf)
|
||||
{
|
||||
refcount_acquire(&conf->use_count);
|
||||
int old_count __diagused = refcount_acquire(&conf->use_count);
|
||||
|
||||
KASSERT(old_count != 0,
|
||||
("MAC/do: Trying to resurrect a destroyed configuration."));
|
||||
}
|
||||
|
||||
static void
|
||||
@@ -1329,7 +1332,7 @@ set_conf(struct prison *const pr, struct conf *const conf)
|
||||
static void
|
||||
set_default_conf(struct prison *const pr)
|
||||
{
|
||||
struct conf *const conf = alloc_conf();
|
||||
struct conf *const conf = new_conf();
|
||||
|
||||
strlcpy(conf->exec_paths.exec_paths_str, "/usr/bin/mdo",
|
||||
MAX_EXEC_PATHS_SIZE);
|
||||
@@ -1337,6 +1340,7 @@ set_default_conf(struct prison *const pr)
|
||||
conf->exec_paths.exec_path_count = 1;
|
||||
|
||||
set_conf(pr, conf);
|
||||
drop_conf(conf);
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -1406,7 +1410,7 @@ parse_and_set_conf(struct prison *pr, const char *rules_string,
|
||||
if (need_applicable_conf)
|
||||
applicable_conf = find_conf(pr, NULL);
|
||||
|
||||
conf = alloc_conf();
|
||||
conf = new_conf();
|
||||
|
||||
if (rules_string != NULL && rules_string[0] != '\0') {
|
||||
error = parse_rules(rules_string, &conf->rules, parse_error);
|
||||
@@ -1429,12 +1433,12 @@ parse_and_set_conf(struct prison *pr, const char *rules_string,
|
||||
|
||||
MPASS(error == 0 && *parse_error == NULL);
|
||||
out:
|
||||
drop_conf(conf);
|
||||
if (applicable_conf != NULL)
|
||||
drop_conf(applicable_conf);
|
||||
return (error);
|
||||
error:
|
||||
MPASS(error != 0 && *parse_error != NULL);
|
||||
drop_conf(conf);
|
||||
goto out;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user