aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlan Somers <asomers@FreeBSD.org>2024-04-03 19:57:44 +0000
committerAlan Somers <asomers@FreeBSD.org>2024-07-08 20:27:54 +0000
commit7f936b0cba08315c0127c425692369d0c162fbcf (patch)
treeccbeff4a6cef3226ae6cfdad736e644825d41b33
parentc1a5da044c6e692250bc5eddcb99c6e9f1ab4b78 (diff)
downloadsrc-7f936b0cba08315c0127c425692369d0c162fbcf.tar.gz
src-7f936b0cba08315c0127c425692369d0c162fbcf.zip
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)
-rw-r--r--sys/fs/fuse/fuse_vnops.c48
-rw-r--r--tests/sys/fs/fusefs/lseek.cc129
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
@@ -113,6 +113,75 @@ TEST_F(LseekPathconf, already_seeked)
}
/*
+ * 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.
*/
@@ -142,6 +211,34 @@ TEST_F(LseekPathconf, enosys_now)
}
/*
+ * 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
* will only try SEEK_HOLE/SEEK_DATA if fpathconf says they're supported.
@@ -178,6 +275,38 @@ TEST_F(LseekPathconf, seek_now)
}
/*
+ * 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
*/
TEST_F(LseekPathconf_7_23, already_enosys)