From nobody Mon Jul 27 17:33:31 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4h85Ks1Rfrz6nHsm for ; Mon, 27 Jul 2026 17:33:37 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "YR1" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4h85Ks0T5lz3Ks9 for ; Mon, 27 Jul 2026 17:33:37 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1785173617; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=g+sG/o3ulNxhz8AFFttkHVek+chV3+IT1TOyk2pm7dE=; b=nnrZuXhKeb8Q6k4iYOeo4PfmBVLdQdfBO7yAJPEGxzlC/6esV+0B5CZ0H2eYQDmS/Cj8Xm qRWJgMsoEX2NYXa0AADqZL6GQEWrHmsA5Zd6aVvXkoJVvbDG6d0gAO6FjZezlPe9Hxiii4 hPdXumgHB+ONDXYhRP4Bsw+m3s0wvAmtM4lHF3KXLzHTfGUlOirh/FJALSqMJ+svJ5q19w yNqd4r9HzmeIJn5Z7/iqEPLNvQEfmdiJkeqGKVkoA9gHbKP6iPTabA3BrT/z26X1BfiNdb DFRYxdtBaZh5+lUNbjJ/lfabRvDspYxEZCy+eLl+HnLB6vaFkwteUIm5pk75Hw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1785173617; a=rsa-sha256; cv=none; b=r1Y+ae3INsIcGS2sK/MAiJ5PeFGA1oEvyKjKCMmKTkuPIWALLKWhna+IcVtpR4wdfAYYSj ijozNqwpGebnPbx9RI89HK1yS290U8CYtwDsrOHtWuciWCDgPT1vBjoiNhCl1GOkpsWdDt x0+F6pROpKbTwU5g2cqbIvDOfovhxbKveng9AcGsqC0WwjDO/05B7mWfbxONvboPdyhyBf YaY7Sx+lq8BGAOOf92fTVW8h1NgyF1H1JTfDC4j3I1EXC8Q3XZSRgqrDVe0na9v9sq62QH PpixkjupfCRY0nrjgjqhA0FH5DXn97ewe+j41RihpOAdOkM7nc177v9/zvXDNw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1785173617; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=g+sG/o3ulNxhz8AFFttkHVek+chV3+IT1TOyk2pm7dE=; b=xwI2xNH3qlAcJxbmx+rEpw60JEA6m4+ALqoFaayPSxyITQ+KM5XawdO0CqbEmNsHT3IMRo plWzSeyQTZzcOEjgroHhvmKTO1PWM0n34N6yF/Yiz4WbUECyw7KM1ZBj2zLX73RaOLm0lE ie7qq0psAdN9nLJ77iQ7PKuWEA8H7vU3FaNuKN7EDULPdD/Tvs5M0cVhtKDdkG2QDXyEHw keX4+qUjs34ekZyo63jartDjpqJG8071n3x9bIQC7kXhgVhJDvaQRn4nGsxqQj+VHT/hgw w55l8S5IZ8Oty+wy4ECvs9KSTLWaun7ocqJNLunICjw/ddyJKsupBDO9kI+iAQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4h85Kr69Kkz1Hkk for ; Mon, 27 Jul 2026 17:33:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 25c32 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Mon, 27 Jul 2026 17:33:31 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: ae6db85b3c14 - stable/15 - FreeBSD: Fix zvol teardown races List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/stable/15 X-Git-Reftype: branch X-Git-Commit: ae6db85b3c14771c920185b516e0bb5e8a65912c Auto-Submitted: auto-generated Date: Mon, 27 Jul 2026 17:33:31 +0000 Message-Id: <6a67966b.25c32.120d1db6@gitrepo.freebsd.org> The branch stable/15 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=ae6db85b3c14771c920185b516e0bb5e8a65912c commit ae6db85b3c14771c920185b516e0bb5e8a65912c Author: Mark Johnston AuthorDate: 2026-02-02 01:37:22 +0000 Commit: Mark Johnston CommitDate: 2026-07-27 17:29:03 +0000 FreeBSD: Fix zvol teardown races zvol_geom_open() may be called to taste an orphaned provider. The test for pp->private == NULL there is racy as no locks are synchronizing the test. Use the GEOM topology lock to interlock the pp->private == NULL test with the zvol state checks. This establishes a new lock order but I believe this is necessary. Set pp->private = NULL under the GEOM topology lock instead of the per-zvol state lock. Modify zvol_os_rename_minor() to drop the zvol state lock to avoid a lock order reversal with the topology lock. Also reverse the order of tests in zvol_geom_open() and zvol_cdev_open() as at least zvol_geom_open() may race with zvol_os_remove_minor(), which sets zv->zv_zso = NULL. Testing for ZVOL_REMOVING first avoids a race which can lead to a NULL pointer dereference. Add a new OS-specific flag to handle the case where zvol_geom_open() drops all locks in order to avoid a lock order reversal when acquiring the suspend lock as the open count transitions 0->1. I don't see anything preventing zvol_os_remove_minor() from racing there. Reviewed-by: Brian Behlendorf Reviewed-by: Alexander Motin Signed-off-by: Mark Johnston Closes #18191 (cherry picked from commit 6de1457a2d0ea9c95edddb9e2d3d8780ae79da3f) --- .../openzfs/module/os/freebsd/zfs/zvol_os.c | 78 ++++++++++++---------- 1 file changed, 44 insertions(+), 34 deletions(-) diff --git a/sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c b/sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c index dc30f6dd939c..fca99c42f73c 100644 --- a/sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c +++ b/sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c @@ -128,7 +128,8 @@ struct zvol_state_os { struct g_provider *zsg_provider; } _zso_geom; } _zso_state; - int zso_dying; + boolean_t zso_opening; + boolean_t zso_dying; }; static uint32_t zvol_minors; @@ -226,12 +227,13 @@ zvol_geom_open(struct g_provider *pp, int flag, int count) } retry: - zv = atomic_load_ptr(&pp->private); + zv = pp->private; if (zv == NULL) return (SET_ERROR(ENXIO)); mutex_enter(&zv->zv_state_lock); - if (zv->zv_zso->zso_dying || zv->zv_flags & ZVOL_REMOVING) { + g_topology_unlock(); + if (zv->zv_flags & ZVOL_REMOVING || zv->zv_zso->zso_dying) { err = SET_ERROR(ENXIO); goto out_locked; } @@ -245,18 +247,16 @@ retry: if (zv->zv_open_count == 0) { drop_suspend = B_TRUE; if (!rw_tryenter(&zv->zv_suspend_lock, ZVOL_RW_READER)) { - mutex_exit(&zv->zv_state_lock); - /* - * Removal may happen while the locks are down, so - * we can't trust zv any longer; we have to start over. + * Set a flag to interlock with zvol_os_remove_minor() + * while locks are dropped. */ - zv = atomic_load_ptr(&pp->private); - if (zv == NULL) - return (SET_ERROR(ENXIO)); - + zv->zv_zso->zso_opening = B_TRUE; + mutex_exit(&zv->zv_state_lock); rw_enter(&zv->zv_suspend_lock, ZVOL_RW_READER); mutex_enter(&zv->zv_state_lock); + zv->zv_zso->zso_opening = B_FALSE; + cv_broadcast(&zv->zv_removing_cv); if (zv->zv_zso->zso_dying || zv->zv_flags & ZVOL_REMOVING) { @@ -289,6 +289,7 @@ retry: rw_exit(&zv->zv_suspend_lock); drop_suspend = B_FALSE; kern_yield(PRI_USER); + g_topology_lock(); goto retry; } else { drop_namespace = B_TRUE; @@ -337,6 +338,7 @@ out_locked: mutex_exit(&zv->zv_state_lock); if (drop_suspend) rw_exit(&zv->zv_suspend_lock); + g_topology_lock(); return (err); } @@ -348,11 +350,12 @@ zvol_geom_close(struct g_provider *pp, int flag, int count) boolean_t drop_suspend = B_TRUE; int new_open_count; - zv = atomic_load_ptr(&pp->private); + zv = pp->private; if (zv == NULL) return (SET_ERROR(ENXIO)); mutex_enter(&zv->zv_state_lock); + g_topology_unlock(); if (zv->zv_flags & ZVOL_EXCL) { ASSERT3U(zv->zv_open_count, ==, 1); zv->zv_flags &= ~ZVOL_EXCL; @@ -413,6 +416,7 @@ zvol_geom_close(struct g_provider *pp, int flag, int count) if (drop_suspend) rw_exit(&zv->zv_suspend_lock); + g_topology_lock(); return (0); } @@ -448,7 +452,7 @@ zvol_geom_access(struct g_provider *pp, int acr, int acw, int ace) ("Unsupported access request to %s (acr=%d, acw=%d, ace=%d).", pp->name, acr, acw, ace)); - if (atomic_load_ptr(&pp->private) == NULL) { + if (pp->private == NULL) { if (acr <= 0 && acw <= 0 && ace <= 0) return (0); return (pp->error); @@ -473,24 +477,16 @@ zvol_geom_access(struct g_provider *pp, int acr, int acw, int ace) if (acw != 0) flags |= FWRITE; - g_topology_unlock(); if (count > 0) error = zvol_geom_open(pp, flags, count); else error = zvol_geom_close(pp, flags, -count); - g_topology_lock(); return (error); } static void zvol_geom_bio_start(struct bio *bp) { - zvol_state_t *zv = bp->bio_to->private; - - if (zv == NULL) { - g_io_deliver(bp, ENXIO); - return; - } if (bp->bio_cmd == BIO_GETATTR) { if (zvol_geom_bio_getattr(bp)) g_io_deliver(bp, EOPNOTSUPP); @@ -507,7 +503,10 @@ zvol_geom_bio_getattr(struct bio *bp) zvol_state_t *zv; zv = bp->bio_to->private; - ASSERT3P(zv, !=, NULL); + if (zv == NULL) { + g_io_deliver(bp, ENXIO); + return (0); + } spa_t *spa = dmu_objset_spa(zv->zv_objset); uint64_t refd, avail, usedobjs, availobjs; @@ -1258,17 +1257,25 @@ zvol_os_rename_minor(zvol_state_t *zv, const char *newname) zv->zv_hash = zvol_name_hash(newname); hlist_del(&zv->zv_hlink); hlist_add_head(&zv->zv_hlink, ZVOL_HT_HEAD(zv->zv_hash)); + strlcpy(zv->zv_name, newname, sizeof (zv->zv_name)); + dataset_kstats_rename(&zv->zv_kstat, newname); if (zv->zv_volmode == ZFS_VOLMODE_GEOM) { struct zvol_state_geom *zsg = &zv->zv_zso->zso_geom; - struct g_provider *pp = zsg->zsg_provider; + struct g_provider *pp; struct g_geom *gp; + mutex_exit(&zv->zv_state_lock); g_topology_lock(); + pp = zsg->zsg_provider; + if (pp->private == NULL) { + g_topology_unlock(); + mutex_enter(&zv->zv_state_lock); + return (SET_ERROR(ENXIO)); + } gp = pp->geom; ASSERT3P(gp, !=, NULL); - zsg->zsg_provider = NULL; g_wither_provider(pp, ENXIO); pp = g_new_providerf(gp, "%s/%s", ZVOL_DRIVER, newname); @@ -1278,6 +1285,7 @@ zvol_os_rename_minor(zvol_state_t *zv, const char *newname) pp->private = zv; zsg->zsg_provider = pp; g_error_provider(pp, 0); + mutex_enter(&zv->zv_state_lock); g_topology_unlock(); } else if (zv->zv_volmode == ZFS_VOLMODE_DEV) { struct zvol_state_dev *zsd = &zv->zv_zso->zso_dev; @@ -1310,8 +1318,6 @@ zvol_os_rename_minor(zvol_state_t *zv, const char *newname) zsd->zsd_cdev = dev; } } - strlcpy(zv->zv_name, newname, sizeof (zv->zv_name)); - dataset_kstats_rename(&zv->zv_kstat, newname); return (error); } @@ -1400,27 +1406,31 @@ zvol_alloc(const char *name, uint64_t volsize, uint64_t volblocksize, void zvol_os_remove_minor(zvol_state_t *zv) { + struct zvol_state_os *zso = zv->zv_zso; + ASSERT(MUTEX_HELD(&zv->zv_state_lock)); ASSERT0(zv->zv_open_count); ASSERT0(atomic_read(&zv->zv_suspend_ref)); ASSERT(zv->zv_flags & ZVOL_REMOVING); - struct zvol_state_os *zso = zv->zv_zso; - zv->zv_zso = NULL; - if (zv->zv_volmode == ZFS_VOLMODE_GEOM) { struct zvol_state_geom *zsg = &zso->zso_geom; - struct g_provider *pp = zsg->zsg_provider; - atomic_store_ptr(&pp->private, NULL); - mutex_exit(&zv->zv_state_lock); + struct g_provider *pp; + while (zso->zso_opening) + cv_wait(&zv->zv_removing_cv, &zv->zv_state_lock); + zv->zv_zso = NULL; + mutex_exit(&zv->zv_state_lock); g_topology_lock(); + pp = zsg->zsg_provider; + pp->private = NULL; g_wither_geom(pp->geom, ENXIO); g_topology_unlock(); } else if (zv->zv_volmode == ZFS_VOLMODE_DEV) { struct zvol_state_dev *zsd = &zso->zso_dev; struct cdev *dev = zsd->zsd_cdev; + zv->zv_zso = NULL; if (dev != NULL) atomic_store_ptr(&dev->si_drv2, NULL); mutex_exit(&zv->zv_state_lock); @@ -1565,10 +1575,10 @@ zvol_os_update_volsize(zvol_state_t *zv, uint64_t volsize) zv->zv_volsize = volsize; if (zv->zv_volmode == ZFS_VOLMODE_GEOM) { struct zvol_state_geom *zsg = &zv->zv_zso->zso_geom; - struct g_provider *pp = zsg->zsg_provider; + struct g_provider *pp; g_topology_lock(); - + pp = zsg->zsg_provider; if (pp->private == NULL) { g_topology_unlock(); return (SET_ERROR(ENXIO));