NetBSD Problem Report #58214
From mdehling@gmail.com Mon Apr 29 21:59:08 2024
Return-Path: <mdehling@gmail.com>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.3 with cipher TLS_AES_256_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 A401C1A9238
for <gnats-bugs@gnats.NetBSD.org>; Mon, 29 Apr 2024 21:59:08 +0000 (UTC)
Message-Id: <CAP+FTfWtJzxPqi-UMijPqPZfYFGtPg=TyQJbQB+QhAkjqE1hcg@mail.gmail.com>
Date: Mon, 29 Apr 2024 14:59:05 -0700
From: Malte Dehling <mdehling@gmail.com>
Reply-To:
To: GNATS Bugs <gnats-bugs@netbsd.org>
In-Reply-To: <CAP+FTfX-x5FWjpGxQhUeZ5Vg+ytHqYM5aX+UzbsRhEMeg623OA@mail.gmail.com>
Subject: Re: cgdconfig(8): Add zfs verification method
References: <CAP+FTfX-x5FWjpGxQhUeZ5Vg+ytHqYM5aX+UzbsRhEMeg623OA@mail.gmail.com>
>Number: 58214
>Category: bin
>Synopsis: Re: cgdconfig(8): Add zfs verification method
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: bin-bug-people
>State: closed
>Class: sw-bug
>Submitter-Id: unknown
>Arrival-Date: Mon Apr 29 22:00:00 +0000 2024
>Closed-Date: Mon May 13 07:38:32 +0000 2024
>Last-Modified: Mon May 13 11:57:22 +0000 2024
>Originator: Malte Dehling <mdehling@gmail.com>
>Release:
>Organization:
>Environment:
>Description:
Hi,
On Sun, Apr 28, 2024 at 01:13:05PM -0700, Malte Dehling wrote:
> >Submitter-Id: net
> >Originator: Malte Dehling
> >Organization:
> >Confidential: no
> >Synopsis: cgdconfig(8): Add zfs verification method
> >Severity: non-critical
> >Priority: medium
> >Category: bin
> >Class: change-request
> >Release: NetBSD 10.0, -current
> >Environment:
> NetBSD 10.0 (GENERIC) #4: Wed Apr 24 12:21:26 PDT 2024
> mdehling@nb-base-dev:/scratch/obj/sys/arch/amd64/compile/GENERIC amd64
>
> >Description:
> I would like to propose the attached patch to add a zfs verification
> method to the cgdconfig(8) program. With such a method, it will be
> possible to use cgd devices as vdevs in a zpool without an intermediate
> gpt.
How about this version instead?
I _think_ I got the byte order logic right:
- deduce the byte order from the magic bytes and use it to initialize
the checksum computation with the same byte order
- on-disk sha256 is always stored in big-endian order, so convert
whatever we compute to BE before comparing
Tested only on LE since I don't have a BE netbsd system handy right now.
---
Implement vdev label checksum verification. Allows using raw cgd
devices as vdevs in a zpool.
---
sbin/cgdconfig/Makefile | 15 +++++
sbin/cgdconfig/cgdconfig.8 | 2 +
sbin/cgdconfig/cgdconfig.c | 111 +++++++++++++++++++++++++++++++++++++
sbin/cgdconfig/params.c | 9 +++
sbin/cgdconfig/params.h | 1 +
5 files changed, 138 insertions(+)
diff --git a/sbin/cgdconfig/Makefile b/sbin/cgdconfig/Makefile
index fec75b4e375..0ed669ab738 100644
--- a/sbin/cgdconfig/Makefile
+++ b/sbin/cgdconfig/Makefile
@@ -29,4 +29,19 @@ ARGON2_NO_THREADS=1
.include "${NETBSDSRCDIR}/external/apache2/argon2/lib/libargon2/Makefile.inc"
.endif
+.if ${MKZFS} != "no"
+CPPFLAGS+= -DHAVE_ZFS
+
+OSNET=${NETBSDSRCDIR}/external/cddl/osnet
+CPPFLAGS.cgdconfig.c+= -I${OSNET}/include
+CPPFLAGS.cgdconfig.c+= -I${OSNET}/sys
+CPPFLAGS.cgdconfig.c+= -I${OSNET}/dist/head
+CPPFLAGS.cgdconfig.c+= -I${OSNET}/dist/lib/libzpool/common
+CPPFLAGS.cgdconfig.c+= -I${OSNET}/dist/uts/common
+CPPFLAGS.cgdconfig.c+= -I${OSNET}/dist/uts/common/fs/zfs
+
+COPTS.cgdconfig.c+= -Wno-unknown-pragmas
+COPTS.cgdconfig.c+= -Wno-strict-prototypes
+.endif
+
.include <bsd.prog.mk>
diff --git a/sbin/cgdconfig/cgdconfig.8 b/sbin/cgdconfig/cgdconfig.8
index 2b0da2cb732..e48ccde02c5 100644
--- a/sbin/cgdconfig/cgdconfig.8
+++ b/sbin/cgdconfig/cgdconfig.8
@@ -270,6 +270,8 @@ scan for a valid Master Boot Record.
scan for a valid GUID partition table.
.It ffs
scan for a valid FFS file system.
+.It zfs
+scan for a valid ZFS vdev label (if compiled with MKZFS).
.It re-enter
prompt for passphrase twice, and ensure entered passphrases are
identical.
diff --git a/sbin/cgdconfig/cgdconfig.c b/sbin/cgdconfig/cgdconfig.c
index 9a634ef37e2..de4d09013bf 100644
--- a/sbin/cgdconfig/cgdconfig.c
+++ b/sbin/cgdconfig/cgdconfig.c
@@ -73,6 +73,11 @@ __RCSID("$NetBSD: cgdconfig.c,v 1.61 2022/11/17
06:40:38 chs Exp $");
#include <ufs/ffs/fs.h>
+#ifdef HAVE_ZFS
+#include <sys/vdev_impl.h>
+#include <sha2.h>
+#endif
+
#include "params.h"
#include "pkcs5_pbkdf2.h"
#include "utils.h"
@@ -170,6 +175,9 @@ static int verify_ffs(int);
static int verify_reenter(struct params *);
static int verify_mbr(int);
static int verify_gpt(int);
+#ifdef HAVE_ZFS
+static int verify_zfs(int);
+#endif
__dead static void usage(void);
@@ -1024,6 +1032,10 @@ verify(struct params *p, int fd)
return verify_mbr(fd);
case VERIFY_GPT:
return verify_gpt(fd);
+#ifdef HAVE_ZFS
+ case VERIFY_ZFS:
+ return verify_zfs(fd);
+#endif
default:
warnx("unimplemented verification method");
return -1;
@@ -1182,6 +1194,105 @@ verify_gpt(int fd)
return ret;
}
+#ifdef HAVE_ZFS
+
+#define ZIO_CHECKSUM_BE(zcp) \
+{ \
+ (zcp)->zc_word[0] = BE_64((zcp)->zc_word[0]); \
+ (zcp)->zc_word[1] = BE_64((zcp)->zc_word[1]); \
+ (zcp)->zc_word[2] = BE_64((zcp)->zc_word[2]); \
+ (zcp)->zc_word[3] = BE_64((zcp)->zc_word[3]); \
+}
+
+static int
+verify_zfs(int fd)
+{
+ struct stat sb;
+ vdev_phys_t *vdev_phys;
+ off_t vdev_phys_off;
+ ssize_t ret;
+ bool byteswap;
+ zio_cksum_t cksum_found, cksum_real;
+ SHA256_CTX ctx;
+
+ if (fstat(fd, &sb)) {
+ warn("fstat");
+ return 1;
+ } else if (sb.st_size == 0) {
+ warnx("fstat returned size zero");
+ return 1;
+ }
+
+ vdev_phys = emalloc(sizeof *vdev_phys);
+ for (int l = 0; l < VDEV_LABELS; l++) {
+ vdev_phys_off = ( l < VDEV_LABELS/2 ?
+ l * sizeof(vdev_label_t) :
+ sb.st_size - (VDEV_LABELS-l) * sizeof(vdev_label_t) ) \
+ + offsetof(vdev_label_t, vl_vdev_phys);
+
+ ret = prog_pread(fd, vdev_phys, sizeof *vdev_phys,
+ vdev_phys_off);
+ if (ret < 0) {
+ warn("pread");
+ ret = 1;
+ break;
+ } else if ((size_t)ret < sizeof *vdev_phys) {
+ warnx("pread: incomplete block");
+ ret = 1;
+ break;
+ }
+
+ ret = 0;
+
+ if (vdev_phys->vp_zbt.zec_magic == BSWAP_64(ZEC_MAGIC))
+ byteswap = true;
+ else if (vdev_phys->vp_zbt.zec_magic == ZEC_MAGIC)
+ byteswap = false;
+ else {
+ ret = 1;
+ break;
+ };
+
+ cksum_found = vdev_phys->vp_zbt.zec_cksum;
+
+ ZIO_SET_CHECKSUM(&vdev_phys->vp_zbt.zec_cksum,
+ vdev_phys_off, 0, 0, 0);
+ if (byteswap)
+ ZIO_CHECKSUM_BSWAP(&vdev_phys->vp_zbt.zec_cksum);
+
+ SHA256Init(&ctx);
+ SHA256Update(&ctx, (uint8_t *)vdev_phys, sizeof *vdev_phys);
+ SHA256Final(&cksum_real, &ctx);
+
+ /*
+ * For historical reasons the on-disk sha256 checksums are
+ * always in big endian format.
+ * (see cddl/osnet/dist/uts/common/fs/zfs/sha256.c)
+ */
+ ZIO_CHECKSUM_BE(&cksum_real);
+
+ if (!ZIO_CHECKSUM_EQUAL(cksum_found, cksum_real)) {
+ warnx("checksum mismatch on vdev label %d", l);
+ warnx("found %lx, %lx, %lx, %lx",
+ (unsigned long)cksum_found.zc_word[0],
+ (unsigned long)cksum_found.zc_word[1],
+ (unsigned long)cksum_found.zc_word[2],
+ (unsigned long)cksum_found.zc_word[3]);
+ warnx("expected %lx, %lx, %lx, %lx",
+ (unsigned long)cksum_real.zc_word[0],
+ (unsigned long)cksum_real.zc_word[1],
+ (unsigned long)cksum_real.zc_word[2],
+ (unsigned long)cksum_real.zc_word[3]);
+ ret = 1;
+ break;
+ }
+ }
+ free(vdev_phys);
+ return ret;
+}
+
+#endif
+
static off_t sblock_try[] = SBLOCKSEARCH;
static int
diff --git a/sbin/cgdconfig/params.c b/sbin/cgdconfig/params.c
index 91af02c21cc..5da25ec0595 100644
--- a/sbin/cgdconfig/params.c
+++ b/sbin/cgdconfig/params.c
@@ -287,6 +287,10 @@ params_verify_method(string_t *in)
p->verify_method = VERIFY_MBR;
if (!strcmp("gpt", vm))
p->verify_method = VERIFY_GPT;
+#ifdef HAVE_ZFS
+ if (!strcmp("zfs", vm))
+ p->verify_method = VERIFY_ZFS;
+#endif
string_free(in);
@@ -1065,6 +1069,11 @@ params_fput(struct params *p, FILE *f)
case VERIFY_GPT:
print_kvpair_cstr(f, ts, "verify_method", "gpt");
break;
+#ifdef HAVE_ZFS
+ case VERIFY_ZFS:
+ print_kvpair_cstr(f, ts, "verify_method", "zfs");
+ break;
+#endif
default:
warnx("unsupported verify_method (%d)", p->verify_method);
return -1;
diff --git a/sbin/cgdconfig/params.h b/sbin/cgdconfig/params.h
index fb79a0deb80..ef49dde6aba 100644
--- a/sbin/cgdconfig/params.h
+++ b/sbin/cgdconfig/params.h
@@ -81,6 +81,7 @@ struct params {
#define VERIFY_REENTER 0x4
#define VERIFY_MBR 0x5
#define VERIFY_GPT 0x6
+#define VERIFY_ZFS 0x7
/* shared key derivation methods */
--
2.44.0
Cheers,
--
Malte Dehling
>How-To-Repeat:
>Fix:
Unknown
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: gnats-admin->kern-bug-people
Responsible-Changed-By: maya@NetBSD.org
Responsible-Changed-When: Wed, 01 May 2024 17:36:56 +0000
Responsible-Changed-Why:
Responsible-Changed-From-To: kern-bug-people->bin-bug-people
Responsible-Changed-By: kre@NetBSD.org
Responsible-Changed-When: Wed, 01 May 2024 23:20:34 +0000
Responsible-Changed-Why:
cgdconfig and its verification methods, is all userlnd responsibility,
the kernel has nothing whatever to do with it.
From: Malte Dehling <mdehling@gmail.com>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org, kern-bug-people@netbsd.org, netbsd-bugs@netbsd.org,
kre@netbsd.org
Subject: Re: Re: bin/58214 (Re: cgdconfig(8): Add zfs verification method)
Date: Sun, 12 May 2024 13:51:03 -0400
Hi,
On Wed, May 01, 2024 at 11:20:34PM +0000, kre@NetBSD.org wrote:
> Synopsis: Re: cgdconfig(8): Add zfs verification method
>
> Responsible-Changed-From-To: kern-bug-people->bin-bug-people
> Responsible-Changed-By: kre@NetBSD.org
> Responsible-Changed-When: Wed, 01 May 2024 23:20:34 +0000
> Responsible-Changed-Why:
> cgdconfig and its verification methods, is all userlnd responsibility,
> the kernel has nothing whatever to do with it.
This is a duplicate of #58212 and should be closed.
Sorry, I think I caused this by replying to the wrong email and thereby
causing gnats to open a new pr.
Thanks,
--
Malte Dehling
State-Changed-From-To: open->closed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Mon, 13 May 2024 07:38:32 +0000
State-Changed-Why:
Dup of bin/58212 ... closed at request of submitter.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.47 2022/09/11 19:34:41 kim Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2024
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home