Fix zfsd with the device_removal pool feature.

Previously zfsd would crash in the presence of a pool with a
top-level-vdev that had previously been removed.  The crash happened
because the configuration nvlist of such a TLV contains an empty
ZPOOL_CONFIG_CHILDREN array, which led to a pop_front from an empty
list, which has undefined behavior.

The crash only happened in stable/14 and later, probably do to
differences in libcxx, but the change should be MFCed anyway.

PR:		273663
Reported by:	Marek Zarychta <zarychtam@plan-b.pwste.edu.pl>
MFC after:	1 week
Sponsored by:	Axcient
Reviewed by:	mav
Differential Revision: https://reviews.freebsd.org/D41818

2 files changed, 38 insertions, 4 deletions

diff --git a/cddl/usr.sbin/zfsd/tests/zfsd_unittest.cc b/cddl/usr.sbin/zfsd/tests/zfsd_unittest.cc
index f3fea2ca83f4..caeb077a3de8 100644
--- a/cddl/usr.sbin/zfsd/tests/zfsd_unittest.cc
+++ b/cddl/usr.sbin/zfsd/tests/zfsd_unittest.cc
@@ -766,3 +766,40 @@ TEST_F(ReEvaluateByGuidTest, ReEvaluateByGuid_five)
 	delete CaseFile4;
 	delete CaseFile5;
 }
+
+/*
+ * Test VdevIterator
+ */
+class VdevIteratorTest : public ::testing::Test
+{
+};
+
+bool VdevIteratorTestCB(Vdev &vdev, void *cbArg) {
+	return (false);
+}
+
+/*
+ * VdevIterator::Next should not crash when run on a pool that has a previously
+ * removed vdev.  Regression for
+ * https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=273663
+ */
+TEST_F(VdevIteratorTest, VdevRemoval)
+{
+	nvlist_t* poolConfig, *rootVdev;
+
+	ASSERT_EQ(0, nvlist_alloc(&rootVdev, NV_UNIQUE_NAME, 0));
+	ASSERT_EQ(0, nvlist_add_uint64(rootVdev, ZPOOL_CONFIG_GUID, 0x5678));
+	/*
+	 * Note: pools with previously-removed top-level VDEVs will contain a
+	 * TLV in their labels that has 0 children.
+	 */
+	ASSERT_EQ(0, nvlist_add_nvlist_array(rootVdev, ZPOOL_CONFIG_CHILDREN,
+				NULL, 0));
+	ASSERT_EQ(0, nvlist_alloc(&poolConfig, NV_UNIQUE_NAME, 0));
+	ASSERT_EQ(0, nvlist_add_uint64(poolConfig,
+			ZPOOL_CONFIG_POOL_GUID, 0x1234));
+	ASSERT_EQ(0, nvlist_add_nvlist(poolConfig, ZPOOL_CONFIG_VDEV_TREE,
+				rootVdev));
+
+	VdevIterator(poolConfig).Each(VdevIteratorTestCB, NULL);
+}
diff --git a/cddl/usr.sbin/zfsd/vdev_iterator.cc b/cddl/usr.sbin/zfsd/vdev_iterator.cc
index f64b0d98440d..e9283108ed3c 100644
--- a/cddl/usr.sbin/zfsd/vdev_iterator.cc
+++ b/cddl/usr.sbin/zfsd/vdev_iterator.cc
@@ -109,10 +109,7 @@ VdevIterator::Next()
 {
 	nvlist_t *vdevConfig;
 
-	if (m_vdevQueue.empty())
-		return (NULL);
-
-	for (;;) {
+	for (vdevConfig = NULL; !m_vdevQueue.empty();) {
 		nvlist_t **vdevChildren;
 		int        result;
 		u_int      numChildren;