From 760841f9c8a410006fa4a9369fc66a6863146797 Mon Sep 17 00:00:00 2001 From: kevans Date: Mon, 27 May 2019 13:12:51 +0000 Subject: [PATCH] MFC r348215, r348219: fix bectl(8) jail w/ numeric BE names r348215: jail_getid(3): validate jid string input Currently, if jail_getid(3) is passed in a numeric string, it assumes that this is a jid string and passes it back converted to an int without checking that it's a valid/existing jid. This breaks consumers that might use jail_getid(3) to see if it can trivially grab a jid from a name if that name happens to be numeric but not actually the name/jid of the jail. Instead of returning -1 for the jail not existing, it'll return the int version of the input and the consumer will not fallback to trying other methods. Pass the numeric input to jail_get(2) as the jid for validation, rather than the name. This works well- the kernel enforces that jid=name if name is numeric, so doing the safe thing and checking numeric input as a jid will still DTRT based on the description of jail_getid. r348219: bectl(8): Add a test for jail/unjail of numeric BE names Fixed by r348215, bectl ujail first attempts the trivial fetch of a jid by passing the first argument to 'ujail' to jail_getid(3) in case a jid/name have been passed in instead of a BE name. For numerically named BEs, this was doing the wrong thing: instead of failing to locate the jid specified and falling back to mountpath search, jail_getid(3) would return the input as-is. While here, I've fixed bectl_jail_cleanup which still used a hard-coded pool name that was overlooked w.r.t. other work that was in-flight around the same time. Approved by: re (marius) --- lib/libjail/jail_getid.c | 19 ++++++++++++------- sbin/bectl/tests/bectl_test.sh | 16 ++++++++++++---- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/libjail/jail_getid.c b/lib/libjail/jail_getid.c index dab2dcf25ce..be5ca101eb8 100644 --- a/lib/libjail/jail_getid.c +++ b/lib/libjail/jail_getid.c @@ -51,13 +51,18 @@ jail_getid(const char *name) struct iovec jiov[4]; jid = strtoul(name, &ep, 10); - if (*name && !*ep) - return jid; - jiov[0].iov_base = __DECONST(char *, "name"); - jiov[0].iov_len = sizeof("name"); - jiov[1].iov_len = strlen(name) + 1; - jiov[1].iov_base = alloca(jiov[1].iov_len); - strcpy(jiov[1].iov_base, name); + if (*name && !*ep) { + jiov[0].iov_base = __DECONST(char *, "jid"); + jiov[0].iov_len = sizeof("jid"); + jiov[1].iov_base = &jid; + jiov[1].iov_len = sizeof(jid); + } else { + jiov[0].iov_base = __DECONST(char *, "name"); + jiov[0].iov_len = sizeof("name"); + jiov[1].iov_len = strlen(name) + 1; + jiov[1].iov_base = alloca(jiov[1].iov_len); + strcpy(jiov[1].iov_base, name); + } jiov[2].iov_base = __DECONST(char *, "errmsg"); jiov[2].iov_len = sizeof("errmsg"); jiov[3].iov_base = jail_errmsg; diff --git a/sbin/bectl/tests/bectl_test.sh b/sbin/bectl/tests/bectl_test.sh index d1aa7fe8c18..6d268c00b17 100755 --- a/sbin/bectl/tests/bectl_test.sh +++ b/sbin/bectl/tests/bectl_test.sh @@ -294,8 +294,15 @@ bectl_jail_body() atf_check cp /rescue/rescue ${root}/rescue/rescue atf_check bectl -r ${zpool}/ROOT umount default - # Prepare a second boot environment + # Prepare some more boot environments atf_check -o empty -s exit:0 bectl -r ${zpool}/ROOT create -e default target + atf_check -o empty -s exit:0 bectl -r ${zpool}/ROOT create -e default 1234 + + # Attempt to unjail a BE with numeric name; jail_getid at one point + # did not validate that the input was a valid jid before returning the + # jid. + atf_check -o empty -s exit:0 bectl -r ${zpool}/ROOT jail -b 1234 + atf_check -o empty -s exit:0 bectl -r ${zpool}/ROOT unjail 1234 # When a jail name is not explicit, it should match the jail id. atf_check -o empty -s exit:0 bectl -r ${zpool}/ROOT jail -b -o jid=233637 default @@ -340,9 +347,10 @@ bectl_jail_body() # attempts to destroy the zpool. bectl_jail_cleanup() { - for bootenv in "default" "target"; do + zpool=$(get_zpool_name) + for bootenv in "default" "target" "1234"; do # mountpoint of the boot environment - mountpoint="$(bectl -r bectl_test/ROOT list -H | grep ${bootenv} | awk '{print $3}')" + mountpoint="$(bectl -r ${zpool}/ROOT list -H | grep ${bootenv} | awk '{print $3}')" # see if any jail paths match the boot environment mountpoint jailid="$(jls | grep ${mountpoint} | awk '{print $1}')" @@ -353,7 +361,7 @@ bectl_jail_cleanup() jail -r ${jailid} done; - bectl_cleanup $(get_zpool_name) + bectl_cleanup ${zpool} } atf_init_test_cases() -- 2.45.0