Page MenuHomeFreeBSD
Files F170806511

D57858.id180787.diff
No OneTemporary
Actions

D57858.id180787.diff
View Options

diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
--- a/sys/kern/kern_jail.c
+++ b/sys/kern/kern_jail.c
@@ -1015,12 +1015,13 @@
#ifdef INET6
struct prison_ip *ip6;
#endif
+ struct proc *p;
struct vfsopt *opt;
struct vfsoptlist *opts;
struct prison *pr, *deadpr, *dinspr, *inspr, *mypr, *ppr, *tpr;
struct ucred *jdcred;
struct vnode *root;
- char *domain, *errmsg, *host, *name, *namelc, *p, *path, *uuid;
+ char *domain, *ep, *errmsg, *host, *name, *namelc, *path, *uuid;
char *g_path, *osrelstr;
struct bool_flags *bf;
struct jailsys_flags *jsf;
@@ -1061,8 +1062,26 @@
if ((flags & (JAIL_USE_DESC | JAIL_AT_DESC)) ==
(JAIL_USE_DESC | JAIL_AT_DESC))
return (EINVAL);
- prison_hold(mypr);
+ /*
+ * Make sure only one thread in the process is trying to
+ * attach to a jail, or they might end up with a process root
+ * from one prison, but attached to the jail of another. This
+ * is also checked in sys_jail_attach and sys_jail_attach_jd.
+ */
+ if (flags & JAIL_ATTACH) {
+ p = td->td_proc;
+ if ((p->p_flag & P_HADTHREADS) != 0) {
+ PROC_LOCK(p);
+ if (thread_single(p, SINGLE_BOUNDARY)) {
+ PROC_UNLOCK(p);
+ return (ERESTART);
+ }
+ PROC_UNLOCK(p);
+ }
+ }
+
+ prison_hold(mypr);
#ifdef INET
ip4 = NULL;
#endif
@@ -1528,8 +1547,8 @@
maybe_changed = false;
if (cuflags == JAIL_CREATE && jid == 0 && name != NULL) {
namelc = strrchr(name, '.');
- jid = strtoul(namelc != NULL ? namelc + 1 : name, &p, 10);
- if (*p != '\0')
+ jid = strtoul(namelc != NULL ? namelc + 1 : name, &ep, 10);
+ if (*ep != '\0')
jid = 0;
}
sx_xlock(&allprison_lock);
@@ -1989,8 +2008,8 @@
*/
if (namelc[0] == '\0')
snprintf(namelc = numbuf, sizeof(numbuf), "%d", jid);
- else if ((strtoul(namelc, &p, 10) != jid ||
- namelc[0] < '1' || namelc[0] > '9') && *p == '\0') {
+ else if ((strtoul(namelc, &ep, 10) != jid ||
+ namelc[0] < '1' || namelc[0] > '9') && *ep == '\0') {
error = EINVAL;
vfs_opterror(opts,
"name cannot be numeric (unless it is the jid)");
@@ -2416,6 +2435,11 @@
if (opts != NULL)
vfs_freeopts(opts);
prison_free(mypr);
+ if ((flags & JAIL_ATTACH) && (p->p_flag & P_HADTHREADS)) {
+ PROC_LOCK(p);
+ thread_single_end(p, SINGLE_BOUNDARY);
+ PROC_UNLOCK(p);
+ }
return (error);
}
@@ -3056,11 +3080,22 @@
sys_jail_attach(struct thread *td, struct jail_attach_args *uap)
{
struct prison *pr;
+ struct proc *p;
int drflags, error;
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. */
+ p = td->td_proc;
+ if ((p->p_flag & P_HADTHREADS) != 0) {
+ PROC_LOCK(p);
+ if (thread_single(p, SINGLE_BOUNDARY)) {
+ PROC_UNLOCK(p);
+ return (ERESTART);
+ }
+ PROC_UNLOCK(p);
+ }
sx_slock(&allprison_lock);
drflags = PD_LIST_SLOCKED;
@@ -3087,6 +3122,11 @@
done:
prison_deref(pr, drflags);
+ if ((p->p_flag & P_HADTHREADS) != 0) {
+ PROC_LOCK(p);
+ thread_single_end(p, SINGLE_BOUNDARY);
+ PROC_UNLOCK(p);
+ }
return (error);
}
@@ -3099,9 +3139,21 @@
sys_jail_attach_jd(struct thread *td, struct jail_attach_jd_args *uap)
{
struct prison *pr;
+ struct proc *p;
struct ucred *jdcred;
int drflags, error;
+ /* Only let a single thread in the process try to attach at a time. */
+ p = td->td_proc;
+ if ((p->p_flag & P_HADTHREADS) != 0) {
+ PROC_LOCK(p);
+ if (thread_single(p, SINGLE_BOUNDARY)) {
+ PROC_UNLOCK(p);
+ return (ERESTART);
+ }
+ PROC_UNLOCK(p);
+ }
+
sx_slock(&allprison_lock);
drflags = PD_LIST_SLOCKED;
pr = NULL;
@@ -3128,6 +3180,11 @@
done:
prison_deref(pr, drflags);
+ if ((p->p_flag & P_HADTHREADS) != 0) {
+ PROC_LOCK(p);
+ thread_single_end(p, SINGLE_BOUNDARY);
+ PROC_UNLOCK(p);
+ }
return (error);
}
@@ -3145,15 +3202,6 @@
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
@@ -3243,7 +3291,7 @@
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;
diff --git a/tests/sys/kern/Makefile b/tests/sys/kern/Makefile
--- a/tests/sys/kern/Makefile
+++ b/tests/sys/kern/Makefile
@@ -23,6 +23,7 @@
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 @@
LIBADD.aslr+= util
LIBADD.copy_file_range+= md
LIBADD.jail_lookup_root+= jail util
+LIBADD.jail_thread+= jail pthread
LIBADD.jaildesc+= kvm pthread
LIBADD.ssl_sendfile+= pthread crypto ssl
CFLAGS.sys_getrandom+= -I${SRCTOP}/sys/contrib/zstd/lib
diff --git a/tests/sys/kern/jail_thread.c b/tests/sys/kern/jail_thread.c
new file mode 100644
--- /dev/null
+++ b/tests/sys/kern/jail_thread.c
@@ -1,0 +1,230 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2026 James Gritton <jamie@FreeBSD.org>
+ */
+
+#include <sys/param.h>
+#include <sys/jail.h>
+#include <sys/stat.h>
+
+#include <err.h>
+#include <errno.h>
+#include <jail.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <atf-c.h>
+
+#define JAIL_NAME "jail_thread_attach_test"
+
+#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 *syscall_name, bool jail_reset)
+{
+ int ri, spn, mixed_jails;
+ size_t ji, ti, ji_hostname, ji_ino;
+ char *cwd;
+ struct stat st;
+ char jnamestr[32], 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),
+ JAIL_NAME "%zu", 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. */
+ 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,
+ sizeof(JAIL_NAME) - 1) == 0,
+ "unexpected jail hostname %s", jnamestr);
+ ji_hostname = strtol(jnamestr + sizeof(JAIL_NAME) - 1,
+ 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);
+}
+
+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_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_setv", 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, "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());
+}

File Metadata

Mime Type
text/plain
Expires
Mon, Sep 7, 6:23 PM (9 h, 28 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
38480134
Default Alt Text
D57858.id180787.diff (12 KB)

Event Timeline