git: 7f936b0cba08 - stable/13 - fusefs: fix two bugs regarding _PC_MIN_HOLE_SIZE

From: Alan Somers <asomers_at_FreeBSD.org>
Date: Tue, 09 Jul 2024 00:03:06 UTC
The branch stable/13 has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=7f936b0cba08315c0127c425692369d0c162fbcf

commit 7f936b0cba08315c0127c425692369d0c162fbcf
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2024-04-03 19:57:44 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2024-07-08 20:27:54 +0000

    fusefs: fix two bugs regarding _PC_MIN_HOLE_SIZE
    
    Background:
    
    If a user does pathconf(_, _PC_MIN_HOLE_SIZE) on a fusefs file system,
    the kernel must actually issue a FUSE_LSEEK operation in order to
    determine whether the server supports it.  We cache that result, so we
    only have to send FUSE_LSEEK the first time that _PC_MIN_HOLE_SIZE is
    requested on any given mountpoint.
    
    Problem 1:
    
    Unlike fpathconf, pathconf operates on files that may not be open.  But
    FUSE_LSEEK requires the file to be open.  As described in PR 278135,
    FUSE_LSEEK cannot be sent for unopened files, causing _PC_MIN_HOLE_size
    to wrongly report EINVAL.  We never noticed that before because the
    fusefs test suite only uses fpathconf, not pathconf.  Fix this bug by
    opening the file if necessary.
    
    Problem 2:
    
    On a completely sparse file, with no data blocks at all, FUSE_LSEEK with
    SEEK_DATA would fail to ENXIO.  That's correct behavior, but
    fuse_vnop_pathconf wrongly interpreted that as "FUSE_LSEEK not
    supported".  Fix the interpretation.
    
    PR:             278135
    Sponsored by:   Axcient
    Differential Revision: https://reviews.freebsd.org/D44618
    
    (cherry picked from commit 6efba04df3f8c77b9b12f1df3e5124a7249b82fc)
---
 sys/fs/fuse/fuse_vnops.c     |  48 ++++++++++++----
 tests/sys/fs/fusefs/lseek.cc | 129 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 167 insertions(+), 10 deletions(-)

diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c
index 75b83c648d38..aa44366ccb95 100644
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -394,6 +394,9 @@ fuse_vnop_do_lseek(struct vnode *vp, struct thread *td, struct ucred *cred,
 	err = fdisp_wait_answ(&fdi);
 	if (err == ENOSYS) {
 		fsess_set_notimpl(mp, FUSE_LSEEK);
+	} else if (err == ENXIO) {
+		/* Note: ENXIO means "no more hole/data regions until EOF" */
+		fsess_set_impl(mp, FUSE_LSEEK);
 	} else if (err == 0) {
 		fsess_set_impl(mp, FUSE_LSEEK);
 		flso = fdi.answ;
@@ -1770,6 +1773,9 @@ fuse_vnop_pathconf(struct vop_pathconf_args *ap)
 {
 	struct vnode *vp = ap->a_vp;
 	struct mount *mp;
+	struct fuse_filehandle *fufh;
+	int err;
+	bool closefufh = false;
 
 	switch (ap->a_name) {
 	case _PC_FILESIZEBITS:
@@ -1799,22 +1805,44 @@ fuse_vnop_pathconf(struct vop_pathconf_args *ap)
 		    !fsess_not_impl(mp, FUSE_LSEEK)) {
 			off_t offset = 0;
 
-			/* Issue a FUSE_LSEEK to find out if it's implemented */
-			fuse_vnop_do_lseek(vp, curthread, curthread->td_ucred,
-			    curthread->td_proc->p_pid, &offset, SEEK_DATA);
+			/*
+			 * Issue a FUSE_LSEEK to find out if it's supported.
+			 * Use SEEK_DATA instead of SEEK_HOLE, because the
+			 * latter generally requires sequential scans of file
+			 * metadata, which can be slow.
+			 */
+			err = fuse_vnop_do_lseek(vp, curthread,
+			    curthread->td_ucred, curthread->td_proc->p_pid,
+			    &offset, SEEK_DATA);
+			if (err == EBADF) {
+				/*
+				 * pathconf() doesn't necessarily open the
+				 * file.  So we may need to do it here.
+				 */
+				err = fuse_filehandle_open(vp, FREAD, &fufh,
+				    curthread, curthread->td_ucred);
+				if (err == 0) {
+					closefufh = true;
+					err = fuse_vnop_do_lseek(vp, curthread,
+					    curthread->td_ucred,
+					    curthread->td_proc->p_pid, &offset,
+					    SEEK_DATA);
+				}
+				if (closefufh)
+					fuse_filehandle_close(vp, fufh,
+					    curthread, curthread->td_ucred);
+			}
+
 		}
 
 		if (fsess_is_impl(mp, FUSE_LSEEK)) {
 			*ap->a_retval = 1;
 			return (0);
-		} else {
-			/*
-			 * Probably FUSE_LSEEK is not implemented.  It might
-			 * be, if the FUSE_LSEEK above returned an error like
-			 * EACCES, but in that case we can't tell, so it's
-			 * safest to report EINVAL anyway.
-			 */
+		} else if (fsess_not_impl(mp, FUSE_LSEEK)) {
+			/* FUSE_LSEEK is not implemented */
 			return (EINVAL);
+		} else {
+			return (err);
 		}
 	default:
 		return (vop_stdpathconf(ap));
diff --git a/tests/sys/fs/fusefs/lseek.cc b/tests/sys/fs/fusefs/lseek.cc
index 5ffeb4b33cbd..2a1cb198bcce 100644
--- a/tests/sys/fs/fusefs/lseek.cc
+++ b/tests/sys/fs/fusefs/lseek.cc
@@ -112,6 +112,75 @@ TEST_F(LseekPathconf, already_seeked)
 	leak(fd);
 }
 
+/*
+ * Use pathconf on a file not already opened.  The server returns EACCES when
+ * the kernel tries to open it.  The kernel should return EACCES, and make no
+ * judgement about whether the server does or does not support FUSE_LSEEK.
+ */
+TEST_F(LseekPathconf, eacces)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const uint64_t ino = 42;
+	off_t fsize = 1 << 30;	/* 1 GiB */
+
+	EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
+	.WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
+		SET_OUT_HEADER_LEN(out, entry);
+		out.body.entry.entry_valid = UINT64_MAX;
+		out.body.entry.attr.mode = S_IFREG | 0644;
+		out.body.entry.nodeid = ino;
+		out.body.entry.attr.size = fsize;
+	})));
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_OPEN &&
+				in.header.nodeid == ino);
+		}, Eq(true)),
+		_)
+	).Times(2)
+	.WillRepeatedly(Invoke(ReturnErrno(EACCES)));
+
+	EXPECT_EQ(-1, pathconf(FULLPATH, _PC_MIN_HOLE_SIZE));
+	EXPECT_EQ(EACCES, errno);
+	/* Check again, to ensure that the kernel didn't record the response */
+	EXPECT_EQ(-1, pathconf(FULLPATH, _PC_MIN_HOLE_SIZE));
+	EXPECT_EQ(EACCES, errno);
+}
+
+/*
+ * If the server returns some weird error when we try FUSE_LSEEK, send that to
+ * the caller but don't record the answer.
+ */
+TEST_F(LseekPathconf, eio)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const uint64_t ino = 42;
+	off_t fsize = 1 << 30;	/* 1 GiB */
+	int fd;
+
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, fsize, 1);
+	expect_open(ino, 0, 1);
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_LSEEK);
+		}, Eq(true)),
+		_)
+	).Times(2)
+	.WillRepeatedly(Invoke(ReturnErrno(EIO)));
+
+	fd = open(FULLPATH, O_RDONLY);
+
+	EXPECT_EQ(-1, fpathconf(fd, _PC_MIN_HOLE_SIZE));
+	EXPECT_EQ(EIO, errno);
+	/* Check again, to ensure that the kernel didn't record the response */
+	EXPECT_EQ(-1, fpathconf(fd, _PC_MIN_HOLE_SIZE));
+	EXPECT_EQ(EIO, errno);
+
+	leak(fd);
+}
+
 /*
  * If no FUSE_LSEEK operation has been attempted since mount, try once as soon
  * as a pathconf request comes in.
@@ -141,6 +210,34 @@ TEST_F(LseekPathconf, enosys_now)
 	leak(fd);
 }
 
+/*
+ * Use pathconf, rather than fpathconf, on a file not already opened.
+ * Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278135
+ */
+TEST_F(LseekPathconf, pathconf)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const uint64_t ino = 42;
+	off_t fsize = 1 << 30;	/* 1 GiB */
+	off_t offset_out = 1 << 29;
+
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, fsize, 1);
+	expect_open(ino, 0, 1);
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_LSEEK);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnImmediate([=](auto i __unused, auto& out) {
+		SET_OUT_HEADER_LEN(out, lseek);
+		out.body.lseek.offset = offset_out;
+	})));
+	expect_release(ino, FuseTest::FH);
+
+	EXPECT_EQ(1, pathconf(FULLPATH, _PC_MIN_HOLE_SIZE)) << strerror(errno);
+}
+
 /*
  * If no FUSE_LSEEK operation has been attempted since mount, try one as soon
  * as a pathconf request comes in.  This is the typical pattern of bsdtar.  It
@@ -177,6 +274,38 @@ TEST_F(LseekPathconf, seek_now)
 	leak(fd);
 }
 
+/*
+ * If the user calls pathconf(_, _PC_MIN_HOLE_SIZE) on a fully sparse or
+ * zero-length file, then SEEK_DATA will return ENXIO.  That should be
+ * interpreted as success.
+ */
+TEST_F(LseekPathconf, zerolength)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const uint64_t ino = 42;
+	off_t fsize = 0;
+	int fd;
+
+	expect_lookup(RELPATH, ino, S_IFREG | 0644, fsize, 1);
+	expect_open(ino, 0, 1);
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_LSEEK &&
+				in.header.nodeid == ino &&
+				in.body.lseek.whence == SEEK_DATA);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnErrno(ENXIO)));
+
+	fd = open(FULLPATH, O_RDONLY);
+	EXPECT_EQ(1, fpathconf(fd, _PC_MIN_HOLE_SIZE));
+	/* Check again, to ensure that the kernel recorded the response */
+	EXPECT_EQ(1, fpathconf(fd, _PC_MIN_HOLE_SIZE));
+
+	leak(fd);
+}
+
 /*
  * For servers using older protocol versions, no FUSE_LSEEK should be attempted
  */