tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: cgdconfig verification method failure



rudolf wrote:
(observed under netbsd-8, attached is an untested (but trivial) patch for current)

Attached is a tested and corrected version of the patch (differences to previous: 1. missing brackets, 2. ignore stderr of rump.cgdconfig in the tests).

cgdconfig(8) currently (and under netbsd-8) behaves inconsistently regarding the verification method failure: if the verification method is "ffs" or "disklabel", then on a password mismatch it prompts for the passphrase again ("cgdconfig: verification failed, please reenter passphrase"). In case of verification methods "mbr" and "gpt", verify() returns -1 and thus cgdconfig(8) exits. This seems like a bug to me and should be fixed by the second and the third hunk of the diff to cgdconfig.c from the attached patch.

cgdconfig(8) also upon an exit due to the verification method failure leaves the cgd device configured (although correctly exiting with a non-zero exit status). This seems also like a bug to me (or an inconvenience) and should be fixed by the first hunk of the diff to cgdconfig.c from the attached patch. The amended tests from the patch cover this behavior (taking the fact that there is a configured device on verification method failure as a test failure). The fact that the devices are currently configured even after the verification failure seems to be in contradiction with the sentence from the cgdconfig(8) manual page (regarding the "-p" flag"): ``If this flag is specified then verification errors will cause the device in question to be unconfigured rather than prompting for the passphrase again.''

r.

diff --git a/sbin/cgdconfig/cgdconfig.c b/sbin/cgdconfig/cgdconfig.c
index d1e035195866..cdc815fa05fb 100644
--- a/sbin/cgdconfig/cgdconfig.c
+++ b/sbin/cgdconfig/cgdconfig.c
@@ -625,8 +625,10 @@ configure(int argc, char **argv, struct params *inparams, int flags)
 			goto bail_err;
 
 		ret = verify(p, fd);
-		if (ret == -1)
+		if (ret == -1) {
+			(void)unconfigure_fd(fd);
 			goto bail_err;
+		}
 		if (!ret)
 			break;
 
@@ -830,7 +832,7 @@ verify_mbr(int fd)
 
 	memcpy(&mbr, buf, sizeof(mbr));
 	if (le16toh(mbr.mbr_magic) != MBR_MAGIC)
-		return -1;
+		return 1;
 
 	return 0;
 }
@@ -916,7 +918,7 @@ verify_gpt(int fd)
 		return -1;
 	}
 
-	ret = -1;
+	ret = 1;
 	for (blksize=DEV_BSIZE;
              (off = blksize * GPT_HDR_BLKNO) <= SCANSIZE - sizeof(hdr);
              blksize <<= 1) {
diff --git a/tests/dev/cgd/t_cgd.sh b/tests/dev/cgd/t_cgd.sh
index 9cd50a5fd86d..f5ac322b4612 100644
--- a/tests/dev/cgd/t_cgd.sh
+++ b/tests/dev/cgd/t_cgd.sh
@@ -150,10 +150,53 @@ unaligned_write_cleanup()
 	env RUMP_SERVER=unix://csock rump.halt || true
 }
 
+vmeth_failure_body()
+{
+
+	local vmeth="$1"
+	local d=$(atf_get_srcdir)
+
+	atf_check -s exit:0 \
+	    ${cgdserver} -d key=/dev/dk,hostpath=dk.img,size=1m unix://csock
+	export RUMP_SERVER=unix://csock
+	atf_check -s not-exit:0 -e ignore -x "echo 12345 | \
+	    rump.cgdconfig -V "${vmeth}" -p cgd0 /dev/dk ${d}/paramsfile"
+	atf_check -s exit:0 -o not-match:"(^| )cgd0( |$)" rump.sysctl -n hw.disknames
+}
+
+test_case_vmeth_failure()
+{
+
+	local vmeth="${1}"
+	local name="vmeth_failure_${vmeth}"
+
+	atf_test_case "${name}" cleanup
+	eval "${name}_head() { \
+		atf_set "descr" "Tests verification method \"${vmeth}\" failure" ; \
+		atf_set "require.progs" "rump_server" ; \
+	}"
+	eval "${name}_body() { \
+		vmeth_failure_body "${vmeth}" ; \
+	}"
+	eval "${name}_cleanup() { \
+		rump.cgdconfig -u cgd0 2>/dev/null ; \
+		env RUMP_SERVER=unix://csock rump.halt || true ; \
+	}"
+}
+
+test_case_vmeth_failure disklabel
+test_case_vmeth_failure ffs
+test_case_vmeth_failure gpt
+test_case_vmeth_failure mbr
+
 atf_init_test_cases()
 {
 
 	atf_add_test_case basic
 	atf_add_test_case wrongpass
 	atf_add_test_case unaligned_write
+	atf_add_test_case vmeth_failure_disklabel
+	atf_add_test_case vmeth_failure_ffs
+	atf_add_test_case vmeth_failure_gpt
+	atf_add_test_case vmeth_failure_mbr
 }


Home | Main Index | Thread Index | Old Index