From nobody Fri May 29 16:01:57 2026 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4gRp5L0Lvyz6flR2 for ; Fri, 29 May 2026 16:01:58 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R13" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4gRp5K5l2tz3J4q for ; Fri, 29 May 2026 16:01:57 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1780070517; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=VT0r9zqvYaJ2EWnSSjgnZmOYcH1OrfSQoBQ64mV7jpY=; b=xgYa4QVOmae+mkT7WJkoP/+0X2Dn++yX8SDII1iSixutxsG5ct1sBWHfDaCh53o/JPCtj4 d9JLrAfg8ZXaooA8C6pBQvRtma/3prnqiacNoosEgliGqd/IJ9+b+r/QMtFceerw9n+zkj yiWJpq5NXTICQ5SmaZX1vMc6HdtUABRP5BVU9XyCwuRoTpBAMmAp4hBjCF4YuVaeoII8mr APlLpMPNHce5JtgYdPbgW6DrProHYyyLe66xI1/wh/PxitoX6dZbaN8WvIvm+zrO5oEaZF FlxRNVHkKabP/Iets9bv3nT6cFOZqtDQgf/IlKQowP4Gh09rd7zz1+wXIp08GA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1780070517; a=rsa-sha256; cv=none; b=jnuNZz2BjZkwfBvCy+M4yjF8PwxGbbDkr4fzN1xMf7awB2fYzop+9nnLjmc3q7mcO5tZ9g TP0qYcZBUnsIauCUy3yIALQ+9bd+t7GkkI3+SgS7aXtFyATJcAatNv9StHRZkPg7pXlgRr 6E5RZ3Pxeq40MJpxme+JrA1Si1NLgos9Z63J9aUstuT7Ch0n/VTTWHCxczKMFv8tubxPbM PDNSq89piuUJ5CP6n6NsnIZpMWQ15HaMrf9V6r/rriqwvC0jVfbj+tAdDGH4hE7OQmpuvq rzHWk2alvG4ZbaoJBWSoDIN+cLqni4F/s7b/H/Nnu8BDtTELqvQfgF++HBhnYg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1780070517; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=VT0r9zqvYaJ2EWnSSjgnZmOYcH1OrfSQoBQ64mV7jpY=; b=kdW90R14E8CxwQiyKA7zZAZLj8oFIl/z9PlNCQsJ7Ufz39DNxH2kGzgf9RfyO/t8ZuTtDO sWUfT3ENrUCIwIj29l+2piqnLpxClNx0FyXxJXeQ+VT939rXN36N82ea+3Tu/NlCKoL4a6 J5uPN/5RePayYCzNi4s6UmGu85hR+iAZbqz+sEIka+8gZmQ7dP9ESt4mxyjzazqg0b8+Xq eL7jr4ObED4E+q/rH3EoRIiYePtfSXpRTAmcDNciR2mJc7w5cpZgffygj96qr61aKyzfAy qDD1xLfwWib7vuRwmibf6cYQc/c1XxEeX5yUVCP9TvtQBCJuUUPQE2i8GQJilA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4gRp5K56hMzgCd for ; Fri, 29 May 2026 16:01:57 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 36111 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Fri, 29 May 2026 16:01:57 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Olivier Certner Subject: git: 4c98f7a0025e - main - MAC/do: Serialize installing/modifying some jail's configuration List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: olce X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 4c98f7a0025eea550ecf6f93f818cd03c6ac0fd5 Auto-Submitted: auto-generated Date: Fri, 29 May 2026 16:01:57 +0000 Message-Id: <6a19b875.36111.367fa35d@gitrepo.freebsd.org> The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=4c98f7a0025eea550ecf6f93f818cd03c6ac0fd5 commit 4c98f7a0025eea550ecf6f93f818cd03c6ac0fd5 Author: Olivier Certner AuthorDate: 2026-04-29 18:32:56 +0000 Commit: Olivier Certner CommitDate: 2026-05-29 15:36:29 +0000 MAC/do: Serialize installing/modifying some jail's configuration See the immediately preceding commit for explanations on what this is fixing. When setting 'mac.do' to 'inherit' on a jail with 'mac.do.rules' and 'mac.do.exec_paths' also specified in the same call, ensure that the check that these passed parameters are the same as those to be inherited is atomic with respect to enabling the inheritance (i.e., removing the jail's 'struct conf' object). (See previous commit "MAC/do: Fix the recent logic to set jail parameters, make it more tolerant" as for why this check exists.) Because we currently only modify a single configuration object per transaction, we introduce the parse_and_commit_conf() wrapper around parse_and_set_conf() to remove duplicated code that would ensue from calling the latter directly, namely, releasing the 'mac_do_rwl' lock and freeing the old configuration object (if any). Taking the 'mac_do_rwl' lock for writing as a way to freeze all accesses to mac_do(4) configurations was deemed too thin an operation to be worth wrapping. Reviewed by: bapt (older version) 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 --- sys/security/mac_do/mac_do.c | 99 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 76 insertions(+), 23 deletions(-) diff --git a/sys/security/mac_do/mac_do.c b/sys/security/mac_do/mac_do.c index 8065ff4a0e47..c30ece0a0794 100644 --- a/sys/security/mac_do/mac_do.c +++ b/sys/security/mac_do/mac_do.c @@ -1404,10 +1404,10 @@ set_conf(struct prison *const pr, struct conf *const conf) * * Destroys the 'osd_jail_slot' slot of the passed jail. */ -static void -remove_conf(struct prison *const pr) +static struct conf * +remove_conf_locked(struct prison *const pr) { - set_conf(pr, NULL); + return (set_conf_locked(pr, NULL, NULL)); } static struct conf * @@ -1481,11 +1481,15 @@ clone_exec_paths(struct exec_paths *const dst, * parameters are copied from those of the passed model configuration (which is * expected to be the currently applicable configuration, i.e., that of the * closest ancestor jail that has one). + * + * 'mac_do_rml' needs to be write-locked (and stays so). 'old_conf' serves to + * return, on no error, the old configuration with a reference (which must be + * eventually freed). */ static int parse_and_set_conf(struct prison *const pr, const char *const rules_string, const char *const exec_paths_string, const struct conf *const model_conf, - struct parse_error **const parse_error) + struct conf **const old_conf, struct parse_error **const parse_error) { struct conf *const conf = new_conf(); int error = 0; @@ -1511,7 +1515,8 @@ parse_and_set_conf(struct prison *const pr, const char *const rules_string, clone_exec_paths(&conf->exec_paths, &model_conf->exec_paths); - set_conf(pr, conf); + MPASS(error == 0); + *old_conf = set_conf_locked(pr, conf, osd_reserve(osd_jail_slot)); MPASS(error == 0 && *parse_error == NULL); out: @@ -1522,6 +1527,30 @@ error: goto out; } +/* + * Calls parse_and_set_conf() and closes the current configuration transaction. + * + * Closes the transaction by unlocking 'mac_do_rml' and releasing the old + * configuration returned by parse_and_set_conf(). + */ +static int +parse_and_commit_conf(struct prison *const pr, const char *const rules_string, + const char *const exec_paths_string, const struct conf *const model_conf, + struct parse_error **const parse_error) +{ + struct conf *old_conf; + int error; + + error = parse_and_set_conf(pr, rules_string, exec_paths_string, + model_conf, &old_conf, parse_error); + rm_wunlock(&mac_do_rml); + + if (error == 0 && old_conf != NULL) + drop_conf(old_conf); + return (error); +} + + static int mac_do_sysctl_rules(SYSCTL_HANDLER_ARGS) { @@ -1531,14 +1560,23 @@ mac_do_sysctl_rules(SYSCTL_HANDLER_ARGS) struct parse_error *parse_error = NULL; int error; - conf = find_conf(pr, NULL); + if (req->newptr != NULL) { + rm_wlock(&mac_do_rml); + conf = find_conf_locked(pr, NULL); + } else + conf = find_conf(pr, NULL); strlcpy(buf, conf->rules.string, MAX_RULE_STRING_SIZE); error = sysctl_handle_string(oidp, buf, MAX_RULE_STRING_SIZE, req); - if (error != 0 || req->newptr == NULL) + if (req->newptr == NULL) + goto out; + if (error != 0) { + rm_wunlock(&mac_do_rml); goto out; + } - error = parse_and_set_conf(pr, buf, NULL, conf, &parse_error); + /* Unlocks 'mac_do_rml'. */ + error = parse_and_commit_conf(pr, buf, NULL, conf, &parse_error); if (error != 0) { if (print_parse_error) printf("MAC/do: Parse error at index %zu: %s\n", @@ -1571,14 +1609,23 @@ mac_do_sysctl_exec_paths(SYSCTL_HANDLER_ARGS) struct parse_error *parse_error = NULL; int error; - conf = find_conf(pr, NULL); + if (req->newptr != NULL) { + rm_wlock(&mac_do_rml); + conf = find_conf_locked(pr, NULL); + } else + conf = find_conf(pr, NULL); strlcpy(buf, conf->exec_paths.exec_paths_str, MAX_EXEC_PATHS_SIZE); error = sysctl_handle_string(oidp, buf, MAX_EXEC_PATHS_SIZE, req); - if (error != 0 || req->newptr == NULL) + if (req->newptr == NULL) goto out; + if (error != 0) { + rm_wunlock(&mac_do_rml); + goto out; + } - error = parse_and_set_conf(pr, NULL, buf, conf, &parse_error); + /* Unlocks 'mac_do_rml'. */ + error = parse_and_commit_conf(pr, NULL, buf, conf, &parse_error); if (error != 0) { if (print_parse_error) printf("MAC/do: Parse error at index %zu: %s\n", @@ -1807,14 +1854,17 @@ mac_do_jail_set(void *obj, void *data) } if (jsys == JAIL_SYS_INHERIT) { + struct conf *old_conf = NULL; + error = 0; + rm_wlock(&mac_do_rml); if (!absent_or_empty_rules || !absent_or_empty_exec_paths) { /* * Some values specified. Check that they match the * ones we are going to inherit. */ - model_conf = find_conf(pr->pr_parent, NULL); + model_conf = find_conf_locked(pr->pr_parent, NULL); if (strcmp(model_conf->rules.string, rules_string) != 0) { error = EINVAL; @@ -1838,23 +1888,25 @@ mac_do_jail_set(void *obj, void *data) } if (error == 0) - /* - * There's no TOCTOU problem here as the removal of the - * current jail's configuration commutes with changing - * the inherited configuration we checked against. - */ - remove_conf(pr); + old_conf = remove_conf_locked(pr); + + rm_wunlock(&mac_do_rml); + + if (old_conf != NULL) + drop_conf(old_conf); return (error); } model_conf = NULL; + /* Freeze configuration accesses. */ + rm_wlock(&mac_do_rml); switch (jsys) { case JAIL_SYS_DISABLE: /* * mac_do(4) is disabled iff one of the parameter's string is - * empty. The parse_and_set_conf() call below treats passing + * empty. The parse_and_commit_conf() call below treats passing * NULL for a parameter as a flag to copy its value from the * relevant ancestor jail's configuration, so we have to watch * for the final result having an empty parameter if no @@ -1876,7 +1928,7 @@ mac_do_jail_set(void *obj, void *data) * values). */ if (rules_string == NULL || exec_paths_string == NULL) - model_conf = find_conf(pr, NULL); + model_conf = find_conf_locked(pr, NULL); /* If both are absent, we have to examine if, in the * currently applicable configuration, one of the * parameters, which we are going to copy, is @@ -1894,16 +1946,17 @@ mac_do_jail_set(void *obj, void *data) break; case JAIL_SYS_NEW: - /* See the comment before the call to find_conf() above. */ + /* See the comment before the same test above. */ if (rules_string == NULL || exec_paths_string == NULL) - model_conf = find_conf(pr, NULL); + model_conf = find_conf_locked(pr, NULL); break; default: __assert_unreachable(); } - error = parse_and_set_conf(pr, rules_string, exec_paths_string, + /* Unlocks 'mac_do_rml'. */ + error = parse_and_commit_conf(pr, rules_string, exec_paths_string, model_conf, &parse_error); if (model_conf != NULL) drop_conf(model_conf);