From nobody Fri May 29 16:01:37 2026 X-Original-To: dev-commits-src-all@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 4gRp4y2xC9z6flF5 for ; Fri, 29 May 2026 16:01:38 +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 4gRp4x5zfJz3HPg for ; Fri, 29 May 2026 16:01:37 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1780070497; 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=bKqAWvH+FBHFg8Q7AkA35xHLXEKaDPr0cMRnKk9DbWw=; b=YdoexmHeSGfJ6RJh+grxN00kYEeOB9r9mmme26b80/z69wvTxWijGvgaZB9SS1LaHgijqz efasnaQLY17gq6Om5j5N9nxlbx8h0rEViJhn+MmbkC0OZxor3OTAGwJkZMHqHwiC7Dsw5/ mHj0W6aiPLb7Zqak8fyGfvrXSFuFjuZLlBEtBzlk2d7R2gqgp0anl60stuQ2B2U/+o1znM 3aZiY6u/gw2q1MqVzI7aYGe4ziPVXDheQtbopqiPBAiwjD72C4Ik3xjS2qNTmGL2hscyEb qHDQ0o5mjfc3KG7VV0/htl5OHOP5eHYvEYr5VhMvj6/9pqypYtOLD/TF2lAnEA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1780070497; a=rsa-sha256; cv=none; b=BlFn1KE6n6YyUsH4ixAGebW8pVvllXHTAdxGtIp40CrWLVh3v7DKqRkQOAFZJWpEIsOgLt C1AcjC5w8zgIynbZCPZd7N5dRYzacsss8eGUjye2jMEBziFxKQs7rN9OuEv9HWw4X4Iky3 sqLF4VhSXK6S1jrMQgIiEvAjqqu4Z00u4Xd4poktiQK3l5hnT3fQ8u4uQi3P3ZINStwXCS kW0JMYW5ivh1f1dSYLOpjH/I4z1XEIMensN8bDbk+YtWyPiaLGQuOrj/7cvb4xeTh/ia4V EGBJoN3vC9jeSQwoC3vMTr+MDWJ8iyz/vqbQ1+PdgSZujAr4l7L7qxZG0Qlgeg== 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=1780070497; 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=bKqAWvH+FBHFg8Q7AkA35xHLXEKaDPr0cMRnKk9DbWw=; b=FvPebxKrj5Y/9NnmbGaao+PZx3SUrlAy3Wry4TVBYc6FXD79j+ZfBHpaqJyRdqCOi3jx/y /mGJ6rXeTBky9geKbrRCOfUOrvE+93cOm2BMycefR4cHZipjKk3FvmIJp5Z2jQT4behEKU XTcqTO5QHv/vyYa0l0Q5bwtzzdKSgiU3SVS8gbLjE+kQ4AymTh1YJ9uUnJELpYAkdug2l8 /rGqnJcX4Kp7ff3nDD6gusYY/sXXwS1Bbj5FyO3sRlgNVDTYMUrRHFm2AptIGGcoDjS4/J tduQcj7k33JwSEY1IylMWxgcbKjbR0h38pEEnUR1CSqaNbs+tj1y2wD9TbbLqw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4gRp4x5NrFzgQw for ; Fri, 29 May 2026 16:01:37 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 36524 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Fri, 29 May 2026 16:01:37 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Olivier Certner Subject: git: 4e4cf18b85cc - main - MAC/do: find_conf(): Return configuration with a true reference List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@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: 4e4cf18b85cc51f41bcae20114f9c0e7b69f76e0 Auto-Submitted: auto-generated Date: Fri, 29 May 2026 16:01:37 +0000 Message-Id: <6a19b861.36524.14a4bcac@gitrepo.freebsd.org> The branch main has been updated by olce: URL: https://cgit.FreeBSD.org/src/commit/?id=4e4cf18b85cc51f41bcae20114f9c0e7b69f76e0 commit 4e4cf18b85cc51f41bcae20114f9c0e7b69f76e0 Author: Olivier Certner AuthorDate: 2026-04-27 14:39:15 +0000 Commit: Olivier Certner CommitDate: 2026-05-29 15:22:09 +0000 MAC/do: find_conf(): Return configuration with a true reference In addition to the applicable configuration, find_conf() was returning a pointer to the actual jail holding the configuration object, with that jail's mutex locked in order to ensure liveness of the returned configuration (if we wouldn't, a concurrent thread modifying the jail's configuration could destroy this configuration object underneath us). But: 1. Ensuring configuration stability by owning the holding jail's mutex requires callers to either keep that mutex locked for a longer period of time than just accessing the corresponding 'struct prison' (in general, bad for concurrency with other operations involving jails) or to perform an additional dance to acquire a real reference in case the jail's mutex, for some reason (in general, LORs or acquiring a sleepable lock) must be dropped before use. 2. Most code does not actually need to know which jail holds the applicable configuration but for unlocking the jail's mutex. Having to deal with the jail holding the configuration can cause confusion about which jail (the current one, or the one holding the configuration) must be used (and actually did in the very initial version of MAC/do, which had a serious flaw as a consequence). So, do not keep a lock on the holding jail. Instead, ensure configuration stability by always acquiring a true reference from the start and passing it to the caller. Those callers not doing the dance mentioned above now need to free it when finished (but this need replaces the one to unlock the prison). Additionally, only return the holding jail if explicitly requested by the caller. mac_do_jail_get() is currently the only caller that needs it, in order to be able to reliably report if the configuration is inherited. 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 | 83 +++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 43 deletions(-) diff --git a/sys/security/mac_do/mac_do.c b/sys/security/mac_do/mac_do.c index cb351cef15ce..a2259ed777ae 100644 --- a/sys/security/mac_do/mac_do.c +++ b/sys/security/mac_do/mac_do.c @@ -1178,14 +1178,19 @@ drop_conf(struct conf *const conf) /* * Find configuration applicable to the passed prison. * - * Returns the applicable configuration (and never NULL). 'pr' must be - * unlocked. 'aprp' is set to the (ancestor) prison holding these, and it must - * be unlocked once the caller is done accessing the configuration. '*aprp' is - * equal to 'pr' if and only if the current jail has its own specific - * configuration. + * Returns the applicable configuration (which always exists), with an + * additional reference that must be freed by the caller. 'pr' must not be + * locked. + * + * The applicable configuration is that of the closest ancestor prison + * (including itself) of the passed prison that actually has a 'struct conf' + * associated to it. + * + * If 'hpr' is not NULL, it is used to return a pointer to the (unlocked) prison + * holding the applicable configuration. */ static struct conf * -find_conf(struct prison *const pr, struct prison **const aprp) +find_conf(struct prison *const pr, struct prison **const hpr) { struct prison *cpr, *ppr; struct conf *conf; @@ -1211,7 +1216,11 @@ find_conf(struct prison *const pr, struct prison **const aprp) cpr = ppr; } - *aprp = cpr; + hold_conf(conf); + prison_unlock(cpr); + + if (hpr != NULL) + *hpr = cpr; return (conf); } @@ -1386,7 +1395,6 @@ static int parse_and_set_conf(struct prison *pr, const char *rules_string, const char *exec_paths_string, struct parse_error **parse_error) { - struct prison *ppr = NULL; struct conf *applicable_conf = NULL; struct conf *conf; int error = 0; @@ -1395,11 +1403,8 @@ parse_and_set_conf(struct prison *pr, const char *rules_string, need_applicable_conf = (rules_string == NULL || rules_string[0] == '\0' || exec_paths_string == NULL || exec_paths_string[0] == '\0'); - if (need_applicable_conf) { - applicable_conf = find_conf(pr, &ppr); - hold_conf(applicable_conf); - prison_unlock(ppr); - } + if (need_applicable_conf) + applicable_conf = find_conf(pr, NULL); conf = alloc_conf(); @@ -1437,22 +1442,20 @@ static int mac_do_sysctl_rules(SYSCTL_HANDLER_ARGS) { char *const buf = malloc(MAX_RULE_STRING_SIZE, M_MAC_DO, M_WAITOK); - struct prison *const td_pr = req->td->td_ucred->cr_prison; - struct prison *pr; + struct prison *const pr = req->td->td_ucred->cr_prison; struct conf *conf; struct parse_error *parse_error = NULL; int error; - conf = find_conf(td_pr, &pr); + conf = find_conf(pr, NULL); strlcpy(buf, conf->rules.string, MAX_RULE_STRING_SIZE); - prison_unlock(pr); error = sysctl_handle_string(oidp, buf, MAX_RULE_STRING_SIZE, req); if (error != 0 || req->newptr == NULL) goto out; /* Set our prison's rules, not that of the jail we inherited from. */ - error = parse_and_set_conf(td_pr, buf, NULL, &parse_error); + error = parse_and_set_conf(pr, buf, NULL, &parse_error); if (error != 0) { if (print_parse_error) printf("MAC/do: Parse error at index %zu: %s\n", @@ -1461,6 +1464,7 @@ mac_do_sysctl_rules(SYSCTL_HANDLER_ARGS) } out: + drop_conf(conf); free(buf, M_MAC_DO); return (error); } @@ -1479,21 +1483,19 @@ static int mac_do_sysctl_exec_paths(SYSCTL_HANDLER_ARGS) { char *const buf = malloc(MAX_EXEC_PATHS_SIZE, M_MAC_DO, M_WAITOK); - struct prison *const td_pr = req->td->td_ucred->cr_prison; - struct prison *pr; + struct prison *const pr = req->td->td_ucred->cr_prison; struct conf *conf; struct parse_error *parse_error = NULL; int error; - conf = find_conf(td_pr, &pr); + conf = find_conf(pr, NULL); strlcpy(buf, conf->exec_paths.exec_paths_str, MAX_EXEC_PATHS_SIZE); - prison_unlock(pr); error = sysctl_handle_string(oidp, buf, MAX_EXEC_PATHS_SIZE, req); if (error != 0 || req->newptr == NULL) goto out; - error = parse_and_set_conf(td_pr, NULL, buf, &parse_error); + error = parse_and_set_conf(pr, NULL, buf, &parse_error); if (error != 0) { if (print_parse_error) printf("MAC/do: Parse error at index %zu: %s\n", @@ -1502,6 +1504,7 @@ mac_do_sysctl_exec_paths(SYSCTL_HANDLER_ARGS) } out: + drop_conf(conf); free(buf, M_MAC_DO); return (error); } @@ -1527,20 +1530,19 @@ mac_do_jail_create(void *obj, void *data) static int mac_do_jail_get(void *obj, void *data) { - struct prison *ppr, *const pr = obj; + struct prison *const pr = obj; struct vfsoptlist *const opts = data; - struct conf *conf; - struct rules *rules; - struct exec_paths *exec_paths; + struct prison *hpr_out; + struct conf *const applicable_conf = find_conf(pr, &hpr_out); + const struct prison *const hpr = hpr_out; + const struct rules *const rules = &applicable_conf->rules; + const struct exec_paths *const exec_paths = &applicable_conf->exec_paths; int jsys, error; - conf = find_conf(pr, &ppr); - rules = &conf->rules; - exec_paths = &conf->exec_paths; - - jsys = pr == ppr ? + jsys = hpr == pr ? (STAILQ_EMPTY(&rules->head) ? JAIL_SYS_DISABLE : JAIL_SYS_NEW) : JAIL_SYS_INHERIT; + error = vfs_setopt(opts, "mac.do", &jsys, sizeof(jsys)); if (error != 0 && error != ENOENT) goto done; @@ -1556,7 +1558,7 @@ mac_do_jail_get(void *obj, void *data) error = 0; done: - prison_unlock(ppr); + drop_conf(applicable_conf); return (error); } @@ -2290,11 +2292,10 @@ mac_do_priv_grant(struct ucred *cred, int priv) static int check_proc(void) { + struct prison *const pr = curproc->p_ucred->cr_prison; char *path, *to_free; struct conf *conf; struct exec_paths *exec_paths; - struct prison *td_pr; - struct prison *pr; int error; /* @@ -2319,8 +2320,7 @@ check_proc(void) return (EPERM); error = EPERM; - td_pr = curproc->p_ucred->cr_prison; - conf = find_conf(td_pr, &pr); + conf = find_conf(pr, NULL); exec_paths = &conf->exec_paths; for (int i = 0; i < exec_paths->exec_path_count; i++) @@ -2329,8 +2329,7 @@ check_proc(void) break; } - prison_unlock(pr); - + drop_conf(conf); free(to_free, M_TEMP); return (error); } @@ -2338,7 +2337,7 @@ check_proc(void) static void mac_do_setcred_enter(void) { - struct prison *pr; + struct prison *const pr = curproc->p_ucred->cr_prison; struct mac_do_setcred_data * data; struct conf *conf; int error; @@ -2362,9 +2361,7 @@ mac_do_setcred_enter(void) /* * Find the currently applicable rules. */ - conf = find_conf(curproc->p_ucred->cr_prison, &pr); - hold_conf(conf); - prison_unlock(pr); + conf = find_conf(pr, NULL); /* * Setup thread data to be used by other hooks.