git: 6efba04df3f8 - main - fusefs: fix two bugs regarding _PC_MIN_HOLE_SIZE
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 24 Jun 2024 16:07:22 UTC
The branch main has been updated by asomers:
URL: https://cgit.FreeBSD.org/src/commit/?id=6efba04df3f8c77b9b12f1df3e5124a7249b82fc
commit 6efba04df3f8c77b9b12f1df3e5124a7249b82fc
Author: Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2024-04-03 19:57:44 +0000
Commit: Alan Somers <asomers@FreeBSD.org>
CommitDate: 2024-06-24 16:02:02 +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
MFC after: 1 week
Sponsored by: Axcient
Differential Revision: https://reviews.freebsd.org/D44618
---
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 eb07861c04ba..bf272ab706da 100644
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -395,6 +395,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;
@@ -1754,6 +1757,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:
@@ -1783,22 +1789,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
*/