Page MenuHomeFreeBSD

D51502.id159020.diff
No OneTemporary

D51502.id159020.diff

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"
}

File Metadata

Mime Type
text/plain
Expires
Sun, Apr 12, 12:42 AM (11 h, 36 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
31318469
Default Alt Text
D51502.id159020.diff (5 KB)

Event Timeline