From nobody Sat Jul 26 03:14:07 2025 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 4bpqb36Gttz62y7s; Sat, 26 Jul 2025 03:14:07 +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 "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4bpqb34S86z3c8q; Sat, 26 Jul 2025 03:14:07 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1753499647; 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=G1z8oIs1aXxyLw/WOtOnm/Uc1f9I0pmhJY6qaPi/8Ok=; b=EXkwgBK4ACrtNhytZnWl1H0IIPPBKq2oqabqETLhkgIhnwT3TMDQB9EIYusR21KBorMhCj GljTiZ0pfAMOlnj5IXNMUpN1yNGIpHeH4mms8fsp0GLeSbh7ZCJIE+gonyEjzmO6buYoBs 5sckeEfqdG5IxNkkAXWXFVprBGiN0wiT2v4/n5GoRFKtI29Bx6Ynb+LgzoIE+7SvD5Fw8V N8sBgSrS/pBYDKMn8qDXcvTmXr7zZAahTz/VY4V1n4fKk/H8cDd9T1qKMR9SQZsRZ7Vq0E pltTFbNyjtejy1XjYq9/7MaHL4mM0Roe5Z/tB38ZPJ4hOGTr7ToHAkR35LeDGA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1753499647; 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=G1z8oIs1aXxyLw/WOtOnm/Uc1f9I0pmhJY6qaPi/8Ok=; b=R47cQ0Zcow6FKHAX7pKGUbLbhLVdXmiJQrk/pJ2/t2Vrt3WCTdT07JebFBUhmModqNy8cf MtXSi6NVOc6GsTTa2o84yOTKsNhnsqhODud6/Wy1PEZc015UcYA5JzHnV4+85CEDVivESl 5/K+HHpMHKKACg4t8HxMFtTZaqHl6JgQYvq6HhUS/jfZiW8F82D734zxtOpj3biMwubAnZ aZJTPbOoNgEukpdPcp/Ivir099ehBmRF8FkjHUwvn8oiVSCRqBUWcqq+e9aD1HyymCTUQk xw0jAaYXdILDUJaN3Sv4vM3wbNUe0nIRVNqAB9QFE6llleqmIIf0hBcUBrltPQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1753499647; a=rsa-sha256; cv=none; b=OgNRp6pzIFM7XuYI3125lqyOGrr4ajcD4Nl2xkckrTgPXIE+hmj2xJqbXdL0+vylWDR6n+ yXCNDqi6UW7eV7TdIxK9f92ZVXwr5MtpM3wcsilOQJtlMEFJDqlL8YVQc5UlHPW4Z2oTR8 lu7rYeiOFZA+Lre9bpGapsvKySmJOSy/7dF7j7cOvnIFiESGr+UmX6rDv2UAxpabUSnjQ5 E+sPy2wiNX7f0SPjprb/dFMyCXn+utIH/DsLFviPDDutKg0runGFT5u9FIYHWX7GxL67wl DKzlmkkrxmvyuvFzWe1zc4uG4huFyNNt3mVf2Z6rKiqAe1wF16IhxGliM3tnsQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4bpqb33rvCz1B38; Sat, 26 Jul 2025 03:14:07 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 56Q3E7Bi002947; Sat, 26 Jul 2025 03:14:07 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 56Q3E72I002944; Sat, 26 Jul 2025 03:14:07 GMT (envelope-from git) Date: Sat, 26 Jul 2025 03:14:07 GMT Message-Id: <202507260314.56Q3E72I002944@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kyle Evans Subject: git: 02944d8c4969 - main - jail: consistently populate the KP_JID and KP_NAME parameters 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 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kevans X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 02944d8c4969ffe97fcf84cb2ccb672e828c1d04 Auto-Submitted: auto-generated The branch main has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=02944d8c4969ffe97fcf84cb2ccb672e828c1d04 commit 02944d8c4969ffe97fcf84cb2ccb672e828c1d04 Author: Kyle Evans AuthorDate: 2025-07-26 03:13:44 +0000 Commit: Kyle Evans CommitDate: 2025-07-26 03:13:44 +0000 jail: consistently populate the KP_JID and KP_NAME parameters The gaps here, specifically, were: - When we have to discover a running jail's jid from name, we should populate the missing jid param - When we populate jid/name from the config, if the name is a jid we wouldn't populate the name; now we do both. - When we create a jail, we should populate jid and name with whatever details we have now that we didn't both. As a consequence, we can cleanup a few things: - vnet.interface and zfs.dataset can just always use the jid - Trying to populate JNAME should always work now, where it would be a little crashy before if you create a jail that didn't have a name or jid on the command line - We can simplify the just-prior JID population now that we'll keep a stringified jid in our intparams. This primarily fixes the below, but the issues with vnet.interface and zfs.dataset were pre-existing. Fixes: d8f021add40c3 ("jail: add JID, JNAME and JPATH to env [...]") Reviewed by: jamie Differential Revision: https://reviews.freebsd.org/D51502 --- usr.sbin/jail/command.c | 38 ++++++---- usr.sbin/jail/config.c | 13 +++- usr.sbin/jail/jail.c | 7 ++ usr.sbin/jail/tests/jail_basic_test.sh | 129 +++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+), 14 deletions(-) diff --git a/usr.sbin/jail/command.c b/usr.sbin/jail/command.c index 8ea3f3ee8795..9da4fe51673a 100644 --- a/usr.sbin/jail/command.c +++ b/usr.sbin/jail/command.c @@ -290,7 +290,7 @@ run_command(struct cfjail *j) const struct cfstring *comstring, *s; login_cap_t *lcap; const char **argv; - char *acs, *ajidstr, *cs, *comcs, *devpath; + char *acs, *cs, *comcs, *devpath; const char *jidstr, *conslog, *fmt, *path, *ruleset, *term, *username; enum intparam comparam; size_t comlen, ret; @@ -332,6 +332,25 @@ run_command(struct cfjail *j) printf("%d\n", j->jid); if (verbose >= 0 && (j->name || verbose > 0)) jail_note(j, "created\n"); + + /* + * Populate our jid and name parameters if they were not + * provided. This simplifies later logic that wants to + * use the jid or name to be able to do so reliably. + */ + if (j->intparams[KP_JID] == NULL) { + char ljidstr[16]; + + (void)snprintf(ljidstr, sizeof(ljidstr), "%d", + j->jid); + add_param(j, NULL, KP_JID, ljidstr); + } + + /* This matches the kernel behavior. */ + if (j->intparams[KP_NAME] == NULL) + add_param(j, j->intparams[KP_JID], KP_NAME, + NULL); + dep_done(j, DF_LIGHT); } return 0; @@ -456,8 +475,7 @@ run_command(struct cfjail *j) argv[0] = _PATH_IFCONFIG; argv[1] = comstring->s; argv[2] = down ? "-vnet" : "vnet"; - jidstr = string_param(j->intparams[KP_JID]); - argv[3] = jidstr ? jidstr : string_param(j->intparams[KP_NAME]); + argv[3] = string_param(j->intparams[KP_JID]); argv[4] = NULL; break; @@ -592,9 +610,7 @@ run_command(struct cfjail *j) case IP_ZFS_DATASET: argv = alloca(4 * sizeof(char *)); - jidstr = string_param(j->intparams[KP_JID]) ? - string_param(j->intparams[KP_JID]) : - string_param(j->intparams[KP_NAME]); + jidstr = string_param(j->intparams[KP_JID]); fmt = "if [ $(/sbin/zfs get -H -o value jailed %s) = on ]; then /sbin/zfs jail %s %s || echo error, attaching %s to jail %s failed; else echo error, you need to set jailed=on for dataset %s; fi"; comlen = strlen(fmt) + 2 * strlen(jidstr) @@ -796,14 +812,10 @@ run_command(struct cfjail *j) endpwent(); } if (!injail) { - if (asprintf(&ajidstr, "%d", j->jid) == -1) { - jail_warnx(j, "asprintf jid=%d: %s", j->jid, - strerror(errno)); - exit(1); - } - setenv("JID", ajidstr, 1); - free(ajidstr); + if (string_param(j->intparams[KP_JID])) + setenv("JID", string_param(j->intparams[KP_JID]), 1); setenv("JNAME", string_param(j->intparams[KP_NAME]), 1); + path = string_param(j->intparams[KP_PATH]); setenv("JPATH", path ? path : "", 1); } diff --git a/usr.sbin/jail/config.c b/usr.sbin/jail/config.c index 3af0088626c9..70de82e662e7 100644 --- a/usr.sbin/jail/config.c +++ b/usr.sbin/jail/config.c @@ -156,11 +156,14 @@ load_config(const char *cfname) TAILQ_CONCAT(&opp, &j->params, tq); /* * The jail name implies its "name" or "jid" parameter, - * though they may also be explicitly set later on. + * though they may also be explicitly set later on. After we + * collect other parameters, we'll go back and ensure they're + * both set if we need to do so here. */ add_param(j, NULL, strtol(j->name, &ep, 10) && !*ep ? KP_JID : KP_NAME, j->name); + /* * Collect parameters for the jail, global parameters/variables, * and any matching wildcard jails. @@ -180,6 +183,14 @@ load_config(const char *cfname) TAILQ_FOREACH(p, &opp, tq) add_param(j, p, 0, NULL); + /* + * We only backfill if it's the name that wasn't set; if it was + * the jid, we can assume that will be populated later when the + * jail is created or found. + */ + if (j->intparams[KP_NAME] == NULL) + add_param(j, j->intparams[KP_JID], KP_NAME, NULL); + /* Resolve any variable substitutions. */ pgen = 0; TAILQ_FOREACH(p, &j->params, tq) { diff --git a/usr.sbin/jail/jail.c b/usr.sbin/jail/jail.c index 27769cc14958..46cabf76ae11 100644 --- a/usr.sbin/jail/jail.c +++ b/usr.sbin/jail/jail.c @@ -890,7 +890,14 @@ running_jid(struct cfjail *j) j->jid = -1; return; } + j->jid = jail_get(jiov, 2, 0); + if (j->jid > 0 && j->intparams[KP_JID] == NULL) { + char jidstr[16]; + + (void)snprintf(jidstr, sizeof(jidstr), "%d", j->jid); + add_param(j, NULL, KP_JID, jidstr); + } } static void diff --git a/usr.sbin/jail/tests/jail_basic_test.sh b/usr.sbin/jail/tests/jail_basic_test.sh index f3c8be4a6595..509900e8569c 100755 --- a/usr.sbin/jail/tests/jail_basic_test.sh +++ b/usr.sbin/jail/tests/jail_basic_test.sh @@ -165,6 +165,133 @@ commands_cleanup() fi } +atf_test_case "jid_name_set" "cleanup" +jid_name_set_head() +{ + atf_set descr 'Test that one can set both the jid and name in a config file' + atf_set require.user root +} + +find_unused_jid() +{ + : ${JAIL_MAX=999999} + + # We'll start at a higher jid number and roll through the space until + # we find one that isn't taken. We start high to avoid racing parallel + # activity for the 'next available', though ideally we don't have a lot + # of parallel jail activity like that. + jid=5309 + while jls -cj "$jid"; do + if [ "$jid" -eq "$JAIL_MAX" ]; then + atf_skip "System has too many jail, cannot find free slot" + fi + + jid=$((jid + 1)) + done + + echo "$jid" | tee -a jails.lst +} +clean_jails() +{ + if [ ! -s jails.lst ]; then + return 0 + fi + + while read jail; do + if jls -e -j "$jail"; then + jail -r "$jail" + fi + done < jails.lst +} + +jid_name_set_body() +{ + local jid=$(find_unused_jid) + + echo "basejail" >> jails.lst + echo "$jid { name = basejail; persist; }" > jail.conf + atf_check -o match:"$jid: created" jail -f jail.conf -c "$jid" + atf_check -o match:"$jid: removed" jail -f jail.conf -r "$jid" + + echo "basejail { jid = $jid; persist; }" > jail.conf + atf_check -o match:"basejail: created" jail -f jail.conf -c basejail + atf_check -o match:"basejail: removed" jail -f jail.conf -r basejail +} + +jid_name_set_cleanup() +{ + clean_jails +} + +atf_test_case "param_consistency" "cleanup" +param_consistency_head() +{ + atf_set descr 'Test for consistency in jid/name params being set implicitly' + atf_set require.user root +} + +param_consistency_body() +{ + local iface jid + + echo "basejail" >> jails.lst + + # Most basic test: exec.poststart running a command without a jail + # config. This would previously crash as we only had the jid and name + # as populated at creation time. + atf_check jail -c path=/ exec.poststart="true" command=/usr/bin/true + + iface=$(ifconfig lo create) + atf_check test -n "$iface" + echo "$iface" >> interfaces.lst + + # Now do it again but exercising IP_VNET_INTERFACE, which is an + # implied command that wants to use the jid or name. This would crash + # as neither KP_JID or KP_NAME are populated when a jail is created, + # just as above- just at a different spot. + atf_check jail -c \ + path=/ vnet=new vnet.interface="$iface" command=/usr/bin/true + + # Test that a jail that we only know by name will have its jid resolved + # and added to its param set. + echo "basejail {path = /; exec.prestop = 'echo STOP'; persist; }" > jail.conf + + atf_check -o ignore jail -f jail.conf -c basejail + atf_check -o match:"STOP" jail -f jail.conf -r basejail + + # Do the same sequence as above, but use a jail with a jid-ish name. + jid=$(find_unused_jid) + echo "$jid {path = /; exec.prestop = 'echo STOP'; persist; }" > jail.conf + + atf_check -o ignore jail -f jail.conf -c "$jid" + atf_check -o match:"STOP" jail -f jail.conf -r "$jid" + + # Ditto, but now we set a name for that jid-jail. + echo "$jid {name = basejail; path = /; exec.prestop = 'echo STOP'; persist; }" > jail.conf + + atf_check -o ignore jail -f jail.conf -c "$jid" + atf_check -o match:"STOP" jail -f jail.conf -r "$jid" + + # Confirm that we have a valid jid available in exec.poststop. It's + # probably debatable whether we should or not. + echo "basejail {path = /; exec.poststop = 'echo JID=\$JID'; persist; }" > jail.conf + atf_check -o ignore jail -f jail.conf -c basejail + jid=$(jls -j basejail jid) + + atf_check -o match:"JID=$jid" jail -f jail.conf -r basejail + +} + +param_consistency_cleanup() +{ + clean_jails + + if [ -f "interfaces.lst" ]; then + while read iface; do + ifconfig "$iface" destroy + done < interfaces.lst + fi +} atf_init_test_cases() { @@ -172,4 +299,6 @@ atf_init_test_cases() atf_add_test_case "list" atf_add_test_case "nested" atf_add_test_case "commands" + atf_add_test_case "jid_name_set" + atf_add_test_case "param_consistency" }