From 4fc1ea9c6c2048d931b9906876d4dc77b8e0812f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Lesimple?= <speed47_github@speed47.net>
Date: Fri, 5 Aug 2022 03:02:57 +0300
Subject: [PATCH] zpool: fix redundancy check after vdev removal
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The presence of indirect vdevs was confusing get_redundancy(), which
considered a pool with e.g. only mirror top-level vdevs and at least
one indirect vdev (due to the removal of a previous vdev) as already
having a broken redundancy, which is not the case. This lead to the
possibility of compromising the redundancy of a pool by adding
mismatched vdevs without requiring the use of `-f`, and with no
visible notice or warning.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Stéphane Lesimple <speed47_github@speed47.net>
Closes #13705
Closes #13711
---
 cmd/zpool/zpool_vdev.c                        |  9 ++-
 .../removal/removal_with_indirect.ksh         | 57 +++++++++++++++++++
 .../functional/removal/remove_mirror.ksh      |  3 +-
 3 files changed, 66 insertions(+), 3 deletions(-)
 create mode 100755 tests/zfs-tests/tests/functional/removal/removal_with_indirect.ksh

diff --git a/cmd/zpool/zpool_vdev.c b/cmd/zpool/zpool_vdev.c
index b9a0af1188..8db1aef765 100644
--- a/cmd/zpool/zpool_vdev.c
+++ b/cmd/zpool/zpool_vdev.c
@@ -514,9 +514,14 @@ get_replication(nvlist_t *nvroot, boolean_t fatal)
 		if (is_log)
 			continue;
 
-		/* Ignore holes introduced by removing aux devices */
+		/*
+		 * Ignore holes introduced by removing aux devices, along
+		 * with indirect vdevs introduced by previously removed
+		 * vdevs.
+		 */
 		verify(nvlist_lookup_string(nv, ZPOOL_CONFIG_TYPE, &type) == 0);
-		if (strcmp(type, VDEV_TYPE_HOLE) == 0)
+		if (strcmp(type, VDEV_TYPE_HOLE) == 0 ||
+		    strcmp(type, VDEV_TYPE_INDIRECT) == 0)
 			continue;
 
 		if (nvlist_lookup_nvlist_array(nv, ZPOOL_CONFIG_CHILDREN,
diff --git a/tests/zfs-tests/tests/functional/removal/removal_with_indirect.ksh b/tests/zfs-tests/tests/functional/removal/removal_with_indirect.ksh
new file mode 100755
index 0000000000..2a7878f4a2
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/removal/removal_with_indirect.ksh
@@ -0,0 +1,57 @@
+#! /bin/ksh -p
+#
+# CDDL HEADER START
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2014, 2018 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/removal/removal.kshlib
+
+TMPDIR=${TMPDIR:-$TEST_BASE_DIR}
+
+DISK1="$TMPDIR/dsk1"
+DISK2="$TMPDIR/dsk2"
+DISK3="$TMPDIR/dsk3"
+DISK4="$TMPDIR/dsk4"
+DISKS="$DISK1 $DISK2 $DISK3 $DISK4"
+
+log_must mkfile $(($MINVDEVSIZE * 2)) $DISK1
+log_must mkfile $(($MINVDEVSIZE * 2)) $DISK2
+log_must mkfile $(($MINVDEVSIZE * 2)) $DISK3
+log_must mkfile $(($MINVDEVSIZE * 2)) $DISK4
+
+function cleanup
+{
+	default_cleanup_noexit
+	log_must rm -f $DISKS
+}
+
+# Build a zpool with 2 mirror vdevs
+log_must default_setup_noexit "mirror $DISK1 $DISK2 mirror $DISK3 $DISK4"
+log_onexit cleanup
+
+# Remove one of the mirrors
+log_must zpool remove $TESTPOOL mirror-1
+log_must wait_for_removal $TESTPOOL
+
+# Attempt to add a single-device vdev, shouldn't work
+log_mustnot zpool add $TESTPOOL $DISK3
+
+# Force it, should work
+log_must zpool add -f $TESTPOOL $DISK3
+
+log_pass "Prevented from adding a non-mirror vdev on a mirrored zpool w/ indirect vdevs"
diff --git a/tests/zfs-tests/tests/functional/removal/remove_mirror.ksh b/tests/zfs-tests/tests/functional/removal/remove_mirror.ksh
index 06335b7036..a62479f2a1 100755
--- a/tests/zfs-tests/tests/functional/removal/remove_mirror.ksh
+++ b/tests/zfs-tests/tests/functional/removal/remove_mirror.ksh
@@ -49,7 +49,8 @@ log_mustnot zpool remove $TESTPOOL $DISK2
 log_must wait_for_removal $TESTPOOL
 
 # Add back the first disk.
-log_must zpool add $TESTPOOL $DISK1
+# We use -f as we're adding a single vdev to zpool with only mirrors.
+log_must zpool add -f $TESTPOOL $DISK1
 
 # Now attempt to remove the mirror.
 log_must zpool remove $TESTPOOL mirror-1