Date: Fri, 29 May 2026 16:01:32 +0000 From: Olivier Certner <olce@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 0aef1c059fae - main - MAC/do: Document and assert when parse error objects must be built Message-ID: <6a19b85c.35df6.481edbe9@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=0aef1c059fae86a7f62b444dc3352eb0d5bdaa31 commit 0aef1c059fae86a7f62b444dc3352eb0d5bdaa31 Author: Olivier Certner <olce@FreeBSD.org> AuthorDate: 2026-05-19 19:38:41 +0000 Commit: Olivier Certner <olce@FreeBSD.org> CommitDate: 2026-05-29 15:21:16 +0000 MAC/do: Document and assert when parse error objects must be built The invariant is that parse_*() functions must create a parse error object and return it through their 'parse_error' argument if and only if they return an error code (non-zero). Add assertions checking this invariant in various places, and in particular in the new parse_exec_paths(), to be future proof. Change the contract of parse_and_set_conf() so that the caller is required to pass a NULL '*parse_error'. Remove useless resetting of '*parse_error' to NULL. While here, remove a test that is always true thanks to this invariant and that was recently introduced with the "executable paths" feature. No functional change intended. Reviewed by: bapt MFC after: 1 month Sponsored by: The FreeBSD Foundation Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38 --- sys/security/mac_do/mac_do.c | 82 +++++++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 32 deletions(-) diff --git a/sys/security/mac_do/mac_do.c b/sys/security/mac_do/mac_do.c index 5aacce1b4d09..dc5d3d29d988 100644 --- a/sys/security/mac_do/mac_do.c +++ b/sys/security/mac_do/mac_do.c @@ -71,6 +71,11 @@ static const char *id_type_to_str[] = { #define PARSE_ERROR_SIZE 256 +/* + * All functions having a parse error parameter must return through it a parse + * error object if and only if they return an error value (non-zero); else, NULL + * must be returned through it. + */ struct parse_error { size_t pos; char msg[PARSE_ERROR_SIZE]; @@ -776,6 +781,7 @@ parse_target_clause(char *to, struct rule *const rule, check_type_and_finish: check_type_and_type_flags(type, *tflags); finish: + MPASS(error == 0 && *parse_error == NULL); return (0); einval: /* We must have built a parse error on error. */ @@ -853,7 +859,7 @@ pour_list_into_rule(const id_type_t type, struct id_list *const list, make_parse_error(parse_error, 0, "Incompatible flags or duplicate " "GID %u.", id); - return (EINVAL); + goto einval; } check_type_and_id_flags(type, array[ref_idx].flags); @@ -867,7 +873,7 @@ pour_list_into_rule(const id_type_t type, struct id_list *const list, */ make_parse_error(parse_error, 0, "Duplicate UID %u.", id); - return (EINVAL); + goto einval; default: __assert_unreachable(); @@ -876,7 +882,12 @@ pour_list_into_rule(const id_type_t type, struct id_list *const list, *nb = ref_idx + 1; } + MPASS(*parse_error == NULL); return (0); + +einval: + MPASS(*parse_error != NULL); + return (EINVAL); } /* @@ -1002,6 +1013,7 @@ parse_single_rule(char *rule, struct rules *const rules, } STAILQ_INSERT_TAIL(&rules->head, new, r_entries); + MPASS(error == 0 && *parse_error == NULL); return (0); einval: @@ -1019,13 +1031,13 @@ einval: /* * Parse rules specification and produce rule structures out of it. * - * 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 - * MAC_RULE_STRING_LEN, 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, - * else '*parse_error' is set to NULL. + * 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 MAC_RULE_STRING_LEN, 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 @@ -1050,8 +1062,6 @@ parse_rules(const char *const string, struct rules *const rules, char *copy, *p, *rule; int error = 0; - *parse_error = NULL; - if (len >= MAC_RULE_STRING_LEN) { make_parse_error(parse_error, 0, "Rule specification string is too long (%zu, max %zu)", @@ -1074,15 +1084,22 @@ parse_rules(const char *const string, struct rules *const rules, if (error != 0) { (*parse_error)->pos += rule - copy; toast_rules(rules); - goto out; + goto error; } } + MPASS(error == 0 && *parse_error == NULL); out: free(copy, M_MAC_DO); return (error); +error: + MPASS(error != 0 && *parse_error != NULL); + goto out; } +/* + * Similar constraints as parse_rules() (which see). + */ static int parse_exec_paths(const char *const string, struct exec_paths *const exec_paths, struct parse_error **const parse_error) @@ -1091,8 +1108,6 @@ parse_exec_paths(const char *const string, struct exec_paths *const exec_paths, char *copy, *p, *path; int error = 0; - *parse_error = NULL; - if (len >= EXEC_PATHS_MAXLEN) { make_parse_error(parse_error, 0, "Exec path specification string is too long (%zu, max %u)", @@ -1119,7 +1134,7 @@ parse_exec_paths(const char *const string, struct exec_paths *const exec_paths, "Too many exec paths specified (max %d)", MAX_EXEC_PATHS); error = EINVAL; - goto out; + goto error; } path_len = strlen(path); @@ -1128,7 +1143,7 @@ parse_exec_paths(const char *const string, struct exec_paths *const exec_paths, "Exec paths too long (%zu, max %u)", path_len, PATH_MAX - 1); error = ENAMETOOLONG; - goto out; + goto error; } strlcpy(exec_paths->exec_paths[exec_paths->exec_path_count], @@ -1139,12 +1154,16 @@ parse_exec_paths(const char *const string, struct exec_paths *const exec_paths, if (exec_paths->exec_path_count == 0) { make_parse_error(parse_error, 0, "No valid exec paths found"); error = EINVAL; - goto out; + goto error; } + MPASS(error == 0 && *parse_error == NULL); out: free(copy, M_MAC_DO); return (error); +error: + MPASS(error != 0 && *parse_error != NULL); + goto out; } /* @@ -1360,6 +1379,7 @@ clone_exec_paths(struct exec_paths *dst, struct exec_paths *const src) sizeof(dst->exec_paths_str)); } +/* Must be called with '*parse_error' set to NULL. */ static int parse_and_set_conf(struct prison *pr, const char *rules_string, const char *exec_paths_string, struct parse_error **parse_error) @@ -1370,8 +1390,6 @@ parse_and_set_conf(struct prison *pr, const char *rules_string, int error = 0; bool need_applicable_conf; - *parse_error = NULL; - need_applicable_conf = (rules_string == NULL || rules_string[0] == '\0' || exec_paths_string == NULL || exec_paths_string[0] == '\0'); @@ -1386,7 +1404,7 @@ parse_and_set_conf(struct prison *pr, const char *rules_string, if (rules_string != NULL && rules_string[0] != '\0') { error = parse_rules(rules_string, &conf->rules, parse_error); if (error != 0) - goto out; + goto error; } else if (applicable_conf != NULL) clone_rules(&conf->rules, &applicable_conf->rules); @@ -1395,20 +1413,22 @@ parse_and_set_conf(struct prison *pr, const char *rules_string, error = parse_exec_paths(exec_paths_string, &conf->exec_paths, parse_error); if (error != 0) - goto out; + goto error; } else if (applicable_conf != NULL) clone_exec_paths(&conf->exec_paths, &applicable_conf->exec_paths); set_conf(pr, conf); + MPASS(error == 0 && *parse_error == NULL); out: if (applicable_conf != NULL) drop_conf(applicable_conf); - if (error != 0) - drop_conf(conf); - return (error); +error: + MPASS(error != 0 && *parse_error != NULL); + drop_conf(conf); + goto out; } static int @@ -1418,7 +1438,7 @@ mac_do_sysctl_rules(SYSCTL_HANDLER_ARGS) struct prison *const td_pr = req->td->td_ucred->cr_prison; struct prison *pr; struct conf *conf; - struct parse_error *parse_error; + struct parse_error *parse_error = NULL; int error; conf = find_conf(td_pr, &pr); @@ -1460,7 +1480,7 @@ mac_do_sysctl_exec_paths(SYSCTL_HANDLER_ARGS) struct prison *const td_pr = req->td->td_ucred->cr_prison; struct prison *pr; struct conf *conf; - struct parse_error *parse_error; + struct parse_error *parse_error = NULL; int error; conf = find_conf(td_pr, &pr); @@ -1725,12 +1745,10 @@ mac_do_jail_set(void *obj, void *data) &parse_error); if (error != 0) { - if (parse_error != NULL) { - vfs_opterror(opts, - "MAC/do: Parse error at index %zu: %s\n", - parse_error->pos, parse_error->msg); - free_parse_error(parse_error); - } + vfs_opterror(opts, + "MAC/do: Parse error at index %zu: %s\n", + parse_error->pos, parse_error->msg); + free_parse_error(parse_error); return (error); }home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a19b85c.35df6.481edbe9>