svn commit: r348132 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs

[Alan Somers]

Author: asomers
Date: Wed May 22 23:30:51 2019
New Revision: 348132
URL: https://svnweb.freebsd.org/changeset/base/348132

Log:
  fusefs: fix "recursing on non recursive lockmgr" panic
  
  When mounted with -o default_permissions and when
  vfs.fusefs.data_cache_mode=2, fuse_io_strategy would try to clear the suid
  bit after a successful write by a non-owner.  When combined with a
  not-yet-committed attribute-caching patch I'm working on, and if the
  FUSE_SETATTR response indicates an unexpected filesize (legal, if the file
  system has other clients), this would end up calling vtruncbuf.  That would
  panic, because the buffer lock was already held by bufwrite or bufstrategy
  or something else upstack from fuse_vnop_strategy.
  
  Sponsored by:	The FreeBSD Foundation

Modified:
  projects/fuse2/sys/fs/fuse/fuse_io.c
  projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc

Modified: projects/fuse2/sys/fs/fuse/fuse_io.c
==============================================================================
--- projects/fuse2/sys/fs/fuse/fuse_io.c	Wed May 22 23:23:16 2019	(r348131)
+++ projects/fuse2/sys/fs/fuse/fuse_io.c	Wed May 22 23:30:51 2019	(r348132)
@@ -883,9 +883,6 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp)
 					bp->b_ioflags |= BIO_ERROR;
 					bp->b_flags |= B_INVAL;
 					bp->b_error = error;
-				} else {
-					fuse_io_clear_suid_on_write(vp, cred,
-						uio.uio_td);
 				}
 				bp->b_dirtyoff = bp->b_dirtyend = 0;
 			}

Modified: projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc
==============================================================================
--- projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc	Wed May 22 23:23:16 2019	(r348131)
+++ projects/fuse2/tests/sys/fs/fusefs/default_permissions.cc	Wed May 22 23:30:51 2019	(r348132)
@@ -68,7 +68,7 @@ virtual void SetUp() {
 }
 
 public:
-void expect_chmod(uint64_t ino, mode_t mode)
+void expect_chmod(uint64_t ino, mode_t mode, uint64_t size = 0)
 {
 	EXPECT_CALL(*m_mock, process(
 		ResultOf([=](auto in) {
@@ -82,6 +82,7 @@ void expect_chmod(uint64_t ino, mode_t mode)
 		SET_OUT_HEADER_LEN(out, attr);
 		out->body.attr.attr.ino = ino;	// Must match nodeid
 		out->body.attr.attr.mode = S_IFREG | mode;
+		out->body.attr.attr.size = size;
 		out->body.attr.attr_valid = UINT64_MAX;
 	})));
 }
@@ -1229,7 +1230,7 @@ TEST_F(Write, clear_suid)
 	expect_lookup(RELPATH, ino, S_IFREG | oldmode, UINT64_MAX);
 	expect_open(ino, 0, 1);
 	expect_write(ino, 0, sizeof(wbuf), sizeof(wbuf), 0, wbuf);
-	expect_chmod(ino, newmode);
+	expect_chmod(ino, newmode, sizeof(wbuf));
 
 	fd = open(FULLPATH, O_WRONLY);
 	ASSERT_LE(0, fd) << strerror(errno);
@@ -1255,7 +1256,7 @@ TEST_F(Write, clear_sgid)
 	expect_lookup(RELPATH, ino, S_IFREG | oldmode, UINT64_MAX);
 	expect_open(ino, 0, 1);
 	expect_write(ino, 0, sizeof(wbuf), sizeof(wbuf), 0, wbuf);
-	expect_chmod(ino, newmode);
+	expect_chmod(ino, newmode, sizeof(wbuf));
 
 	fd = open(FULLPATH, O_WRONLY);
 	ASSERT_LE(0, fd) << strerror(errno);
@@ -1264,3 +1265,34 @@ TEST_F(Write, clear_sgid)
 	EXPECT_EQ(S_IFREG | newmode, sb.st_mode);
 	/* Deliberately leak fd.  close(2) will be tested in release.cc */
 }
+
+/* Regression test for a specific recurse-of-nonrecursive-lock panic
+ *
+ * With writeback caching, we can't call vtruncbuf from fuse_io_strategy, or it
+ * may panic.  That happens if the FUSE_SETATTR response indicates that the
+ * file's size has changed since the write.
+ */
+TEST_F(Write, recursion_panic_while_clearing_suid)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	mode_t oldmode = 04777;
+	mode_t newmode = 0777;
+	char wbuf[1] = {'x'};
+	int fd;
+
+	expect_getattr(1, S_IFDIR | 0755, UINT64_MAX, 1);
+	expect_lookup(RELPATH, ino, S_IFREG | oldmode, UINT64_MAX);
+	expect_open(ino, 0, 1);
+	expect_write(ino, 0, sizeof(wbuf), sizeof(wbuf), 0, wbuf);
+	/* XXX Return a smaller file size than what we just wrote! */
+	expect_chmod(ino, newmode, 0);
+
+	fd = open(FULLPATH, O_WRONLY);
+	ASSERT_LE(0, fd) << strerror(errno);
+	ASSERT_EQ(1, write(fd, wbuf, sizeof(wbuf))) << strerror(errno);
+	/* Deliberately leak fd.  close(2) will be tested in release.cc */
+}
+
+