Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 27 May 2019 13:12:51 +0000 (UTC)
From:      Kyle Evans <kevans@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r348304 - in stable/11: lib/libjail sbin/bectl/tests
Message-ID:  <201905271312.x4RDCpHP066095@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kevans
Date: Mon May 27 13:12:51 2019
New Revision: 348304
URL: https://svnweb.freebsd.org/changeset/base/348304

Log:
  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)

Modified:
  stable/11/lib/libjail/jail_getid.c
  stable/11/sbin/bectl/tests/bectl_test.sh
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/lib/libjail/jail_getid.c
==============================================================================
--- stable/11/lib/libjail/jail_getid.c	Mon May 27 12:41:41 2019	(r348303)
+++ stable/11/lib/libjail/jail_getid.c	Mon May 27 13:12:51 2019	(r348304)
@@ -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;

Modified: stable/11/sbin/bectl/tests/bectl_test.sh
==============================================================================
--- stable/11/sbin/bectl/tests/bectl_test.sh	Mon May 27 12:41:41 2019	(r348303)
+++ stable/11/sbin/bectl/tests/bectl_test.sh	Mon May 27 13:12:51 2019	(r348304)
@@ -294,9 +294,16 @@ 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
 	atf_check -o inline:"233637\n" -s exit:0 -x "jls -j 233637 name"
@@ -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()



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201905271312.x4RDCpHP066095>