From owner-svn-src-projects@freebsd.org Tue Sep 3 14:06:11 2019 Return-Path: Delivered-To: svn-src-projects@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 15B2EDC42A for ; Tue, 3 Sep 2019 14:06:09 +0000 (UTC) (envelope-from yuripv@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2610:1c1:1:6074::16:84]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "freefall.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46N7z36Hp3z4PDY; Tue, 3 Sep 2019 14:06:07 +0000 (UTC) (envelope-from yuripv@freebsd.org) Received: by freefall.freebsd.org (Postfix, from userid 1452) id 181E01A1A4; Tue, 3 Sep 2019 14:05:59 +0000 (UTC) X-Original-To: yuripv@localmail.freebsd.org Delivered-To: yuripv@localmail.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client CN "mx1.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by freefall.freebsd.org (Postfix) with ESMTPS id C7D1655FA; Wed, 3 Apr 2019 20:57:48 +0000 (UTC) (envelope-from owner-src-committers@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [96.47.72.132]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "freefall.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4A1256D857; Wed, 3 Apr 2019 20:57:48 +0000 (UTC) (envelope-from owner-src-committers@freebsd.org) Received: by freefall.freebsd.org (Postfix, from userid 538) id 1A99455D1; Wed, 3 Apr 2019 20:57:48 +0000 (UTC) Delivered-To: src-committers@localmail.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client CN "mx1.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by freefall.freebsd.org (Postfix) with ESMTPS id 68FA055CF for ; Wed, 3 Apr 2019 20:57:45 +0000 (UTC) (envelope-from asomers@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) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 36D7A6D852; Wed, 3 Apr 2019 20:57:45 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 0DB121B6A4; Wed, 3 Apr 2019 20:57:45 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x33KviEY081274; Wed, 3 Apr 2019 20:57:44 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x33Kvinu081270; Wed, 3 Apr 2019 20:57:44 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201904032057.x33Kvinu081270@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r345854 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs X-SVN-Group: projects X-SVN-Commit-Author: asomers X-SVN-Commit-Paths: in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs X-SVN-Commit-Revision: 345854 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk X-Loop: FreeBSD.org Sender: owner-src-committers@freebsd.org X-Rspamd-Queue-Id: 4A1256D857 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.99 / 15.00]; local_wl_from(0.00)[freebsd.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_SHORT(-0.99)[-0.988,0]; ASN(0.00)[asn:11403, ipnet:96.47.64.0/20, country:US]; NEURAL_HAM_LONG(-1.00)[-1.000,0] Status: O X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.29 List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Date: Tue, 03 Sep 2019 14:06:11 -0000 X-Original-Date: Wed, 3 Apr 2019 20:57:44 +0000 (UTC) X-List-Received-Date: Tue, 03 Sep 2019 14:06:11 -0000 Author: asomers Date: Wed Apr 3 20:57:43 2019 New Revision: 345854 URL: https://svnweb.freebsd.org/changeset/base/345854 Log: fusefs: fix a panic in VOP_READDIR The original fusefs import, r238402, contained a bug in fuse_vnop_close that could close a directory's file handle while there were still other open file descriptors. The code looks deliberate, but there is no explanation for it. This necessitated a workaround in fuse_vnop_readdir that would open a new file handle if, "for some mysterious reason", that vnode didn't have any open file handles. r345781 had the effect of causing the workaround to panic, making the problem more visible. This commit removes the workaround and the original bug, which also fixes the panic. Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c projects/fuse2/tests/sys/fs/fusefs/readdir.cc projects/fuse2/tests/sys/fs/fusefs/releasedir.cc projects/fuse2/tests/sys/fs/fusefs/utils.cc projects/fuse2/tests/sys/fs/fusefs/utils.hh Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_vnops.c Wed Apr 3 20:37:14 2019 (r345853) +++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Wed Apr 3 20:57:43 2019 (r345854) @@ -324,21 +324,13 @@ fuse_vnop_close(struct vop_close_args *ap) pid_t pid = td->td_proc->p_pid; int err = 0; - if (fuse_isdeadfs(vp)) { + if (fuse_isdeadfs(vp)) return 0; - } - if (vnode_isdir(vp)) { - struct fuse_filehandle *fufh; - - // XXX: what if two file descriptors have the same directory - // opened? We shouldn't close the file handle too soon. - if (fuse_filehandle_get_dir(vp, &fufh, cred, pid) == 0) - fuse_filehandle_close(vp, fufh, NULL, cred); + if (vnode_isdir(vp)) return 0; - } - if (fflag & IO_NDELAY) { + if (fflag & IO_NDELAY) return 0; - } + err = fuse_flush(vp, cred, pid, fflag); /* TODO: close the file handle, if we're sure it's no longer used */ if ((VTOFUD(vp)->flag & FN_SIZECHANGE) != 0) { @@ -1342,7 +1334,6 @@ fuse_vnop_readdir(struct vop_readdir_args *ap) struct fuse_filehandle *fufh = NULL; struct fuse_iov cookediov; int err = 0; - int freefufh = 0; pid_t pid = curthread->td_proc->p_pid; if (fuse_isdeadfs(vp)) { @@ -1353,27 +1344,15 @@ fuse_vnop_readdir(struct vop_readdir_args *ap) return EINVAL; } - if ((err = fuse_filehandle_get_dir(vp, &fufh, cred, pid)) != 0) { - SDT_PROBE2(fuse, , vnops, trace, 1, - "calling readdir() before open()"); - /* - * This was seen to happen in getdirentries as used by - * shells/fish, but I can't reproduce it. - */ - err = fuse_filehandle_open(vp, FREAD, &fufh, NULL, cred); - freefufh = 1; - } - if (err) { + err = fuse_filehandle_get_dir(vp, &fufh, cred, pid); + if (err) return (err); - } #define DIRCOOKEDSIZE FUSE_DIRENT_ALIGN(FUSE_NAME_OFFSET + MAXNAMLEN + 1) fiov_init(&cookediov, DIRCOOKEDSIZE); err = fuse_internal_readdir(vp, uio, fufh, &cookediov); fiov_teardown(&cookediov); - if (freefufh) - fuse_filehandle_close(vp, fufh, NULL, cred); return err; } Modified: projects/fuse2/tests/sys/fs/fusefs/readdir.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/readdir.cc Wed Apr 3 20:37:14 2019 (r345853) +++ projects/fuse2/tests/sys/fs/fusefs/readdir.cc Wed Apr 3 20:57:43 2019 (r345854) @@ -213,13 +213,59 @@ TEST_F(Readdir, getdirentries) out->header.len = sizeof(out->header); }))); - errno = 0; fd = open(FULLPATH, O_DIRECTORY); ASSERT_LE(0, fd) << strerror(errno); r = getdirentries(fd, buf, sizeof(buf), 0); - ASSERT_EQ(0, r); + ASSERT_EQ(0, r) << strerror(errno); /* Deliberately leak fd. RELEASEDIR will be tested separately */ +} + +/* + * Nothing bad should happen if getdirentries is called on two file descriptors + * which were concurrently open, but one has already been closed. + * This is a regression test for a specific bug dating from r238402. + */ +TEST_F(Readdir, getdirentries_concurrent) +{ + const char FULLPATH[] = "mountpoint/some_dir"; + const char RELPATH[] = "some_dir"; + uint64_t ino = 42; + int fd0, fd1; + char buf[8192]; + ssize_t r; + + FuseTest::expect_lookup(RELPATH, ino, S_IFDIR | 0755, 0, 2); + expect_opendir(ino); + + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_READDIR && + in->header.nodeid == ino && + in->body.readdir.size == 8192); + }, Eq(true)), + _) + ).Times(2) + .WillRepeatedly(Invoke(ReturnImmediate([=](auto in __unused, auto out) { + out->header.error = 0; + out->header.len = sizeof(out->header); + }))); + + fd0 = open(FULLPATH, O_DIRECTORY); + ASSERT_LE(0, fd0) << strerror(errno); + + fd1 = open(FULLPATH, O_DIRECTORY); + ASSERT_LE(0, fd1) << strerror(errno); + + r = getdirentries(fd0, buf, sizeof(buf), 0); + ASSERT_EQ(0, r) << strerror(errno); + + EXPECT_EQ(0, close(fd0)) << strerror(errno); + + r = getdirentries(fd1, buf, sizeof(buf), 0); + ASSERT_EQ(0, r) << strerror(errno); + + /* Deliberately leak fd1. */ } /* Modified: projects/fuse2/tests/sys/fs/fusefs/releasedir.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/releasedir.cc Wed Apr 3 20:37:14 2019 (r345853) +++ projects/fuse2/tests/sys/fs/fusefs/releasedir.cc Wed Apr 3 20:57:43 2019 (r345854) @@ -45,18 +45,6 @@ void expect_lookup(const char *relpath, uint64_t ino) { FuseTest::expect_lookup(relpath, ino, S_IFDIR | 0755, 0, 1); } - -void expect_releasedir(uint64_t ino, ProcessMockerT r) -{ - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - return (in->header.opcode == FUSE_RELEASEDIR && - in->header.nodeid == ino && - in->body.release.fh == FH); - }, Eq(true)), - _) - ).WillOnce(Invoke(r)); -} }; /* If a file descriptor is duplicated, only the last close causes RELEASE */ Modified: projects/fuse2/tests/sys/fs/fusefs/utils.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/utils.cc Wed Apr 3 20:37:14 2019 (r345853) +++ projects/fuse2/tests/sys/fs/fusefs/utils.cc Wed Apr 3 20:57:43 2019 (r345854) @@ -232,6 +232,18 @@ void FuseTest::expect_release(uint64_t ino, uint64_t f ).WillOnce(Invoke(ReturnErrno(0))); } +void FuseTest::expect_releasedir(uint64_t ino, ProcessMockerT r) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_RELEASEDIR && + in->header.nodeid == ino && + in->body.release.fh == FH); + }, Eq(true)), + _) + ).WillOnce(Invoke(r)); +} + void FuseTest::expect_write(uint64_t ino, uint64_t offset, uint64_t isize, uint64_t osize, uint32_t flags, const void *contents) { Modified: projects/fuse2/tests/sys/fs/fusefs/utils.hh ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/utils.hh Wed Apr 3 20:37:14 2019 (r345853) +++ projects/fuse2/tests/sys/fs/fusefs/utils.hh Wed Apr 3 20:57:43 2019 (r345854) @@ -124,6 +124,12 @@ class FuseTest : public ::testing::Test { void expect_release(uint64_t ino, uint64_t fh); /* + * Create an expectation that FUSE_RELEASEDIR will be called exactly + * once for the given inode + */ + void expect_releasedir(uint64_t ino, ProcessMockerT r); + + /* * Create an expectation that FUSE_WRITE will be called exactly once * for the given inode, at offset offset, with write_flags flags, * size isize and buffer contents. It will return osize