From 4a2e9a17d5b81ae97b2a1b72437def9ed4996aca Mon Sep 17 00:00:00 2001 From: ilovezfs Date: Mon, 25 Jan 2016 23:41:11 -0800 Subject: [PATCH] OpenZFS 6541 - Pool feature-flag check defeated if "verify" is included in the dedup property value Authored by: ilovezfs Reviewed by: Matthew Ahrens Reviewed by: Richard Laager Approved by: Robert Mustacchi Ported-by: Tony Hutter zio_checksum_to_feature() expects a zio_checksum enum not a raw property intval, so the new checksums weren't being detected when the ZIO_CHECKSUM_VERIFY flag got in the way. Given a pool without feature@sha512, zfs create -o dedup=sha512 naughty/fivetwelve_noverify_ds would fail as expected since the raw intval would indeed be equal to SPA_FEATURE_SHA512. However, zfs create -o dedup=sha512,verify naughty/fivetwelve_verify_ds would incorrectly succeed because ZIO_CHECKSUM_VERIFY would be in the way, the raw intval would not be a member of the enum, and zio_checksum_to_feature() would return SPA_FEATURE_NONE, with the result that spa_feature_is_enabled() would never be called. This was first detected with edonr, since in that case verify is required. This commit clears the ZIO_CHECKSUM_VERIFY flag before calling zio_checksum_to_feature() using the ZIO_CHECKSUM_MASK and verifies in zio_checksum_to_feature() that ZIO_CHECKSUM_MASK has been applied by the caller to attempt to prevent the same bug from occurring again in the future. OpenZFS-issue: https://www.illumos.org/issues/6541 OpenZFS-commit: https://github.com/illumos/illumos-gate/commit/971640e6aa954c91b0706543741aa4570299f4d7 Porting notes: This code was originally from Illumos, but I actually ported it from: openzfsonosx/zfs@bef06e1 --- module/zfs/zfs_ioctl.c | 2 +- module/zfs/zio_checksum.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index 9140c62a62..e5704e258d 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -3906,7 +3906,7 @@ zfs_check_settable(const char *dsname, nvpair_t *pair, cred_t *cr) return (SET_ERROR(EINVAL)); /* check prop value is enabled in features */ - feature = zio_checksum_to_feature(intval); + feature = zio_checksum_to_feature(intval & ZIO_CHECKSUM_MASK); if (feature == SPA_FEATURE_NONE) break; diff --git a/module/zfs/zio_checksum.c b/module/zfs/zio_checksum.c index 59871c50ea..d3d2f05a80 100644 --- a/module/zfs/zio_checksum.c +++ b/module/zfs/zio_checksum.c @@ -135,9 +135,15 @@ zio_checksum_info_t zio_checksum_table[ZIO_CHECKSUM_FUNCTIONS] = { ZCHECKSUM_FLAG_NOPWRITE, "edonr"}, }; +/* + * The flag corresponding to the "verify" in dedup=[checksum,]verify + * must be cleared first, so callers should use ZIO_CHECKSUM_MASK. + */ spa_feature_t zio_checksum_to_feature(enum zio_checksum cksum) { + VERIFY((cksum & ~ZIO_CHECKSUM_MASK) == 0); + switch (cksum) { case ZIO_CHECKSUM_SHA512: return (SPA_FEATURE_SHA512);