Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 12 May 2022 20:40:47 GMT
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 04f7286f44c4 - stable/13 - fusefs: correctly handle servers that report too much data written
Message-ID:  <202205122040.24CKel5T042863@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=04f7286f44c484b4288b3071139d451ae4a623d7

commit 04f7286f44c484b4288b3071139d451ae4a623d7
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-04-18 23:03:53 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-05-12 20:39:47 +0000

    fusefs: correctly handle servers that report too much data written
    
    During a FUSE_WRITE, the kernel requests the server to write a certain
    amount of data, and the server responds with the amount that it actually
    did write.  It is obviously an error for the server to write more than
    it was provided, and we always treated it as such, but there were two
    problems:
    
    * If the server responded with a huge amount, greater than INT_MAX, it
      would trigger an integer overflow which would cause a panic.
    
    * When extending the file, we wrongly set the file's size before
      validing the amount written.
    
    PR:             263263
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Sponsored by:   Axcient
    Reviewed by:    emaste
    Differential Revision: https://reviews.freebsd.org/D34955
    
    (cherry picked from commit 3a1b3c6a1e68063330e897a5a5c94518edae4a3b)
---
 sys/fs/fuse/fuse_io.c        | 18 ++++++++-----
 tests/sys/fs/fusefs/write.cc | 61 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c
index 8d2e79d0a073..c70eca17ec29 100644
--- a/sys/fs/fuse/fuse_io.c
+++ b/sys/fs/fuse/fuse_io.c
@@ -395,8 +395,19 @@ retry:
 
 		fwo = ((struct fuse_write_out *)fdi.answ);
 
+		if (fwo->size > fwi->size) {
+			fuse_warn(data, FSESS_WARN_WROTE_LONG,
+				"wrote more data than we provided it.");
+			/* This is bonkers.  Clear attr cache. */
+			fvdat->flag &= ~FN_SIZECHANGE;
+			fuse_vnode_clear_attr_cache(vp);
+			err = EINVAL;
+			break;
+		}
+
 		/* Adjust the uio in the case of short writes */
 		diff = fwi->size - fwo->size;
+
 		as_written_offset = uio->uio_offset - diff;
 
 		if (as_written_offset - diff > filesize) {
@@ -406,12 +417,7 @@ retry:
 		if (as_written_offset - diff >= filesize)
 			fvdat->flag &= ~FN_SIZECHANGE;
 
-		if (diff < 0) {
-			fuse_warn(data, FSESS_WARN_WROTE_LONG,
-				"wrote more data than we provided it.");
-			err = EINVAL;
-			break;
-		} else if (diff > 0) {
+		if (diff > 0) {
 			/* Short write */
 			if (!direct_io) {
 				fuse_warn(data, FSESS_WARN_SHORT_WRITE,
diff --git a/tests/sys/fs/fusefs/write.cc b/tests/sys/fs/fusefs/write.cc
index db5bb3fe4441..d685bd13aa17 100644
--- a/tests/sys/fs/fusefs/write.cc
+++ b/tests/sys/fs/fusefs/write.cc
@@ -410,6 +410,67 @@ TEST_F(Write, indirect_io_short_write)
 	leak(fd);
 }
 
+/* It is an error if the daemon claims to have written more data than we sent */
+TEST_F(Write, indirect_io_long_write)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const char *CONTENTS = "abcdefghijklmnop";
+	uint64_t ino = 42;
+	int fd;
+	ssize_t bufsize = strlen(CONTENTS);
+	ssize_t bufsize_out = 100;
+	off_t some_other_size = 25;
+	struct stat sb;
+
+	expect_lookup(RELPATH, ino, 0);
+	expect_open(ino, 0, 1);
+	expect_write(ino, 0, bufsize, bufsize_out, CONTENTS);
+	expect_getattr(ino, some_other_size);
+
+	fd = open(FULLPATH, O_WRONLY);
+	ASSERT_LE(0, fd) << strerror(errno);
+
+	ASSERT_EQ(-1, write(fd, CONTENTS, bufsize)) << strerror(errno);
+	ASSERT_EQ(EINVAL, errno);
+
+	/*
+	 * Following such an error, we should requery the server for the file's
+	 * size.
+	 */
+	fstat(fd, &sb);
+	ASSERT_EQ(sb.st_size, some_other_size);
+
+	leak(fd);
+}
+
+/*
+ * Don't crash if the server returns a write that can't be represented as a
+ * signed 32 bit number.  Regression test for
+ * https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263263
+ */
+TEST_F(Write, indirect_io_very_long_write)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const char *CONTENTS = "abcdefghijklmnop";
+	uint64_t ino = 42;
+	int fd;
+	ssize_t bufsize = strlen(CONTENTS);
+	ssize_t bufsize_out = 3 << 30;
+
+	expect_lookup(RELPATH, ino, 0);
+	expect_open(ino, 0, 1);
+	expect_write(ino, 0, bufsize, bufsize_out, CONTENTS);
+
+	fd = open(FULLPATH, O_WRONLY);
+	ASSERT_LE(0, fd) << strerror(errno);
+
+	ASSERT_EQ(-1, write(fd, CONTENTS, bufsize)) << strerror(errno);
+	ASSERT_EQ(EINVAL, errno);
+	leak(fd);
+}
+
 /* 
  * When the direct_io option is used, filesystems are allowed to write less
  * data than requested.  We should return the short write to userland.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202205122040.24CKel5T042863>