From nobody Sun Jul 5 02:54:59 2026 X-Original-To: dev-commits-src-branches@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 4gtBtC48DQz6dd5G for ; Sun, 05 Jul 2026 02:54:59 +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 "YR1" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4gtBtC3lYPz4Nh3 for ; Sun, 05 Jul 2026 02:54:59 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1783220099; 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=CwYt+5lHAJvVQTeOHgVKsZ1o7rkkxVY3OE7YRgiYPTI=; b=PkaNMAHajz9IqNOXFRQiaD9r3UwiYeJMYpiwtojW7lIvBkzzSvkYckiRryh4o87fypOcA6 cfZG8Dlyvzz8LNLtZO6wnRvtHwLR5GPs5AAYk2JsQ23KdJcYEBTUr80YdTEaaHdAttRMK1 aBcy3wvBRpq/Wlf6dLpAos7RWE5I7zlwapBrNIT58oFd6PvsjTGLvfdjgQQ1hjE5Sunxi7 TabEkPhQ4KWNV1M/EVF05sbmBe1GeFMhJBUV7y7qyoZvRc9FSRlL03X8OkdTZ0vBFf4T6I 9uN+jOvhzlf2ei2d3DmvvhiGDIfKZlXHfXvoTzrbexCVNw7Mxnn2/gBOD+k3mw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1783220099; a=rsa-sha256; cv=none; b=I9VtMug4KlzQgEEhipGy7fm/boVmP8MqANXw/cG9IaXI/Tq6bXORamdLa7/qJMrtGzxnwm wqIBhE/9JE1H9OJMo3UOf20jSyr0ImslozpY9OvMqXlsF3TiG4oUIkspSY689FY8y5z0QH 5xUyI+p9JwLUUuUXaHdNvpM0aQpZX975OouB0cl43fxma5LfevFcq+VNQF99M8kpiyGPLM 3Pj457YkGN87CCXJ16dF/axd8coy3unz3VcwDEm3llbVIEdMLcCFSqv643pVc8bXVuFAef F6xHrzY6xKYXzWqLlGHHqMPRQL2svOSMeldCtmkYxwte6ZJU40Y38EhNw758Ig== 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=1783220099; 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=CwYt+5lHAJvVQTeOHgVKsZ1o7rkkxVY3OE7YRgiYPTI=; b=iJUMNuduss7o2Y0k7TOrYGwKc/JsHlpyDQJxEFxKjm6iXrfjYwz0yPmT1w7rhgvoZA57jh TvEZW6xeVs7G04I9LIv408nZr3apM9kyW0SmG0C0zlQVYOrIgNk5D3j4NkQTzyB5Fj9X3a jshJ9GdnrI5Xoe37Koy7yGSYRKpvNBjwGo+udKJHT9/h1OBdbaIzOvqGVFzs6BXiWfg3HA AgG8dJ94W0jyDe1KXyd2jYH4EfiruhabBagIDD9bphQ6WMlykQpbw7k4p8m9LUcVj98/Rq 5ns/7mE1ycqrrYZ1SeUwi8mw6xa0o62rAJRV7EQWXWNxoEjWHO1Ow/z1NiasEA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4gtBtC2Dzwz10Vg for ; Sun, 05 Jul 2026 02:54:59 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 44930 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Sun, 05 Jul 2026 02:54:59 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Jamie Gritton Subject: git: a4df9e3efa30 - stable/15 - jail: prevent a race between jail_attach in different threads List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-branches@freebsd.org Sender: owner-dev-commits-src-branches@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: jamie X-Git-Repository: src X-Git-Refname: refs/heads/stable/15 X-Git-Reftype: branch X-Git-Commit: a4df9e3efa30d2486776a530e6a3548bc74d7b98 Auto-Submitted: auto-generated Date: Sun, 05 Jul 2026 02:54:59 +0000 Message-Id: <6a49c783.44930.5d4a0855@gitrepo.freebsd.org> The branch stable/15 has been updated by jamie: URL: https://cgit.FreeBSD.org/src/commit/?id=a4df9e3efa30d2486776a530e6a3548bc74d7b98 commit a4df9e3efa30d2486776a530e6a3548bc74d7b98 Author: Jamie Gritton AuthorDate: 2026-07-02 22:48:07 +0000 Commit: Jamie Gritton CommitDate: 2026-07-05 02:54:41 +0000 jail: prevent a race between jail_attach in different threads Attaching to a jail changes its root directory and its process credentials. These operations both require unlocking the jail, and also need allprison_lock unlocked. That means that if two threads are trying to attach to different jails at the same time, it's possible for the process to end up with one jail's root directory but the other jail's credentials. Solve this by forcing the process into single-threaded mode during system calls that attach to a jail (jail_attach, jail_attach_jd, and sometimes jail_set). Reviewed by: kib, markj Differential Revision: https://reviews.freebsd.org/D57858 (cherry picked from commit d4e0f4dab2d7f4de46bb79db1ca7e6e8a2e34746) --- sys/kern/kern_jail.c | 72 +++++++++++--- tests/sys/kern/Makefile | 2 + tests/sys/kern/jail_thread.c | 231 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 294 insertions(+), 11 deletions(-) diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c index 8a6c3e1d3fd0..6b0ae8fbb8ce 100644 --- a/sys/kern/kern_jail.c +++ b/sys/kern/kern_jail.c @@ -155,6 +155,8 @@ int lastdeadid = 0; static int get_next_prid(struct prison **insprp); static int get_next_deadid(struct prison **insprp); static int do_jail_attach(struct thread *td, struct prison *pr, int *drflagsp); +static int prison_attach_thread_single(struct thread *td); +static void prison_attach_thread_single_end(struct thread *td); static void prison_complete(void *context, int pending); static void prison_deref(struct prison *pr, int flags); static void prison_deref_kill(struct prison *pr, struct prisonlist *freeprison); @@ -1045,7 +1047,13 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) if ((flags & (JAIL_USE_DESC | JAIL_AT_DESC)) == (JAIL_USE_DESC | JAIL_AT_DESC)) return (EINVAL); - prison_hold(mypr); + + /* Only let a single thread in the process try to attach at a time. */ + if (flags & JAIL_ATTACH) { + error = prison_attach_thread_single(td); + if (error != 0) + return (error); + } #ifdef INET ip4 = NULL; @@ -1056,6 +1064,7 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) g_path = NULL; jfp_out = NULL; jfd_out = -1; + prison_hold(mypr); /* * Check all the parameters before committing to anything. Not all * errors can be caught early, but we may as well try. Also, this @@ -2342,6 +2351,8 @@ kern_jail_set(struct thread *td, struct uio *optuio, int flags) if (opts != NULL) vfs_freeopts(opts); prison_free(mypr); + if (flags & JAIL_ATTACH) + prison_attach_thread_single_end(td); return (error); } @@ -2940,6 +2951,10 @@ sys_jail_attach(struct thread *td, struct jail_attach_args *uap) error = priv_check(td, PRIV_JAIL_ATTACH); if (error) return (error); + /* Only let a single thread in the process try to attach at a time. */ + error = prison_attach_thread_single(td); + if (error != 0) + return (error); sx_slock(&allprison_lock); drflags = PD_LIST_SLOCKED; @@ -2960,6 +2975,7 @@ sys_jail_attach(struct thread *td, struct jail_attach_args *uap) done: prison_deref(pr, drflags); + prison_attach_thread_single_end(td); return (error); } @@ -2975,6 +2991,11 @@ sys_jail_attach_jd(struct thread *td, struct jail_attach_jd_args *uap) struct ucred *jdcred; int drflags, error; + /* Only let a single thread in the process try to attach at a time. */ + error = prison_attach_thread_single(td); + if (error != 0) + return (error); + sx_slock(&allprison_lock); drflags = PD_LIST_SLOCKED; pr = NULL; @@ -2997,6 +3018,7 @@ sys_jail_attach_jd(struct thread *td, struct jail_attach_jd_args *uap) done: prison_deref(pr, drflags); + prison_attach_thread_single_end(td); return (error); } @@ -3014,15 +3036,6 @@ do_jail_attach(struct thread *td, struct prison *pr, int *drflagsp) sx_assert(&allprison_lock, SX_LOCKED); KASSERT(prison_isvalid(pr), ("Attaching to invalid prison %p", pr)); - /* - * XXX: Note that there is a slight race here if two threads - * in the same privileged process attempt to attach to two - * different jails at the same time. It is important for - * user processes not to do this, or they might end up with - * a process root from one prison, but attached to the jail - * of another. - */ - /* * Note the caller's locking state, but gain and track our own * references. The caller will see that locks have been @@ -3105,7 +3118,7 @@ do_jail_attach(struct thread *td, struct prison *pr, int *drflagsp) e_unlock: VOP_UNLOCK(pr->pr_root); e_revert_osd: - /* Tell modules this thread is still in its old jail after all. */ + /* Tell modules this process is still in its old jail after all. */ if (!(drflags & (PD_LIST_SLOCKED | PD_LIST_XLOCKED))) { sx_slock(&allprison_lock); drflags |= PD_LIST_SLOCKED; @@ -3115,6 +3128,43 @@ do_jail_attach(struct thread *td, struct prison *pr, int *drflagsp) return (error); } +/* + * Only one thread in a process should try to attach to a jail, or + * they might end up with a process root from one prison, but attached + * to the jail of another. Enforce this by making the process run in + * single-threaded mode for the duration of the system call, which + * also prevents other related calls such as chroot. + */ +static int +prison_attach_thread_single(struct thread *td) +{ + struct proc *p; + int error; + + error = 0; + p = td->td_proc; + if ((atomic_load_int(&p->p_flag) & P_HADTHREADS) != 0) { + PROC_LOCK(p); + if (thread_single(p, SINGLE_BOUNDARY)) + error = ERESTART; + PROC_UNLOCK(p); + } + return (error); +} + +static void +prison_attach_thread_single_end(struct thread *td) +{ + struct proc *p; + + p = td->td_proc; + if ((atomic_load_int(&p->p_flag) & P_HADTHREADS) != 0) { + PROC_LOCK(p); + thread_single_end(p, SINGLE_BOUNDARY); + PROC_UNLOCK(p); + } +} + /* * Returns a locked prison instance, or NULL on failure. */ diff --git a/tests/sys/kern/Makefile b/tests/sys/kern/Makefile index 4fe194771f56..03aca4f19a56 100644 --- a/tests/sys/kern/Makefile +++ b/tests/sys/kern/Makefile @@ -24,6 +24,7 @@ ATF_TESTS_C+= exterr_test ATF_TESTS_C+= fdgrowtable_test ATF_TESTS_C+= getdirentries_test ATF_TESTS_C+= jail_lookup_root +ATF_TESTS_C+= jail_thread ATF_TESTS_C+= jaildesc ATF_TESTS_C+= inotify_test ATF_TESTS_C+= kill_zombie @@ -96,6 +97,7 @@ PROGS+= sendfile_helper LIBADD.aslr+= util LIBADD.copy_file_range+= md LIBADD.jail_lookup_root+= jail util +LIBADD.jail_thread+= jail pthread LIBADD.jaildesc+= kvm pthread CFLAGS.sys_getrandom+= -I${SRCTOP}/sys/contrib/zstd/lib LIBADD.sys_getrandom+= zstd diff --git a/tests/sys/kern/jail_thread.c b/tests/sys/kern/jail_thread.c new file mode 100644 index 000000000000..1af25f12386d --- /dev/null +++ b/tests/sys/kern/jail_thread.c @@ -0,0 +1,231 @@ +/*- + * SPDX-License-Identifier: BSD-2-Clause + * + * Copyright (c) 2026 James Gritton + */ + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define NJAILS 3 /* One master jail and two to race. */ +#define NROUNDS 10000 /* Number of attempts to make race happen. */ + +struct jailinfo { + int jfd; + ino_t ino; + char path[MAXPATHLEN]; +}; + +static pthread_barrier_t barrier; +static struct jailinfo jinfo[NJAILS]; + +/* Attach a thread to a jail with jail_attach_jd. */ +static void * +thread_jail_attach_jd(void *arg) +{ + int error; + + /* + * Synchronize to get as close as possible to the same time, + * then attach to a jail. + */ + error = pthread_barrier_wait(&barrier); + ATF_REQUIRE_MSG(error == 0 || error == PTHREAD_BARRIER_SERIAL_THREAD, + "pthread_barrier_wait: %s", strerror(errno)); + ATF_REQUIRE_MSG(jail_attach_jd(jinfo[(size_t)arg].jfd) == 0, + "jail_attach_jd: %s", strerror(errno)); + return (NULL); +} + +/* Attach a thread to a jail with jail_setv. */ +static void * +thread_jail_setv(void *arg) +{ + int error; + char jdescstr[16]; + + error = pthread_barrier_wait(&barrier); + ATF_REQUIRE_MSG(error == 0 || error == PTHREAD_BARRIER_SERIAL_THREAD, + "pthread_barrier_wait: %s", strerror(errno)); + snprintf(jdescstr, sizeof(jdescstr), "%d", jinfo[(size_t)arg].jfd); + ATF_REQUIRE_MSG(jail_setv(JAIL_UPDATE | JAIL_ATTACH | JAIL_USE_DESC, + "desc", jdescstr, NULL) > 0, + "jail_setv: %s", jail_errmsg[0] ? jail_errmsg : strerror(errno)); + return (NULL); +} + +/* Attach a thread to a jail with chroot. */ +static void * +thread_chroot(void *arg) +{ + int error; + + /* This is a race between jail_attach and chroot. */ + if ((size_t)arg > 1) + return thread_jail_attach_jd(arg); + error = pthread_barrier_wait(&barrier); + ATF_REQUIRE_MSG(error == 0 || error == PTHREAD_BARRIER_SERIAL_THREAD, + "pthread_barrier_wait: %s", strerror(errno)); + ATF_REQUIRE_MSG(chroot(jinfo[(size_t)arg].path) == 0 || errno == ENOENT, + "chroot: %s", strerror(errno)); + return (NULL); +} + +static void +thread_attach_test(void *(*thread_handler)(void*), const char *jail_name, + const char *syscall_name, bool jail_reset) +{ + int ri, spn, mixed_jails; + size_t ji, ti, ji_hostname, ji_ino, jail_namelen; + char *cwd; + struct stat st; + char jnamestr[64], jdescstr[16]; + pthread_t threads[NJAILS]; + + if (jinfo[0].jfd == 0) { + /* Start with a master jail, so we can return to real root. */ + jdescstr[0] = '\0'; + ATF_REQUIRE_MSG(jail_setv(JAIL_CREATE | JAIL_OWN_DESC, + "name", jail_name, + "path", "/", + "desc", jdescstr, + "persist", "true", + NULL) > 0, + "jail_setv jail 0: %s", + jail_errmsg[0] ? jail_errmsg : strerror(errno)); + jinfo[0].jfd = strtol(jdescstr, NULL, 10); + /* Make enough jails to cause contention. */ + cwd = getcwd(NULL, MAXPATHLEN); + ATF_REQUIRE_MSG(cwd != NULL, "getcwd: %s", strerror(errno)); + for (ji = 1; ji < NJAILS; ++ji) { + snprintf(jnamestr, sizeof(jnamestr), + "%s%zu", jail_name, ji); + spn = snprintf(jinfo[ji].path, MAXPATHLEN, + "%s/jail%zu", cwd, ji); + ATF_REQUIRE_MSG((size_t)spn < MAXPATHLEN, + "snprintf exceeded MAXPATHLEN: %d", spn); + ATF_REQUIRE_MSG( + mkdir(jinfo[ji].path, 0755) == 0 || errno == EEXIST, + "mkdir %s: %s", jinfo[ji].path, strerror(errno)); + ATF_REQUIRE_MSG(stat(jinfo[ji].path, &st) == 0, + "stat %s: %s", jinfo[ji].path, strerror(errno)); + jinfo[ji].ino = st.st_ino; + jdescstr[0] = '\0'; + ATF_REQUIRE_MSG(jail_setv(JAIL_CREATE | JAIL_OWN_DESC, + "name", jnamestr, + "host.hostname", jnamestr, + "path", jinfo[ji].path, + "desc", jdescstr, + "persist", "true", + NULL) > 0, + "jail_setv: %s", + jail_errmsg[0] ? jail_errmsg : strerror(errno)); + jinfo[ji].jfd = strtol(jdescstr, NULL, 10); + } + } else + ATF_REQUIRE_MSG(jail_attach_jd(jinfo[0].jfd) == 0, + "jail_attach_jd: %s", strerror(errno)); + + /* Check the different system calls that can race. */ + jail_namelen = strlen(jail_name); + mixed_jails = 0; + for (ri = 0; ri < NROUNDS; ++ri) { + ATF_REQUIRE_MSG( + pthread_barrier_init(&barrier, NULL, NJAILS - 1) == 0, + "pthread_barrier_init: %s", strerror(errno)); + for (ti = 1; ti < NJAILS; ++ti) + ATF_REQUIRE_MSG( + pthread_create(&threads[ti], NULL, thread_handler, + (void*)ti) == 0, + "pthread_create: %s", strerror(errno)); + for (ti = 1; ti < NJAILS; ++ti) + ATF_REQUIRE_MSG( + pthread_join(threads[ti], NULL) == 0, + "pthread_join: %s", strerror(errno)); + ATF_REQUIRE_MSG(pthread_barrier_destroy(&barrier) == 0, + "pthread_barrier_destroy: %s", strerror(errno)); + /* + * Find the current jail from the hostname, and also + * by the root inode. They should be the same. + */ + ATF_REQUIRE_MSG( + gethostname(jnamestr, sizeof(jnamestr)) == 0, + "gethostname: %s", strerror(errno)); + ATF_REQUIRE_MSG(strncmp(jnamestr, jail_name, jail_namelen) == 0, + "unexpected jail hostname %s", jnamestr); + ji_hostname = strtol(jnamestr + jail_namelen, NULL, 10); + ATF_REQUIRE_MSG(stat("/", &st) == 0, + "stat /: %s", strerror(errno)); + for (ji_ino = 1; ji_ino < NJAILS; ++ji_ino) + if (jinfo[ji_ino].ino == st.st_ino) + break; + ATF_REQUIRE_MSG(ji_ino < NJAILS, + "unexpected jail root inode %lu", + (unsigned long)st.st_ino); + mixed_jails += ji_hostname != ji_ino; + /* Reset to the master jail, required for chroot. */ + if (jail_reset) + ATF_REQUIRE_MSG(jail_attach_jd(jinfo[0].jfd) == 0, + "jail_attach_jd: %s", strerror(errno)); + } + /* It's an error if any of the rounds had a mismatch. */ + ATF_REQUIRE_MSG(mixed_jails == 0, + "%d of %d %s races with different root and " + "credentials", mixed_jails, NROUNDS, syscall_name); +} + +#define JAIL_NAME "jail_thread_attach_test" + + +ATF_TC(jail_thread_attach); +ATF_TC_HEAD(jail_thread_attach, tc) +{ + atf_tc_set_md_var(tc, "require.user", "root"); +} +ATF_TC_BODY(jail_thread_attach, tc) +{ + thread_attach_test(thread_jail_attach_jd, + "jail_thread_attach_test", "jail_attach_jd", false); +} + +ATF_TC(jail_thread_attach_jail_set); +ATF_TC_HEAD(jail_thread_attach_jail_set, tc) +{ + atf_tc_set_md_var(tc, "require.user", "root"); +} +ATF_TC_BODY(jail_thread_attach_jail_set, tc) +{ + thread_attach_test(thread_jail_setv, + "jail_thread_attach_test_jail_set", "jail_set", false); +} + +ATF_TC(jail_thread_attach_chroot); +ATF_TC_HEAD(jail_thread_attach_chroot, tc) +{ + atf_tc_set_md_var(tc, "require.user", "root"); +} +ATF_TC_BODY(jail_thread_attach_chroot, tc) +{ + thread_attach_test(thread_chroot, + "jail_thread_attach_test_chroot", "chroot", true); +} + +ATF_TP_ADD_TCS(tp) +{ + ATF_TP_ADD_TC(tp, jail_thread_attach); + ATF_TP_ADD_TC(tp, jail_thread_attach_jail_set); + ATF_TP_ADD_TC(tp, jail_thread_attach_chroot); + return (atf_no_error()); +}