upstream/opnsense-src
1
0
Fork 0
mirror of https://github.com/opnsense/src.git synced 2026-05-28 04:12:45 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

fusefs: fix some bugs updating atime during close

Browse source 
When using cached attributes, we must update a file's atime during
close, if it has been read since the last attribute refresh.  But,

* Don't update atime if we lack write permissions to the file or if the
  file system is readonly.
* If the daemon fails our atime update request for any reason, don't
  report this as a failure for VOP_CLOSE.

PR:		270749
Reported by:	Jamie Landeg-Jones <jamie@catflap.org>
Sponsored by:	Axcient
Reviewed by:	pfg
Differential Revision: https://reviews.freebsd.org/D41925

(cherry picked from commit fb619c94c679e939496fe0cf94b8d2cba95e6e63)

fusefs: fix unused variables from fb619c94c67

PR:		270749
Reported by:	cy
Sponsored by:	Axcient

(cherry picked from commit e5236d25f2c0709acf3547e6af45f4bb4eec4f02)
This commit is contained in:
Alan Somers 2023-09-20 15:37:31 -06:00
parent 98504b6471
commit 13f188ce0b
4 changed files with 147 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
sys/fs/fuse/fuse_vnops.c
View file

@ -779,6 +779,7 @@ static int
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 @@ fuse_vnop_close(struct vop_close_args *ap)
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) {

2
tests/sys/fs/fusefs/access.cc
View file

@ -55,7 +55,7 @@ void expect_lookup(const char *relpath, uint64_t ino)
}
/*
* 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)

41
tests/sys/fs/fusefs/default_permissions.cc
View file

@ -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 Fspacectl: public DefaultPermissions {};
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,44 @@ TEST_F(Rename, ok_to_remove_src_because_of_stickiness)
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";
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";

83
tests/sys/fs/fusefs/read.cc
View file

@ -57,6 +57,14 @@ void expect_lookup(const char *relpath, uint64_t ino, uint64_t size)
}
};
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,47 @@ TEST_F(Read, atime_during_close)
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";
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 +1370,40 @@ INSTANTIATE_TEST_SUITE_P(RA, ReadAhead,
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";
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)
{