diff --git a/usr.sbin/jail/command.c b/usr.sbin/jail/command.c --- a/usr.sbin/jail/command.c +++ b/usr.sbin/jail/command.c @@ -290,7 +290,7 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 --- a/usr.sbin/jail/config.c +++ b/usr.sbin/jail/config.c @@ -156,11 +156,14 @@ 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. We set + * both here if the name is numeric, otherwise the jid will get + * populated later. */ - add_param(j, NULL, - strtol(j->name, &ep, 10) && !*ep ? KP_JID : KP_NAME, - j->name); + add_param(j, NULL, KP_NAME, j->name); + if (strtol(j->name, &ep, 10) && *ep == '\0') + add_param(j, j->intparams[KP_NAME], KP_JID, NULL); + /* * Collect parameters for the jail, global parameters/variables, * and any matching wildcard jails. diff --git a/usr.sbin/jail/jail.c b/usr.sbin/jail/jail.c --- a/usr.sbin/jail/jail.c +++ b/usr.sbin/jail/jail.c @@ -890,7 +890,14 @@ 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 --- a/usr.sbin/jail/tests/jail_basic_test.sh +++ b/usr.sbin/jail/tests/jail_basic_test.sh @@ -165,6 +165,61 @@ fi } +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() +{ + + # 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 + + # 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() +{ + if jls -j basejail > /dev/null 2>&1; then + jail -r basejail + fi + + if [ -f "interfaces.lst" ]; then + while read iface; do + ifconfig "$iface" destroy + done < interfaces.lst + fi +} atf_init_test_cases() { @@ -172,4 +227,5 @@ atf_add_test_case "list" atf_add_test_case "nested" atf_add_test_case "commands" + atf_add_test_case "param_consistency" }