From nobody Fri Apr 24 06:44:26 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 4g23NZ1g67z6Zbqf; Fri, 24 Apr 2026 06:44:46 +0000 (UTC) (envelope-from zlei@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (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 "smtp.freebsd.org", Issuer "R12" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4g23NZ0djlz43JZ; Fri, 24 Apr 2026 06:44:46 +0000 (UTC) (envelope-from zlei@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1777013086; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EC4X/7Lmp78Vk3iZBoqwIbT2RfmINzInsbcRW7BLCxs=; b=ytVGlp/Tn2IsTRM7rsohYqlEjB0botgpb6e4AegQKVGjm3Z7gOfJPv/TL4xLbXU46gtuyQ W3t3iATevHSNWsrwNWKGFdP2scrrXAaR3pZGD2YES7yWovSOMHK/pb0DT0nJTajKmuOScm n5cnR+TjZjxShkeIMvOLh5M67OG+acOfqY5Sdsr3kTDq8vpjIUPRb8+CSwZsnIq4T6kA9/ 4OYtBOT7nevlRMFrGtf8DdF+m5i5/Cxh0CRFrPeKhu55tAfCbtrv58LGiB58HiWz9SYizg 1iCAMFCfNTP20GKgPe8FQyNKMhHZ8xdOPGrjDQp69yplFL9kvBX7sDaLcwd4ww== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1777013086; a=rsa-sha256; cv=none; b=NjG+jOrfFFHqSV1rxjMjP6PiwSZZIVv6QfdBKmz2TsPSPenrkmp6bC95eevMLFc9i7+3Ki 3cJwXPvQhdm87IDkaJrIgK9fFUii9t9dSe6LEyCu5VxSJLuqG3X1l8ks0TjwGNDbx0wNYa b98a0CaeW5L+ZYL7IqSTRKvNlaahiPZ5WWsbHarw2WcTiMWlT0WSGlyvYK/8SXc0v09VtB gXuDzdMcxfU67+bxfCLvkfw/qFS9ip5jOzlKRtOufMvffS506q62SWzibS49/X6yrLAjI2 AlpEWMUMjd6PJY9VSif+el/Vambv7hFZT3SnhoQaQqRgUZrSJvRqlUhfsyXUoQ== 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=1777013086; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EC4X/7Lmp78Vk3iZBoqwIbT2RfmINzInsbcRW7BLCxs=; b=KDZghIfZhjOFlaFjtwm4Bnfe8TLnJLgFVpmKHkHc9khO0tcnnxziDhwFhruk9KVqo4099B an1x06+ac6MoPQhi7O504qfI8cGCjUnjtSPUPrciEDBsxHcEZSsXGXwadfau1MaRFk/Rgq lcCQv+oSSA04WF20/FnJg7ZwcFQCryyg/rswQ1aU4C92uiyiwJDrYKqpdTmAAV0/aLZN48 WpZGgdgbqFyFNrID1tL2Rou7C6RhrGJF36PLPGWrV2rG3HaGYhXnkrPrShg6WKiSXkddtt FM/LaenargmZ+QATy3JFjcq0cdnVmqkZ+DU0gVeLFgK5qBQJmadbzHPwAhI9Nw== Received: from smtpclient.apple (unknown [IPv6:2001:19f0:6001:9db:98f0:9fe0:3545:10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) (Authenticated sender: zlei/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4g23NT6NNGz2Lf; Fri, 24 Apr 2026 06:44:41 +0000 (UTC) (envelope-from zlei@FreeBSD.org) Content-Type: text/plain; charset=us-ascii 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 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.10\)) Subject: Re: git: df670d26ad5b - stable/14 - jail: consistently populate the KP_JID and KP_NAME parameters From: Zhenlei Huang In-Reply-To: <69e996e4.3ac9c.14b0bfb4@gitrepo.freebsd.org> Date: Fri, 24 Apr 2026 14:44:26 +0800 Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-branches@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: <1B692EB3-E2C4-4AB2-9694-2F24223E0E17@FreeBSD.org> References: <69e996e4.3ac9c.14b0bfb4@gitrepo.freebsd.org> To: Kyle Evans X-Mailer: Apple Mail (2.3696.120.41.1.10) > On Apr 23, 2026, at 11:49 AM, Kyle Evans wrote: >=20 > The branch stable/14 has been updated by kevans: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3Ddf670d26ad5bebce71f39fc3bdc3ef11= ed0f1ea8 >=20 > commit df670d26ad5bebce71f39fc3bdc3ef11ed0f1ea8 > Author: Kyle Evans > AuthorDate: 2025-07-26 03:13:44 +0000 > Commit: Kyle Evans > CommitDate: 2026-04-23 03:22:59 +0000 >=20 > jail: consistently populate the KP_JID and KP_NAME parameters >=20 > 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. >=20 > 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. >=20 > This primarily fixes the below, but the issues with vnet.interface = and > zfs.dataset were pre-existing. Thanks for doing this ! >=20 > Fixes: d8f021add40c3 ("jail: add JID, JNAME and JPATH to env = [...]") > Reviewed by: jamie >=20 > (cherry picked from commit = 02944d8c4969ffe97fcf84cb2ccb672e828c1d04) > --- > usr.sbin/jail/command.c | 36 ++++++--- > 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(+), 12 deletions(-) >=20 > diff --git a/usr.sbin/jail/command.c b/usr.sbin/jail/command.c > index 20f28abc6706..26eaeca442a9 100644 > --- a/usr.sbin/jail/command.c > +++ b/usr.sbin/jail/command.c > @@ -291,8 +291,8 @@ run_command(struct cfjail *j) > const struct cfstring *comstring, *s; > login_cap_t *lcap; > const char **argv; > - char *acs, *ajidstr, *cs, *comcs, *devpath; > - const char *jidstr, *conslog, *path, *ruleset, *term, *username; > + char *acs, *cs, *comcs, *devpath; > + const char *conslog, *path, *ruleset, *term, *username; > enum intparam comparam; > size_t comlen; > pid_t pid; > @@ -333,6 +333,25 @@ run_command(struct cfjail *j) > printf("%d\n", j->jid); > if (verbose >=3D 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] =3D=3D 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] =3D=3D NULL) > + add_param(j, j->intparams[KP_JID], = KP_NAME, > + NULL); > + > dep_done(j, DF_LIGHT); > } > return 0; > @@ -457,8 +476,7 @@ run_command(struct cfjail *j) > argv[0] =3D _PATH_IFCONFIG; > argv[1] =3D comstring->s; > argv[2] =3D down ? "-vnet" : "vnet"; > - jidstr =3D string_param(j->intparams[KP_JID]); > - argv[3] =3D jidstr ? jidstr : = string_param(j->intparams[KP_NAME]); > + argv[3] =3D string_param(j->intparams[KP_JID]); > argv[4] =3D NULL; > break; >=20 > @@ -772,14 +790,10 @@ run_command(struct cfjail *j) > endpwent(); > } > if (!injail) { > - if (asprintf(&ajidstr, "%d", j->jid) =3D=3D -1) { > - jail_warnx(j, "asprintf jid=3D%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 =3D 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 8c9ff0a7bd09..42498f01efd0 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); >=20 > + /* > + * 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] =3D=3D NULL) > + add_param(j, j->intparams[KP_JID], KP_NAME, = NULL); > + > /* Resolve any variable substitutions. */ > pgen =3D 0; > TAILQ_FOREACH(p, &j->params, tq) { > diff --git a/usr.sbin/jail/jail.c b/usr.sbin/jail/jail.c > index 9e443c9f3f1d..c99fb162786b 100644 > --- a/usr.sbin/jail/jail.c > +++ b/usr.sbin/jail/jail.c > @@ -887,7 +887,14 @@ running_jid(struct cfjail *j, int dflag) > j->jid =3D -1; > return; > } > + > j->jid =3D jail_get(jiov, 2, dflag ? JAIL_DYING : 0); > + if (j->jid > 0 && j->intparams[KP_JID] =3D=3D NULL) { > + char jidstr[16]; > + > + (void)snprintf(jidstr, sizeof(jidstr), "%d", j->jid); > + add_param(j, NULL, KP_JID, jidstr); > + } > } >=20 > static void > diff --git a/usr.sbin/jail/tests/jail_basic_test.sh = b/usr.sbin/jail/tests/jail_basic_test.sh > index 51ccafb45f60..a6859b4dc7f2 100755 > --- a/usr.sbin/jail/tests/jail_basic_test.sh > +++ b/usr.sbin/jail/tests/jail_basic_test.sh > @@ -137,10 +137,139 @@ commands_cleanup() > fi > } >=20 > +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=3D999999} > + > + # 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=3D5309 > + while jls -cj "$jid"; do > + if [ "$jid" -eq "$JAIL_MAX" ]; then > + atf_skip "System has too many jail, cannot find = free slot" > + fi > + > + jid=3D$((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=3D$(find_unused_jid) > + > + echo "basejail" >> jails.lst > + echo "$jid { name =3D 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 =3D $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=3D/ exec.poststart=3D"true" = command=3D/usr/bin/true > + > + iface=3D$(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=3D/ vnet=3Dnew vnet.interface=3D"$iface" = command=3D/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 =3D /; exec.prestop =3D '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=3D$(find_unused_jid) > + echo "$jid {path =3D /; exec.prestop =3D '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 =3D basejail; path =3D /; exec.prestop =3D = '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 =3D /; exec.poststop =3D 'echo JID=3D\$JID'; = persist; }" > jail.conf > + atf_check -o ignore jail -f jail.conf -c basejail > + jid=3D$(jls -j basejail jid) > + > + atf_check -o match:"JID=3D$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 > +} >=20 > atf_init_test_cases() > { > atf_add_test_case "basic" > atf_add_test_case "nested" > atf_add_test_case "commands" > + atf_add_test_case "jid_name_set" > + atf_add_test_case "param_consistency" > } >=20