From nobody Sat Jan 01 03:39:05 2022 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 43B82191F2E4; Sat, 1 Jan 2022 03:39:06 +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 4JQnmn4J56z3sKn; Sat, 1 Jan 2022 03:39:05 +0000 (UTC) (envelope-from git@FreeBSD.org) 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 6618617A45; Sat, 1 Jan 2022 03:39:05 +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 2013d5ID095775; Sat, 1 Jan 2022 03:39:05 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 2013d52d095774; Sat, 1 Jan 2022 03:39:05 GMT (envelope-from git) Date: Sat, 1 Jan 2022 03:39:05 GMT Message-Id: <202201010339.2013d52d095774@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Alan Somers Subject: git: 1613087a8127 - main - fusefs: fix .. lookups when the parent has been reclaimed. 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: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: asomers X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 1613087a8127122b03a3730046d051adf4edd14f Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1641008345; 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=P0qsEeJGZVzlfrbur9+Ke1+tw4PjZdzpSixbDDaFuKs=; b=wrJt+Ggh4jScmT3e8C5lsMe1MVX1JIxGBAY2gwAsggGR78iniX2Yeu2crU3Rn29qQhOZk7 pf2u0TLEA0+EXZEsLiUmTX5kNpRRblb80rqRPU6H/FYmuBMy6RXrghIIM+fykAtKnViEh6 Gpo0MMfByFHrfhmDhmisBOdlZAQ5l40biLiVS5cCJAeB2/0EsFTn7oLPQ4sXj/r67mXoZX nxvbFfYYkEG7QvNgvD2ln/v6nvqL8iwu6lh5aOG2m6BimNHK1eB3g+vXdJQBqBYnoT676j lcHwIzo6O01dZlg49xpQtiri4Qt3n8PyIaM0k6nlAjZhFL3gh1hfNUJ1/FvEOw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1641008345; a=rsa-sha256; cv=none; b=gJJh76BvWSryRzkXf4sG3gnMFIzW+xKfk4JVlOvOovX8Z3bj8NQZg/lurRp/j4VWAEceGO drhIflVtXFnelD1RAv1WUx/Jug+SEDCDKPvOiIRUONMYPi6fQKuYkphDmd1h63EJ4R6vnm 4qFKcNEvKC44ZT4sKyLniUPkuDVnipSz7CpTWJN+Ju2gSi6ymtmYhrE0iTd70M1XOHn1K/ lX131uzBzGM/8nNKdTmM6QJsbXfFuLnxdQMPss7zYVVBlfzDXvEzjUh/qUGbISlLSeuGcR 2YOlXuL8NVVaGIb0wujQdZ/PBbCu/HAEDUzJK+KsGVL8+Yq765DzV71OsmZlHQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by asomers: URL: https://cgit.FreeBSD.org/src/commit/?id=1613087a8127122b03a3730046d051adf4edd14f commit 1613087a8127122b03a3730046d051adf4edd14f Author: Alan Somers AuthorDate: 2021-12-02 02:50:47 +0000 Commit: Alan Somers CommitDate: 2022-01-01 03:38:27 +0000 fusefs: fix .. lookups when the parent has been reclaimed. By default, FUSE file systems are assumed not to support lookups for "." and "..". They must opt-in to that. To cope with this limitation, the fusefs kernel module caches every fuse vnode's parent's inode number, and uses that during VOP_LOOKUP for "..". But if the parent's vnode has been reclaimed that won't be possible. Previously we paniced in this situation. Now, we'll return ESTALE instead. Or, if the file system has opted into ".." lookups, we'll just do that instead. This commit also fixes VOP_LOOKUP to respect the cache timeout for ".." lookups, if the FUSE file system specified a finite timeout. PR: 259974 MFC after: 2 weeks Reviewed by: pfg Differential Revision: https://reviews.freebsd.org/D33239 --- sys/fs/fuse/fuse_vnops.c | 26 ++++-- tests/sys/fs/fusefs/lookup.cc | 203 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 221 insertions(+), 8 deletions(-) diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index ada7dc1dcc81..640f9d2f8243 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -1320,9 +1320,15 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) else if ((err = fuse_internal_access(dvp, VEXEC, td, cred))) return err; - if (flags & ISDOTDOT) { - KASSERT(VTOFUD(dvp)->flag & FN_PARENT_NID, - ("Looking up .. is TODO")); + if ((flags & ISDOTDOT) && !(data->dataflags & FSESS_EXPORT_SUPPORT)) + { + if (!(VTOFUD(dvp)->flag & FN_PARENT_NID)) { + /* + * Since the file system doesn't support ".." lookups, + * we have no way to find this entry. + */ + return ESTALE; + } nid = VTOFUD(dvp)->parent_nid; if (nid == 0) return ENOENT; @@ -1375,9 +1381,8 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) return err; } - nid = VTOI(dvp); fdisp_init(&fdi, cnp->cn_namelen + 1); - fdisp_make(&fdi, FUSE_LOOKUP, mp, nid, td, cred); + fdisp_make(&fdi, FUSE_LOOKUP, mp, VTOI(dvp), td, cred); memcpy(fdi.indata, cnp->cn_nameptr, cnp->cn_namelen); ((char *)fdi.indata)[cnp->cn_namelen] = '\0'; @@ -1396,11 +1401,16 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) lookup_err = ENOENT; if (cnp->cn_flags & MAKEENTRY) { fuse_validity_2_timespec(feo, &timeout); + /* Use the same entry_time for .. as for + * the file itself. That doesn't honor + * exactly what the fuse server tells + * us, but to do otherwise would require + * another cache lookup at this point. + */ + struct timespec *dtsp = NULL; cache_enter_time(dvp, *vpp, cnp, - &timeout, NULL); + &timeout, dtsp); } - } else if (nid == FUSE_ROOT_ID) { - lookup_err = EINVAL; } vtyp = IFTOVT(feo->attr.mode); filesize = feo->attr.size; diff --git a/tests/sys/fs/fusefs/lookup.cc b/tests/sys/fs/fusefs/lookup.cc index d301990c2048..2dfa10730ec8 100644 --- a/tests/sys/fs/fusefs/lookup.cc +++ b/tests/sys/fs/fusefs/lookup.cc @@ -31,6 +31,10 @@ */ extern "C" { +#include +#include + +#include #include } @@ -40,6 +44,7 @@ extern "C" { using namespace testing; class Lookup: public FuseTest {}; + class Lookup_7_8: public Lookup { public: virtual void SetUp() { @@ -48,6 +53,14 @@ virtual void SetUp() { } }; +class LookupExportable: public Lookup { +public: +virtual void SetUp() { + m_init_flags = FUSE_EXPORT_SUPPORT; + Lookup::SetUp(); +} +}; + /* * If lookup returns a non-zero cache timeout, then subsequent VOP_GETATTRs * should use the cached attributes, rather than query the daemon @@ -181,6 +194,89 @@ TEST_F(Lookup, dotdot) ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno); } +/* + * Lookup ".." when that vnode's entry cache has timed out, but its child's + * hasn't. Since this file system doesn't set FUSE_EXPORT_SUPPORT, we have no + * choice but to use the cached entry, even though it expired. + */ +TEST_F(Lookup, dotdot_entry_cache_timeout) +{ + uint64_t foo_ino = 42; + uint64_t bar_ino = 43; + + EXPECT_LOOKUP(FUSE_ROOT_ID, "foo") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = foo_ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = 0; // immediate timeout + }))); + EXPECT_LOOKUP(foo_ino, "bar") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = bar_ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = UINT64_MAX; + }))); + expect_opendir(bar_ino); + + int fd = open("mountpoint/foo/bar", O_EXEC| O_DIRECTORY); + ASSERT_LE(0, fd) << strerror(errno); + EXPECT_EQ(0, faccessat(fd, "../..", F_OK, 0)) << strerror(errno); +} + +/* + * Lookup ".." for a vnode with no valid parent nid + * Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=259974 + * Since the file system is not exportable, we have no choice but to return an + * error. + */ +TEST_F(Lookup, dotdot_no_parent_nid) +{ + uint64_t foo_ino = 42; + uint64_t bar_ino = 43; + int fd; + + EXPECT_LOOKUP(FUSE_ROOT_ID, "foo") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = foo_ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = UINT64_MAX; + }))); + EXPECT_LOOKUP(foo_ino, "bar") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = bar_ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = UINT64_MAX; + }))); + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in.header.opcode == FUSE_OPENDIR); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, open); + }))); + expect_forget(FUSE_ROOT_ID, 1, NULL); + expect_forget(foo_ino, 1, NULL); + + fd = open("mountpoint/foo/bar", O_EXEC| O_DIRECTORY); + ASSERT_LE(0, fd) << strerror(errno); + // Try (and fail) to unmount the file system, to reclaim the mountpoint + // and foo vnodes. + ASSERT_NE(0, unmount("mountpoint", 0)); + EXPECT_EQ(EBUSY, errno); + nap(); // Because vnode reclamation is asynchronous + EXPECT_NE(0, faccessat(fd, "../..", F_OK, 0)); + EXPECT_EQ(ESTALE, errno); +} + TEST_F(Lookup, enoent) { const char FULLPATH[] = "mountpoint/does_not_exist"; @@ -398,4 +494,111 @@ TEST_F(Lookup_7_8, ok) ASSERT_EQ(0, access(FULLPATH, F_OK)) << strerror(errno); } +/* + * Lookup ".." when that vnode's entry cache has timed out, but its child's + * hasn't. + */ +TEST_F(LookupExportable, dotdot_entry_cache_timeout) +{ + uint64_t foo_ino = 42; + uint64_t bar_ino = 43; + + EXPECT_LOOKUP(FUSE_ROOT_ID, "foo") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = foo_ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = 0; // immediate timeout + }))); + EXPECT_LOOKUP(foo_ino, "bar") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = bar_ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = UINT64_MAX; + }))); + expect_opendir(bar_ino); + EXPECT_LOOKUP(foo_ino, "..") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = FUSE_ROOT_ID; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = UINT64_MAX; + }))); + + int fd = open("mountpoint/foo/bar", O_EXEC| O_DIRECTORY); + ASSERT_LE(0, fd) << strerror(errno); + /* FreeBSD's fusefs driver always uses the same cache expiration time + * for ".." as for the directory itself. So we need to look up two + * levels to find an expired ".." cache entry. + */ + EXPECT_EQ(0, faccessat(fd, "../..", F_OK, 0)) << strerror(errno); +} + +/* + * Lookup ".." for a vnode with no valid parent nid + * Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=259974 + * Since the file system is exportable, we should resolve the problem by + * sending a FUSE_LOOKUP for "..". + */ +TEST_F(LookupExportable, dotdot_no_parent_nid) +{ + uint64_t foo_ino = 42; + uint64_t bar_ino = 43; + int fd; + + EXPECT_LOOKUP(FUSE_ROOT_ID, "foo") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = foo_ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = UINT64_MAX; + }))); + EXPECT_LOOKUP(foo_ino, "bar") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = bar_ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = UINT64_MAX; + }))); + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in.header.opcode == FUSE_OPENDIR); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, open); + }))); + expect_forget(FUSE_ROOT_ID, 1, NULL); + expect_forget(foo_ino, 1, NULL); + EXPECT_LOOKUP(bar_ino, "..") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = foo_ino; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = UINT64_MAX; + }))); + EXPECT_LOOKUP(foo_ino, "..") + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { + SET_OUT_HEADER_LEN(out, entry); + out.body.entry.attr.mode = S_IFDIR | 0755; + out.body.entry.nodeid = FUSE_ROOT_ID; + out.body.entry.attr_valid = UINT64_MAX; + out.body.entry.entry_valid = UINT64_MAX; + }))); + fd = open("mountpoint/foo/bar", O_EXEC| O_DIRECTORY); + ASSERT_LE(0, fd) << strerror(errno); + // Try (and fail) to unmount the file system, to reclaim the mountpoint + // and foo vnodes. + ASSERT_NE(0, unmount("mountpoint", 0)); + EXPECT_EQ(EBUSY, errno); + nap(); // Because vnode reclamation is asynchronous + EXPECT_EQ(0, faccessat(fd, "../..", F_OK, 0)) << strerror(errno); +}