From nobody Mon Mar 07 19:52:13 2022 X-Original-To: dev-commits-src-main@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 1E67919FA2E0; Mon, 7 Mar 2022 19:52:14 +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 4KC8Hf09Q1z3K9g; Mon, 7 Mar 2022 19:52:14 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1646682734; 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=/+7L/0lEEn+E45hDv+UMCEXI3lcWgeZzuGacGUsD9nM=; b=gW2bYO2MVgAmj1HWL5c0spMVLh1JIxYwDI17FlDyJQUkxgPDBpmtsNkmL4cNzQ7UGgMEuJ gIyFRdxLNx+rhvaUSUOt4rsHBfE5sTZ8qX3/Csh849MUv71KaY/+9rO54l3J6UBogprjZv TkNvVovxXuiWNRz5iRUmFx2gvamZwg1g89qPxoUuvGpNOQqyYIOx4ebnIu9qIhGL4Frl3I lUZXjtK1ZafEeSjkG9QsNuKEB9E5TZJov18Frutfzyobl3wtvhACEE310DW8kH3bh47vF/ xdFp4qjGcBLJyIhZHdU7h/tP3bXdPATY+X/g5XXa34x+Qggwdn8pEAoAbYA53A== 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 DA8BA69EE; Mon, 7 Mar 2022 19:52:13 +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 227JqDuh007635; Mon, 7 Mar 2022 19:52:13 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 227JqD0m007634; Mon, 7 Mar 2022 19:52:13 GMT (envelope-from git) Date: Mon, 7 Mar 2022 19:52:13 GMT Message-Id: <202203071952.227JqD0m007634@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Robert Wing Subject: git: 0455cc7104ec - main - ffs_mount(): return early if namei() fails to lookup disk device List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@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/main X-Git-Reftype: branch X-Git-Commit: 0455cc7104ec8e8dd54b3f44049112a5a8ca329c Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1646682734; 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=/+7L/0lEEn+E45hDv+UMCEXI3lcWgeZzuGacGUsD9nM=; b=kTiSFld+IFYTD6GrBkuR20rSIjinl1eFFXAnPJCBRpKvaZ8rLBX7LU/+IpvxXVdmCvRVsI Q7hVfsUZAT01+z1ucDRm4W27pKqP5IJCTGkzPuRHGjPVakwW2HImrdBW5HsUBp0CQRsemb 4feplK+gfoLycBK7UxtzRWyx0bVOghFwVcT0AfFF3I32Esx5+x5QJ7htXPAjpcvSr6UGGx 5hHeA8g8ih169pFkmNmU3Ne+miMEbVfprJ0mMQG5mElW64L18PO4XOnCv1R9GytPn0lT+Y VjFXiOn7oqNZB8Mf+8lIAOWBnfrB1Hm4LdpJ/CzBDJoOICdiMXGgLmmZbbhNAQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1646682734; a=rsa-sha256; cv=none; b=RW7Py0HW/7RZ7qmiJS/xciam5v6pCax6xa5arzLXKpQXupnvq6zRoLmEuhJya4uk+D1O+h MLFGn6OZRFn9TxcW0jNXiMlaTicASHcWig/2Si4yiTd8eoQoJJk67kGMbZG9ogL0UnP6UQ tbnhLyL+N2g0YYNefA8ry+J660Cn5aHQjdXu6dRMr7WEyKxq+JstMWsfbBWBHlqaojqqYE HZF/UsUyh8JkWEQ6ZCKj7eTrnp9fr1VNa30d3pYHWjMKRRpA03OSMqY5NYwa5s7jrLsvtH X/8xXmNxjcNp2YAPncVtfQWe1iPGsbuhs3Ggr5jk1os6HwfU4RgXxAhN9njZ/w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by rew: URL: https://cgit.FreeBSD.org/src/commit/?id=0455cc7104ec8e8dd54b3f44049112a5a8ca329c commit 0455cc7104ec8e8dd54b3f44049112a5a8ca329c Author: Robert Wing AuthorDate: 2022-03-07 19:18:03 +0000 Commit: Robert Wing CommitDate: 2022-03-07 19:48:44 +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 MFC After: 2 weeks Reviewed by: mckusick, kib Differential Revision: https://reviews.freebsd.org/D30870 --- 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 a2a361e813e4..578e71014a23 100644 --- a/sys/ufs/ffs/ffs_vfsops.c +++ b/sys/ufs/ffs/ffs_vfsops.c @@ -409,15 +409,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); + 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)) { /* @@ -620,79 +689,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); - 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);