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:          9818224174c4 ("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

1 files changed, 10 insertions, 6 deletions

diff --git a/sys/security/mac_do/mac_do.c b/sys/security/mac_do/mac_do.c
index a2259ed777ae..58abf1a8f608 100644
--- a/sys/security/mac_do/mac_do.c
+++ b/sys/security/mac_do/mac_do.c
@@ -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;
 }