dnode: fix heap-use-after-free in dnode_rele_and_unlock()

The ZFS_DEBUG-only assert in dnode_rele_and_unlock() dereferenced dnh,
the dnode's handle, after mutex_exit(&dn->dn_mtx), below the comment
stating "dnode could get destroyed at this point, so don't use it
anymore".

For an objset's special dnodes that is a real use-after-free. They have
no dnode block (dn_dbuf == NULL), so nothing pins their handle; it is
embedded in the objset_t. Only dn_nodnholds orders a release against
teardown, and this function is what signals it: on the last hold it
broadcasts and drops dn_mtx, freeing dnode_special_close() to destroy
that handle and, via dmu_objset_evict_done(), the objset it lives in.
The assert then reads it.

Read the zrlock ownership before dropping dn_mtx instead, where dn_mtx
still blocks dnode_special_close() and, for an ordinary dnode, the
reference from dbuf_add_ref(db, dnh) is not yet released.

ZFS_DEBUG-only, no change to non-debug builds.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Richard Yao <[email protected]>
Signed-off-by: Alexander Moch <[email protected]>
Closes #19016

module/zfs/dnode.c

@@ -1779,8 +1779,6 @@ dnode_rele_and_unlock(dnode_t *dn, const void *tag, boolean_t evicting)
 	refs = zfs_refcount_remove(&dn->dn_holds, tag);
 	if (refs == 0)
 		cv_broadcast(&dn->dn_nodnholds);
-	mutex_exit(&dn->dn_mtx);
-	/* dnode could get destroyed at this point, so don't use it anymore */
 
 	/*
 	 * It's unsafe to release the last hold on a dnode by dnode_rele() or
@@ -1795,6 +1793,12 @@ dnode_rele_and_unlock(dnode_t *dn, const void *tag, boolean_t evicting)
 	ASSERT(refs > 0 || zrl_owner(&dnh->dnh_zrlock) != curthread);
 #endif
 
+	mutex_exit(&dn->dn_mtx);
+	/*
+	 * After the dn_mtx is released the dn and dnh may be destroyed,
+	 * they are no longer safe to use after this point.
+	 */
+
 	/* NOTE: the DNODE_DNODE does not have a dn_dbuf */
 	if (refs == 0 && db != NULL) {
 		/*