Page MenuHomeFreeBSD
Files F115907791

D41925.id127597.diff
No OneTemporary
Actions

D41925.id127597.diff
View Options

diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -779,6 +779,7 @@
fuse_vnop_close(struct vop_close_args *ap)
{
struct vnode *vp = ap->a_vp;
+ struct mount *mp = vnode_mount(vp);
struct ucred *cred = ap->a_cred;
int fflag = ap->a_fflag;
struct thread *td = ap->a_td;
@@ -794,12 +795,30 @@
return 0;
err = fuse_flush(vp, cred, pid, fflag);
- if (err == 0 && (fvdat->flag & FN_ATIMECHANGE)) {
+ if (err == 0 && (fvdat->flag & FN_ATIMECHANGE) && !vfs_isrdonly(mp)) {
struct vattr vap;
+ struct fuse_data *data;
+ int dataflags;
+ int access_e = 0;
- VATTR_NULL(&vap);
- vap.va_atime = fvdat->cached_attrs.va_atime;
- err = fuse_internal_setattr(vp, &vap, td, NULL);
+ data = fuse_get_mpdata(mp);
+ dataflags = data->dataflags;
+ if (dataflags & FSESS_DEFAULT_PERMISSIONS) {
+ struct vattr va;
+
+ fuse_internal_getattr(vp, &va, cred, td);
+ access_e = vaccess(vp->v_type, va.va_mode, va.va_uid,
+ va.va_gid, VWRITE, cred);
+ }
+ if (access_e == 0) {
+ VATTR_NULL(&vap);
+ vap.va_atime = fvdat->cached_attrs.va_atime;
+ /*
+ * Ignore errors setting when setting atime. That
+ * should not cause close(2) to fail.
+ */
+ fuse_internal_setattr(vp, &vap, td, NULL);
+ }
}
/* TODO: close the file handle, if we're sure it's no longer used */
if ((fvdat->flag & FN_SIZECHANGE) != 0) {
diff --git a/tests/sys/fs/fusefs/access.cc b/tests/sys/fs/fusefs/access.cc
--- a/tests/sys/fs/fusefs/access.cc
+++ b/tests/sys/fs/fusefs/access.cc
@@ -55,7 +55,7 @@
}
/*
- * Expect tha FUSE_ACCESS will never be called for the given inode, with any
+ * Expect that FUSE_ACCESS will never be called for the given inode, with any
* bits in the supplied access_mask set
*/
void expect_noaccess(uint64_t ino, mode_t access_mask)
diff --git a/tests/sys/fs/fusefs/default_permissions.cc b/tests/sys/fs/fusefs/default_permissions.cc
--- a/tests/sys/fs/fusefs/default_permissions.cc
+++ b/tests/sys/fs/fusefs/default_permissions.cc
@@ -30,7 +30,7 @@
/*
* Tests for the "default_permissions" mount option. They must be in their own
- * file so they can be run as an unprivileged user
+ * file so they can be run as an unprivileged user.
*/
extern "C" {
@@ -163,6 +163,7 @@
class Lookup: public DefaultPermissions {};
class Open: public DefaultPermissions {};
class PosixFallocate: public DefaultPermissions {};
+class Read: public DefaultPermissions {};
class Setattr: public DefaultPermissions {};
class Unlink: public DefaultPermissions {};
class Utimensat: public DefaultPermissions {};
@@ -1239,6 +1240,45 @@
ASSERT_EQ(0, rename(FULLSRC, FULLDST)) << strerror(errno);
}
+// Don't update atime during close after read, if we lack permissions to write
+// that file.
+TEST_F(Read, atime_during_close)
+{
+ const char FULLPATH[] = "mountpoint/some_file.txt";
+ const char RELPATH[] = "some_file.txt";
+ struct stat sb;
+ uint64_t ino = 42;
+ int fd;
+ ssize_t bufsize = 100;
+ uint8_t buf[bufsize];
+ const char *CONTENTS = "abcdefgh";
+ ssize_t fsize = sizeof(CONTENTS);
+
+ expect_getattr(FUSE_ROOT_ID, S_IFDIR | 0755, UINT64_MAX, 1);
+ FuseTest::expect_lookup(RELPATH, ino, S_IFREG | 0755, fsize,
+ 1, UINT64_MAX, 0, 0);
+ expect_open(ino, 0, 1);
+ expect_read(ino, 0, fsize, fsize, CONTENTS);
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([&](auto in) {
+ return (in.header.opcode == FUSE_SETATTR);
+ }, Eq(true)),
+ _)
+ ).Times(0);
+ expect_flush(ino, 1, ReturnErrno(0));
+ expect_release(ino, FuseTest::FH);
+
+ fd = open(FULLPATH, O_RDONLY);
+ ASSERT_LE(0, fd) << strerror(errno);
+
+ /* Ensure atime will be different than during lookup */
+ nap();
+
+ ASSERT_EQ(fsize, read(fd, buf, bufsize)) << strerror(errno);
+
+ close(fd);
+}
+
TEST_F(Setattr, ok)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
diff --git a/tests/sys/fs/fusefs/read.cc b/tests/sys/fs/fusefs/read.cc
--- a/tests/sys/fs/fusefs/read.cc
+++ b/tests/sys/fs/fusefs/read.cc
@@ -57,6 +57,14 @@
}
};
+class RofsRead: public Read {
+public:
+virtual void SetUp() {
+ m_ro = true;
+ Read::SetUp();
+}
+};
+
class Read_7_8: public FuseTest {
public:
virtual void SetUp() {
@@ -454,6 +462,48 @@
close(fd);
}
+/*
+ * When not using -o default_permissions, the daemon may make its own decisions
+ * regarding access permissions, and these may be unpredictable. If it rejects
+ * our attempt to set atime, that should not cause close(2) to fail.
+ */
+TEST_F(Read, atime_during_close_eacces)
+{
+ const char FULLPATH[] = "mountpoint/some_file.txt";
+ const char RELPATH[] = "some_file.txt";
+ const char *CONTENTS = "abcdefgh";
+ struct stat sb;
+ uint64_t ino = 42;
+ int fd;
+ ssize_t bufsize = strlen(CONTENTS);
+ uint8_t buf[bufsize];
+
+ expect_lookup(RELPATH, ino, bufsize);
+ expect_open(ino, 0, 1);
+ expect_read(ino, 0, bufsize, bufsize, CONTENTS);
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([&](auto in) {
+ uint32_t valid = FATTR_ATIME;
+ return (in.header.opcode == FUSE_SETATTR &&
+ in.header.nodeid == ino &&
+ in.body.setattr.valid == valid);
+ }, Eq(true)),
+ _)
+ ).WillOnce(Invoke(ReturnErrno(EACCES)));
+ expect_flush(ino, 1, ReturnErrno(0));
+ expect_release(ino, FuseTest::FH);
+
+ fd = open(FULLPATH, O_RDONLY);
+ ASSERT_LE(0, fd) << strerror(errno);
+
+ /* Ensure atime will be different than during lookup */
+ nap();
+
+ ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno);
+
+ ASSERT_EQ(0, close(fd));
+}
+
/* A cached atime should be flushed during FUSE_SETATTR */
TEST_F(Read, atime_during_setattr)
{
@@ -1321,6 +1371,41 @@
tuple<bool, int>(true, 1),
tuple<bool, int>(true, 2)));
+/* With read-only mounts, fuse should never update atime during close */
+TEST_F(RofsRead, atime_during_close)
+{
+ const char FULLPATH[] = "mountpoint/some_file.txt";
+ const char RELPATH[] = "some_file.txt";
+ const char *CONTENTS = "abcdefgh";
+ struct stat sb;
+ uint64_t ino = 42;
+ int fd;
+ ssize_t bufsize = strlen(CONTENTS);
+ uint8_t buf[bufsize];
+
+ expect_lookup(RELPATH, ino, bufsize);
+ expect_open(ino, 0, 1);
+ expect_read(ino, 0, bufsize, bufsize, CONTENTS);
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([&](auto in) {
+ return (in.header.opcode == FUSE_SETATTR);
+ }, Eq(true)),
+ _)
+ ).Times(0);
+ expect_flush(ino, 1, ReturnErrno(0));
+ expect_release(ino, FuseTest::FH);
+
+ fd = open(FULLPATH, O_RDONLY);
+ ASSERT_LE(0, fd) << strerror(errno);
+
+ /* Ensure atime will be different than during lookup */
+ nap();
+
+ ASSERT_EQ(bufsize, read(fd, buf, bufsize)) << strerror(errno);
+
+ close(fd);
+}
+
/* fuse_init_out.time_gran controls the granularity of timestamps */
TEST_P(TimeGran, atime_during_setattr)
{

File Metadata

Mime Type
text/plain
Expires
Thu, May 1, 6:54 AM (17 h, 41 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
17868013
Default Alt Text
D41925.id127597.diff (6 KB)

Event Timeline