The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=79d0dbc9c6d6a1627c9f3eeab0633c232a6a0a34
commit 79d0dbc9c6d6a1627c9f3eeab0633c232a6a0a34 Author: Olivier Certner <[email protected]> AuthorDate: 2026-06-01 08:52:16 +0000 Commit: Olivier Certner <[email protected]> CommitDate: 2026-06-01 15:23:51 +0000 MAC/do: Fix double-free on parse error after "executable paths" feature parse_rules() has been calling toast_rules() in case of a parse error in order to deallocate the 'struct rule' objects it has constructed up to that point. toast_rules() would take a pointer to a full 'struct rules' object, and besides freeing all 'struct rule' referenced by it, would also free the holding 'struct rules' itself. With the introduction of the "executable paths" feature, and the embedding of 'struct rules' into 'struct conf', meaning that the lifecycle for 'struct rules' was no longer independent, toast_rules() was changed not to free the passed 'struct rules' (as it was a field of a 'struct conf' object). Unfortunately, this change was not completed with a reinitialization of the rules list head, so the 'struct conf' object would continue to reference just-freed rules, which then would be freed a second time on destruction of that container. So, make toast_rules() re-initialize the rules list in 'struct rules', which it logically has been having to do since not freeing the enclosing 'struct rules'. This alone is enough to fix the bug, but let's use the occasion to change the contract of parse_rules() and bring its herald comment up-to-date: On error, parse_rules() now simply leaves already constructed 'struct rule' objects in 'conf'. The latter is eventually destroyed and the rule objects reclaimed at that point. Add a test trying to set an invalid rules configuration with the first rule being valid and the second being invalid, which triggers the bug (and an immediate panic() on an INVARIANTS kernel). Reported by: impost0r(ret2plt) <[email protected]> Reviewed by: markj Fixes: 9818224174c4 ("MAC/do: Executable paths feature (GSoC 2025's final state)") Sponsored by: The FreeBSD Foundation --- sys/security/mac_do/mac_do.c | 16 ++++++++-------- tests/sys/mac/do/invalid_configs.sh | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/sys/security/mac_do/mac_do.c b/sys/security/mac_do/mac_do.c index 93f2084d1c93..7bb3e2a150c5 100644 --- a/sys/security/mac_do/mac_do.c +++ b/sys/security/mac_do/mac_do.c @@ -390,6 +390,7 @@ toast_rules(struct rules *const rules) free(rule->gids, M_MAC_DO); free(rule, M_MAC_DO); } + STAILQ_INIT(head); } static inline void @@ -1071,13 +1072,13 @@ einval: /* * Parse rules specification and produce rule structures out of it. * - * Must be called with '*parse_error' set to NULL. Returns 0 on success, with - * '*rulesp' made to point to a 'struct rule' representing the rules. On error, - * the returned value is non-zero and '*rulesp' is unchanged. If 'string' has - * length greater or equal to MAX_RULE_STRING_SIZE, ENAMETOOLONG is returned. If - * it is not in the expected format, EINVAL is returned. If an error is - * returned, '*parse_error' is set to point to a 'struct parse_error' giving an - * error message for the problem. + * Must be called with '*parse_error' set to NULL. Returns 0 on success, + * filling the passed '*rules' with 'struct rule' objects. On error, the + * returned value is non-zero, and '*rules' may have been changed. If 'string' + * has length greater or equal to MAX_RULE_STRING_SIZE, ENAMETOOLONG is + * returned. If it is not in the expected format, EINVAL is returned. If an + * error is returned, '*parse_error' is set to point to a 'struct parse_error' + * giving an error message for the problem. * * Expected format: A >-colon-separated list of rules of the form * "<from>><target>" (for backwards compatibility, a semi-colon ":" is accepted @@ -1123,7 +1124,6 @@ parse_rules(const char *const string, struct rules *const rules, error = parse_single_rule(rule, rules, parse_error); if (error != 0) { (*parse_error)->pos += rule - copy; - toast_rules(rules); goto error; } } diff --git a/tests/sys/mac/do/invalid_configs.sh b/tests/sys/mac/do/invalid_configs.sh index d1a9eb8c1e96..91e38a0055c0 100644 --- a/tests/sys/mac/do/invalid_configs.sh +++ b/tests/sys/mac/do/invalid_configs.sh @@ -72,6 +72,19 @@ rules_wrong_separator_body() sysctl_set_and_check_fails_rules "uid=1001>gid=0:gid=1001>gid=5" } +# Added after observing a panic() in this situation because of a double-free +# after introduction of "exec_paths". +atf_test_case non_first_rule_unparseable +non_first_rule_unparseable_head() +{ + atf_set descr "Non-first rule wrong" +} + +non_first_rule_unparseable_body() +{ + sysctl_set_and_check_fails_rules "gid=1001>uid=0;hello" +} + atf_init_test_cases() { @@ -83,4 +96,5 @@ atf_init_test_cases() atf_add_test_case rule_user_names_fail atf_add_test_case rule_group_names_fail atf_add_test_case rules_wrong_separator + atf_add_test_case non_first_rule_unparseable }
