fusefs: correctly set fuse_release_in.flags in an error path

fuse_vnop_create must close the newly created file if it can't allocate a
vnode.  When it does so, it must use the same file flags for FUSE_RELEASE as
it used for FUSE_OPEN or FUSE_CREATE.

Reported by:	Coverity
Coverity CID:	1066204
Sponsored by:	The FreeBSD Foundation

sys/fs/fuse/fuse_vnops.c

@@ -436,7 +436,7 @@ fuse_vnop_create(struct vop_create_args *ap)
 		fdisp_make(fdip, FUSE_RELEASE, mp, nodeid, td, cred);
 		fri = fdip->indata;
 		fri->fh = fh_id;
-		fri->flags = OFLAGS(mode);
+		fri->flags = fuse_filehandle_xlate_to_oflags(FUFH_RDWR);
 		fuse_insert_callback(fdip->tick, fuse_internal_forget_callback);
 		fuse_insert_message(fdip->tick);
 		goto out;

tests/sys/fs/fusefs/allow_other.cc

@@ -136,7 +136,7 @@ TEST_F(AllowOther, allowed)
 		 */
 		expect_lookup(RELPATH, ino, S_IFREG | 0644, 0, 1);
 		expect_open(ino, 0, 1);
-		expect_release(ino, 1, 0, 0);
+		expect_release(ino);
 		/* Until the attr cache is working, we may send an additional
 		 * GETATTR */
 		expect_getattr(ino, 0);

tests/sys/fs/fusefs/fsync.cc

@@ -139,7 +139,7 @@ TEST_F(Fsync, close)
 		}, Eq(true)),
 		_)
 	).Times(0);
-	expect_release(ino, 1, 0, 0);
+	expect_release(ino);
 
 	fd = open(FULLPATH, O_RDWR);
 	ASSERT_LE(0, fd) << strerror(errno);

tests/sys/fs/fusefs/readdir.cc

@@ -218,7 +218,7 @@ TEST_F(Readdir, getdirentries)
 	r = getdirentries(fd, buf, sizeof(buf), 0);
 	ASSERT_EQ(0, r);
 
-	/* Deliberately leak dir.  RELEASEDIR will be tested separately */
+	/* Deliberately leak fd.  RELEASEDIR will be tested separately */
 }
 
 /*

tests/sys/fs/fusefs/release.cc

@@ -45,6 +45,22 @@ void expect_lookup(const char *relpath, uint64_t ino, int times)
 {
 	FuseTest::expect_lookup(relpath, ino, S_IFREG | 0644, 0, times);
 }
+
+void expect_release(uint64_t ino, uint64_t lock_owner,
+	uint32_t flags, int error)
+{
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in->header.opcode == FUSE_RELEASE &&
+				in->header.nodeid == ino &&
+				in->body.release.lock_owner == lock_owner &&
+				in->body.release.fh == FH &&
+				in->body.release.flags == flags);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnErrno(error)))
+	.RetiresOnSaturation();
+}
 };
 
 class ReleaseWithLocks: public Release {
@@ -66,7 +82,7 @@ TEST_F(Release, dup)
 	expect_lookup(RELPATH, ino, 1);
 	expect_open(ino, 0, 1);
 	expect_getattr(ino, 0);
-	expect_release(ino, 1, 0, 0);
+	expect_release(ino, 0, O_RDONLY, 0);
 	
 	fd = open(FULLPATH, O_RDONLY);
 	EXPECT_LE(0, fd) << strerror(errno);
@@ -95,7 +111,7 @@ TEST_F(Release, eio)
 	expect_lookup(RELPATH, ino, 1);
 	expect_open(ino, 0, 1);
 	expect_getattr(ino, 0);
-	expect_release(ino, 1, 0, EIO);
+	expect_release(ino, 0, O_WRONLY, EIO);
 	
 	fd = open(FULLPATH, O_WRONLY);
 	EXPECT_LE(0, fd) << strerror(errno);
@@ -103,6 +119,28 @@ TEST_F(Release, eio)
 	ASSERT_TRUE(0 == close(fd) || errno == EIO) << strerror(errno);
 }
 
+/*
+ * FUSE_RELEASE should contain the same flags used for FUSE_OPEN
+ */
+/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236340 */
+TEST_F(Release, DISABLED_flags)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	int fd;
+
+	expect_lookup(RELPATH, ino, 1);
+	expect_open(ino, 0, 1);
+	expect_getattr(ino, 0);
+	expect_release(ino, 0, O_RDWR | O_APPEND, 0);
+	
+	fd = open(FULLPATH, O_RDWR | O_APPEND);
+	EXPECT_LE(0, fd) << strerror(errno);
+
+	ASSERT_EQ(0, close(fd)) << strerror(errno);
+}
+
 /*
  * fuse(4) will issue multiple FUSE_OPEN operations for the same file if it's
  * opened with different modes.  Each FUSE_OPEN should get its own
@@ -118,11 +156,12 @@ TEST_F(Release, multiple_opens)
 	expect_lookup(RELPATH, ino, 2);
 	expect_open(ino, 0, 2);
 	expect_getattr(ino, 0);
-	expect_release(ino, 2, 0, 0);
+	expect_release(ino, 0, O_RDONLY, 0);
 	
 	fd = open(FULLPATH, O_RDONLY);
 	EXPECT_LE(0, fd) << strerror(errno);
 
+	expect_release(ino, 0, O_WRONLY, 0);
 	fd2 = open(FULLPATH, O_WRONLY);
 	EXPECT_LE(0, fd2) << strerror(errno);
 
@@ -140,7 +179,7 @@ TEST_F(Release, ok)
 	expect_lookup(RELPATH, ino, 1);
 	expect_open(ino, 0, 1);
 	expect_getattr(ino, 0);
-	expect_release(ino, 1, 0, 0);
+	expect_release(ino, 0, O_RDONLY, 0);
 	
 	fd = open(FULLPATH, O_RDONLY);
 	EXPECT_LE(0, fd) << strerror(errno);
@@ -173,7 +212,7 @@ TEST_F(ReleaseWithLocks, DISABLED_unlock_on_close)
 		SET_OUT_HEADER_LEN(out, setlk);
 		out->body.setlk.lk = in->body.setlk.lk;
 	})));
-	expect_release(ino, 1, (uint64_t)pid, 0);
+	expect_release(ino, (uint64_t)pid, O_RDWR, 0);
 
 	fd = open(FULLPATH, O_RDWR);
 	ASSERT_LE(0, fd) << strerror(errno);

tests/sys/fs/fusefs/utils.cc

@@ -199,20 +199,18 @@ void FuseTest::expect_read(uint64_t ino, uint64_t offset, uint64_t isize,
 	}))).RetiresOnSaturation();
 }
 
-void FuseTest::expect_release(uint64_t ino, int times, uint64_t lock_owner,
-	int error)
+void FuseTest::expect_release(uint64_t ino)
 {
 	EXPECT_CALL(*m_mock, process(
 		ResultOf([=](auto in) {
 			return (in->header.opcode == FUSE_RELEASE &&
 				in->header.nodeid == ino &&
-				in->body.release.lock_owner == lock_owner &&
 				in->body.release.fh == FH);
 		}, Eq(true)),
 		_)
-	).Times(times)
-	.WillRepeatedly(Invoke(ReturnErrno(error)));
+	).WillOnce(Invoke(ReturnErrno(0)));
 }
+
 void FuseTest::expect_write(uint64_t ino, uint64_t offset, uint64_t isize,
 	uint64_t osize, uint32_t flags, const void *contents)
 {

tests/sys/fs/fusefs/utils.hh

@@ -112,11 +112,10 @@ class FuseTest : public ::testing::Test {
 		uint64_t osize, const void *contents);
 
 	/* 
-	 * Create an expectation that FUSE_RELEASE will be called times times
-	 * for the given inode, returning error error
+	 * Create an expectation that FUSE_RELEASE will be called exactly once
+	 * for the given inode, returning success
 	 */
-	void expect_release(uint64_t ino, int times, uint64_t lock_owner,
-		int error);
+	void expect_release(uint64_t ino);
 
 	/*
 	 * Create an expectation that FUSE_WRITE will be called exactly once