From nobody Wed Apr 13 21:03:59 2022 X-Original-To: dev-commits-src-branches@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 488591B4C4E8; Wed, 13 Apr 2022 21:04:00 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Kdw7N1ZC3z3wXM; Wed, 13 Apr 2022 21:04:00 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1649883840; 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=xyhUat8iv/LgDGIVhiLxkVRa8aAuKnykQxOvfAEJuds=; b=k+eJ1EQ8ufk2v7icoNLqPtE0AvY7axTpkEFkb81Gefwj7ANtPqoVv2z0/mtAB+DawpUt0z pbpBJU1fivM/oMxGjmmqUN0Pd5ikSEG8GYG4MwNtV19M+BJk8go9Cg7GRBdyje5iE3y4TG Pt7dbVHN6pkBPE19D2gkyEiFaFL8lU7qDF1p5u4fFZKRq3eejvYBmv2SbY0D7Xb3zTHLDW 9Cn9qmlx5fhMD834bu6oUFoTL4Pjudyo7zb0PkrGZaIL4Kagf6Z5y5aHIMQ1prbLHv0MXC f7bf1E3dJrQ3DnVXvkgsgxGHrv9WwFogZ0vcZ4MVzoHHMZ1QT2lXfOShBdjY4w== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 157491391E; Wed, 13 Apr 2022 21:04:00 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 23DL3xJi012318; Wed, 13 Apr 2022 21:03:59 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 23DL3xP2012317; Wed, 13 Apr 2022 21:03:59 GMT (envelope-from git) Date: Wed, 13 Apr 2022 21:03:59 GMT Message-Id: <202204132103.23DL3xP2012317@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Robert Wing Subject: git: d7d8cc989150 - stable/13 - ffs_mount(): return early if namei() fails to lookup disk device List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rew X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: d7d8cc9891507ec3d2dcd9334be2afade1ed9d0b Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1649883840; 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=xyhUat8iv/LgDGIVhiLxkVRa8aAuKnykQxOvfAEJuds=; b=Oynn+uPa4cAkaD9CS3Q10Nt6BzzLsXbpImjustOwjgU2n9uKgiJj+6MI+M6yjkzcaJN8gB GIwxz6B+AdGmNNDlp4CKtR78MSQC6spc7TR2uw5fg5EUi+GslgK6P74VU7hYhuPpdWjGH6 j+7qYg6NeaXS9uRcjIoiZvjbCu7KTYKxryetFglBud4FCZSe96mw/Z8IFk1JbiR0LjjWI0 BVahZOpf1MAUxruiPvadKeg0DP9UAdA//oWTUj4UqDqUVTA8nJU+F4Rx2WsgXJPhwUfon2 ZAHEfDXrhOTN/ZD2l1eFma29Kra0GrengVD41SVzNtqkpd4JwPwVShAjWSiX4Q== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1649883840; a=rsa-sha256; cv=none; b=J/JoVqC2EY0NPYXVN26lGxo3WGxqzikmp4DjUwHzEPSvze0MOXDk/BltC43Weza87b2acO Fmch5jBvjCjdHSqTM/jeouvh105LFwyQGNI4aW10P6kVHnGICyDjufcuoyp5CpNvQna2+Q v9MG+JHRIhk+ftWSmdXS5/SBiCaKrqGkfFRS9a/2uwVVbusZmAyEWHktPDm9HCWFEJoDJ7 mbgesVCSrHxVgZMZqRceb85ALDBoEIaBlRb0qIOC7q4isdNO+AWY3oYGJQTWod8qSw5hAW tSjaM4dlGQ7qF90Cpmu/exkIhAYRuHyhx5CP/lSh0swiZKCqBU1u/v+O80/3Og== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by rew: URL: https://cgit.FreeBSD.org/src/commit/?id=d7d8cc9891507ec3d2dcd9334be2afade1ed9d0b commit d7d8cc9891507ec3d2dcd9334be2afade1ed9d0b Author: Robert Wing AuthorDate: 2022-03-07 19:18:03 +0000 Commit: Robert Wing CommitDate: 2022-04-13 20:59:12 +0000 ffs_mount(): return early if namei() fails to lookup disk device With soft updates enabled, an INVARIANTS panic is hit in ffs_unmount(). The problem occurs in ffs_mount() when upgrading a mount from ro->rw. During a mount update, the soft update code gets set up but doesn't get cleaned up if namei() fails when looking up the disk device. Avoid this scenario by looking up the disk device first and bail early if the namei() lookup fails. PR: 256511 Reviewed by: mckusick, kib Differential Revision: https://reviews.freebsd.org/D30870 (cherry picked from commit 0455cc7104ec8e8dd54b3f44049112a5a8ca329c) --- sys/ufs/ffs/ffs_vfsops.c | 148 +++++++++++++++++++++++------------------------ 1 file changed, 72 insertions(+), 76 deletions(-) diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c index 89fdc4336834..2eae7c5fa630 100644 --- a/sys/ufs/ffs/ffs_vfsops.c +++ b/sys/ufs/ffs/ffs_vfsops.c @@ -436,15 +436,84 @@ ffs_mount(struct mount *mp) mp->mnt_kern_flag &= ~MNTK_FPLOOKUP; mp->mnt_flag |= mntorflags; MNT_IUNLOCK(mp); + + /* + * Must not call namei() while owning busy ref. + */ + if (mp->mnt_flag & MNT_UPDATE) + vfs_unbusy(mp); + + /* + * Not an update, or updating the name: look up the name + * and verify that it refers to a sensible disk device. + */ + NDINIT(&ndp, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, fspec, td); + error = namei(&ndp); + if ((mp->mnt_flag & MNT_UPDATE) != 0) { + /* + * Unmount does not start if MNT_UPDATE is set. Mount + * update busies mp before setting MNT_UPDATE. We + * must be able to retain our busy ref successfully, + * without sleep. + */ + error1 = vfs_busy(mp, MBF_NOWAIT); + MPASS(error1 == 0); + } + if (error != 0) + return (error); + NDFREE(&ndp, NDF_ONLY_PNBUF); + if (!vn_isdisk_error(ndp.ni_vp, &error)) { + vput(ndp.ni_vp); + return (error); + } + /* - * If updating, check whether changing from read-only to - * read/write; if there is no device name, that's all we do. + * If mount by non-root, then verify that user has necessary + * permissions on the device. + */ + accmode = VREAD; + if ((mp->mnt_flag & MNT_RDONLY) == 0) + accmode |= VWRITE; + error = VOP_ACCESS(ndp.ni_vp, accmode, td->td_ucred, td); + if (error) + error = priv_check(td, PRIV_VFS_MOUNT_PERM); + if (error) { + vput(ndp.ni_vp); + return (error); + } + + /* + * New mount + * + * We need the name for the mount point (also used for + * "last mounted on") copied in. If an error occurs, + * the mount point is discarded by the upper level code. + * Note that vfs_mount_alloc() populates f_mntonname for us. */ - if (mp->mnt_flag & MNT_UPDATE) { + if ((mp->mnt_flag & MNT_UPDATE) == 0) { + if ((error = ffs_mountfs(ndp.ni_vp, mp, td)) != 0) { + vrele(ndp.ni_vp); + return (error); + } + } else { + /* + * When updating, check whether changing from read-only to + * read/write; if there is no device name, that's all we do. + */ ump = VFSTOUFS(mp); fs = ump->um_fs; odevvp = ump->um_odevvp; devvp = ump->um_devvp; + + /* + * If it's not the same vnode, or at least the same device + * then it's not correct. + */ + if (ndp.ni_vp->v_rdev != ump->um_odevvp->v_rdev) + error = EINVAL; /* needs translation */ + vput(ndp.ni_vp); + if (error) + return (error); if (fs->fs_ronly == 0 && vfs_flagopt(mp->mnt_optnew, "ro", NULL, 0)) { /* @@ -647,79 +716,6 @@ ffs_mount(struct mount *mp) */ if (mp->mnt_flag & MNT_SNAPSHOT) return (ffs_snapshot(mp, fspec)); - - /* - * Must not call namei() while owning busy ref. - */ - vfs_unbusy(mp); - } - - /* - * Not an update, or updating the name: look up the name - * and verify that it refers to a sensible disk device. - */ - NDINIT(&ndp, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, fspec, td); - error = namei(&ndp); - if ((mp->mnt_flag & MNT_UPDATE) != 0) { - /* - * Unmount does not start if MNT_UPDATE is set. Mount - * update busies mp before setting MNT_UPDATE. We - * must be able to retain our busy ref succesfully, - * without sleep. - */ - error1 = vfs_busy(mp, MBF_NOWAIT); - MPASS(error1 == 0); - } - if (error != 0) - return (error); - NDFREE(&ndp, NDF_ONLY_PNBUF); - devvp = ndp.ni_vp; - if (!vn_isdisk_error(devvp, &error)) { - vput(devvp); - return (error); - } - - /* - * If mount by non-root, then verify that user has necessary - * permissions on the device. - */ - accmode = VREAD; - if ((mp->mnt_flag & MNT_RDONLY) == 0) - accmode |= VWRITE; - error = VOP_ACCESS(devvp, accmode, td->td_ucred, td); - if (error) - error = priv_check(td, PRIV_VFS_MOUNT_PERM); - if (error) { - vput(devvp); - return (error); - } - - if (mp->mnt_flag & MNT_UPDATE) { - /* - * Update only - * - * If it's not the same vnode, or at least the same device - * then it's not correct. - */ - - if (devvp->v_rdev != ump->um_devvp->v_rdev) - error = EINVAL; /* needs translation */ - vput(devvp); - if (error) - return (error); - } else { - /* - * New mount - * - * We need the name for the mount point (also used for - * "last mounted on") copied in. If an error occurs, - * the mount point is discarded by the upper level code. - * Note that vfs_mount_alloc() populates f_mntonname for us. - */ - if ((error = ffs_mountfs(devvp, mp, td)) != 0) { - vrele(devvp); - return (error); - } } MNT_ILOCK(mp);