NetBSD Problem Report #53999

From netbsd@eq.cz  Fri Feb 22 07:50:18 2019
Return-Path: <netbsd@eq.cz>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 442727A151
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 22 Feb 2019 07:50:18 +0000 (UTC)
Message-Id: <20190222075014.15059.qmail@a4.ip.eq.cz>
Date: 22 Feb 2019 07:50:14 -0000
From: rudolf <netbsd@eq.cz>
Reply-To: rudolf <netbsd@eq.cz>
To: gnats-bugs@NetBSD.org
Subject: cgdconfig -V gpt silent failure
X-Send-Pr-Version: 3.95

>Number:         53999
>Category:       bin
>Synopsis:       cgdconfig -V gpt silent failure
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kre
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Feb 22 07:55:00 +0000 2019
>Closed-Date:    Wed Apr 10 07:49:19 +0000 2019
>Last-Modified:  Wed Apr 10 07:50:00 +0000 2019
>Originator:     rudolf <netbsd@eq.cz>
>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
 }

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: bin-bug-people->kre
Responsible-Changed-By: kre@NetBSD.org
Responsible-Changed-When: Mon, 08 Apr 2019 18:57:48 +0000
Responsible-Changed-Why:
I am looking into this PR


From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53999 CVS commit: src
Date: Wed, 10 Apr 2019 06:09:39 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Wed Apr 10 06:09:39 UTC 2019

 Modified Files:
 	src/sbin/cgdconfig: cgdconfig.c
 	src/tests/dev/cgd: t_cgd.sh

 Log Message:
 PR bin/53999 from rudolf (eq.cz)

 Fix cgdconfig to report verification failures with gpt and mbr
 verification methods (and not treat them as silent hard errors).
 This also causes the cgd to be unconfigured when one of those
 verification methods fails.

 Add ATF tests to check that bad verification is reported, and
 does not leave the cgd configured.

 Patches from the PR applied.


 To generate a diff of this commit:
 cvs rdiff -u -r1.48 -r1.49 src/sbin/cgdconfig/cgdconfig.c
 cvs rdiff -u -r1.11 -r1.12 src/tests/dev/cgd/t_cgd.sh

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: kre@NetBSD.org
State-Changed-When: Wed, 10 Apr 2019 06:15:26 +0000
State-Changed-Why:
Is this OK for you now?

Your patch was applied, then I made a few minor cleanups.


State-Changed-From-To: feedback->closed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Wed, 10 Apr 2019 07:49:19 +0000
State-Changed-Why:
Patch supplied applied


From: rudolf <netbsd@eq.cz>
To: gnats-bugs@netbsd.org
Cc: kre@NetBSD.org
Subject: Re: bin/53999 (cgdconfig -V gpt silent failure)
Date: Wed, 10 Apr 2019 09:45:12 +0200

 kre@NetBSD.org wrote:
 > Is this OK for you now?
 > 
 > Your patch was applied, then I made a few minor cleanups.

 It's perfect, thanks for the cleanups.

 r.

>Unformatted:

Home	PR Database Search