bin/53999: cgdconfig -V gpt silent failure

[rudolf <netbsd%eq.cz@localhost>]

>Number:         53999
>Category:       bin
>Synopsis:       cgdconfig -V gpt silent failure
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Feb 22 07:55:00 +0000 2019
>Originator:     rudolf <netbsd%eq.cz@localhost>
>Release:        NetBSD 8.0_STABLE
>Organization:
	rudolf
>Environment:
System: NetBSD 8.0_STABLE
Architecture: x86_64
Machine: amd64


>Description:

In case of verification method failure with methods "mbr" and "gpt",
cgdconfig(8) currently exits with a non-zero exit status but prints no
message to stderr and leaves the cgd device configured.  The interactive
user has thus no immediate indication of the failure.

http://mail-index.netbsd.org/tech-userlevel/2019/02/20/msg011795.html


>How-To-Repeat:

# dd if=/dev/zero of=img bs=1024k count=1
1+0 records in
1+0 records out
1048576 bytes transferred in 0.013 secs (80659692 bytes/sec)
# vnconfig -c vnd0 img
# cgdconfig -V gpt cgd0 /dev/vnd0d /usr/tests/dev/cgd/paramsfile
/dev/vnd0d's passphrase: (enter anything)
lat# echo $?
255
# cgdconfig -l
cgd0: vnd0d
cgd1: not in use
cgd2: not in use
cgd3: not in use


>Fix:

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
 }