From 9db3538517d15c4dc39cda852e0f9a70a1e87cfa Mon Sep 17 00:00:00 2001 From: asomers Date: Tue, 28 May 2019 01:09:19 +0000 Subject: [PATCH] fusefs: set the flags fields of fuse_write_in and fuse_read_in These fields are supposed to contain the file descriptor flags as supplied to open(2) or set by fcntl(2). The feature is kindof useless on FreeBSD since we don't supply all of these flags to fuse (because of the weak relationship between struct file and struct vnode). But we should at least set the access mode flags (O_RDONLY, etc). This is the last fusefs change needed to get full protocol 7.9 support. There are still a few options we don't support for good reason (mandatory file locking is dumb, flock support is broken in the protocol until 7.17, etc), but there's nothing else to do at this protocol level. Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_file.c | 38 ----------------------------------- sys/fs/fuse/fuse_file.h | 35 ++++++++++++++++++++++++++++++++ sys/fs/fuse/fuse_internal.c | 12 ++++++++++- sys/fs/fuse/fuse_io.c | 4 ++-- tests/sys/fs/fusefs/mockfs.cc | 6 +++++- tests/sys/fs/fusefs/utils.cc | 11 ++++++++-- tests/sys/fs/fusefs/utils.hh | 2 +- tests/sys/fs/fusefs/write.cc | 2 +- 8 files changed, 64 insertions(+), 46 deletions(-) diff --git a/sys/fs/fuse/fuse_file.c b/sys/fs/fuse/fuse_file.c index f505534d15c..807a832a7af 100644 --- a/sys/fs/fuse/fuse_file.c +++ b/sys/fs/fuse/fuse_file.c @@ -114,44 +114,6 @@ fflags_2_fufh_type(int fflags) panic("FUSE: What kind of a flag is this (%x)?", fflags); } -/* - * Get the flags to use for FUSE_CREATE, FUSE_OPEN and FUSE_RELEASE - * - * These are supposed to be the same as the flags argument to open(2). - * However, since we can't reliably associate a fuse_filehandle with a specific - * file descriptor it would would be dangerous to include anything more than - * the access mode flags. For example, suppose we open a file twice, once with - * O_APPEND and once without. Then the user pwrite(2)s to offset using the - * second file descriptor. If fusefs uses the first file handle, then the - * server may append the write to the end of the file rather than at offset 0. - * To prevent problems like this, we only ever send the portion of flags - * related to access mode. - * - * It's essential to send that portion, because FUSE uses it for server-side - * authorization. - * - * TODO: consider sending O_APPEND after upgrading to protocol 7.9, which - * includes flags in fuse_write_in. - */ -static inline int -fufh_type_2_fflags(fufh_type_t type) -{ - int oflags = -1; - - switch (type) { - case FUFH_RDONLY: - case FUFH_WRONLY: - case FUFH_RDWR: - case FUFH_EXEC: - oflags = type; - break; - default: - break; - } - - return oflags; -} - int fuse_filehandle_open(struct vnode *vp, int a_mode, struct fuse_filehandle **fufhp, struct thread *td, struct ucred *cred) diff --git a/sys/fs/fuse/fuse_file.h b/sys/fs/fuse/fuse_file.h index 468024bc5cd..6c6d21446de 100644 --- a/sys/fs/fuse/fuse_file.h +++ b/sys/fs/fuse/fuse_file.h @@ -148,6 +148,41 @@ struct fuse_filehandle { #define FUFH_IS_VALID(f) ((f)->fufh_type != FUFH_INVALID) +/* + * Get the flags to use for FUSE_CREATE, FUSE_OPEN and FUSE_RELEASE + * + * These are supposed to be the same as the flags argument to open(2). + * However, since we can't reliably associate a fuse_filehandle with a specific + * file descriptor it would would be dangerous to include anything more than + * the access mode flags. For example, suppose we open a file twice, once with + * O_APPEND and once without. Then the user pwrite(2)s to offset using the + * second file descriptor. If fusefs uses the first file handle, then the + * server may append the write to the end of the file rather than at offset 0. + * To prevent problems like this, we only ever send the portion of flags + * related to access mode. + * + * It's essential to send that portion, because FUSE uses it for server-side + * authorization. + */ +static inline int +fufh_type_2_fflags(fufh_type_t type) +{ + int oflags = -1; + + switch (type) { + case FUFH_RDONLY: + case FUFH_WRONLY: + case FUFH_RDWR: + case FUFH_EXEC: + oflags = type; + break; + default: + break; + } + + return oflags; +} + bool fuse_filehandle_validrw(struct vnode *vp, int mode, struct ucred *cred, pid_t pid); int fuse_filehandle_get(struct vnode *vp, int fflag, diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index 7ad14a52a6e..575530d4387 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -779,6 +779,10 @@ fuse_internal_init_callback(struct fuse_ticket *tick, struct uio *uio) data->dataflags |= FSESS_POSIX_LOCKS; if (fiio->flags & FUSE_EXPORT_SUPPORT) data->dataflags |= FSESS_EXPORT_SUPPORT; + /* + * Don't bother to check FUSE_BIG_WRITES, because it's + * redundant with max_write + */ } else { err = EINVAL; } @@ -816,7 +820,13 @@ fuse_internal_send_init(struct fuse_data *data, struct thread *td) * the size of a buffer cache block. */ fiii->max_readahead = maxbcachebuf; - fiii->flags = FUSE_ASYNC_READ | FUSE_POSIX_LOCKS | FUSE_EXPORT_SUPPORT ; + /* + * Unsupported features: + * FUSE_FILE_OPS: No known FUSE server or client supports it + * FUSE_ATOMIC_O_TRUNC: our VFS cannot support it + */ + fiii->flags = FUSE_ASYNC_READ | FUSE_POSIX_LOCKS | FUSE_EXPORT_SUPPORT + | FUSE_BIG_WRITES; fuse_insert_callback(fdi.tick, fuse_internal_init_callback); fuse_insert_message(fdi.tick, false); diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c index 26360d070e5..e76ec80584f 100644 --- a/sys/fs/fuse/fuse_io.c +++ b/sys/fs/fuse/fuse_io.c @@ -380,7 +380,7 @@ fuse_read_directbackend(struct vnode *vp, struct uio *uio, if (fuse_libabi_geq(data, 7, 9)) { /* See comment regarding FUSE_WRITE_LOCKOWNER */ fri->read_flags = 0; - fri->flags = 0; /* TODO */ + fri->flags = fufh_type_2_fflags(fufh->fufh_type); } SDT_PROBE1(fusefs, , io, read_directbackend_start, fri); @@ -461,7 +461,7 @@ fuse_write_directbackend(struct vnode *vp, struct uio *uio, fwi->size = chunksize; fwi->write_flags = write_flags; if (fuse_libabi_geq(data, 7, 9)) { - fwi->flags = 0; /* TODO */ + fwi->flags = fufh_type_2_fflags(fufh->fufh_type); fwi_data = (char *)fdi.indata + sizeof(*fwi); } else { fwi_data = (char *)fdi.indata + diff --git a/tests/sys/fs/fusefs/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc index b8dfd99e992..b178e81f5da 100644 --- a/tests/sys/fs/fusefs/mockfs.cc +++ b/tests/sys/fs/fusefs/mockfs.cc @@ -218,6 +218,8 @@ void debug_fuseop(const mockfs_buf_in &in) printf(" offset=%" PRIu64 " size=%u", in.body.read.offset, in.body.read.size); + if (verbosity > 1) + printf(" flags=%#x", in.body.read.flags); break; case FUSE_READDIR: printf(" fh=%#" PRIx64 " offset=%" PRIu64 " size=%u", @@ -278,10 +280,12 @@ void debug_fuseop(const mockfs_buf_in &in) break; case FUSE_WRITE: printf(" fh=%#" PRIx64 " offset=%" PRIu64 - " size=%u flags=%u", + " size=%u write_flags=%u", in.body.write.fh, in.body.write.offset, in.body.write.size, in.body.write.write_flags); + if (verbosity > 1) + printf(" flags=%#x", in.body.write.flags); break; default: break; diff --git a/tests/sys/fs/fusefs/utils.cc b/tests/sys/fs/fusefs/utils.cc index 989f52e0f96..6338a5967e1 100644 --- a/tests/sys/fs/fusefs/utils.cc +++ b/tests/sys/fs/fusefs/utils.cc @@ -36,6 +36,7 @@ extern "C" { #include #include +#include #include #include #include @@ -273,7 +274,7 @@ void FuseTest::expect_opendir(uint64_t ino) } void FuseTest::expect_read(uint64_t ino, uint64_t offset, uint64_t isize, - uint64_t osize, const void *contents) + uint64_t osize, const void *contents, int flags) { EXPECT_CALL(*m_mock, process( ResultOf([=](auto in) { @@ -281,7 +282,11 @@ void FuseTest::expect_read(uint64_t ino, uint64_t offset, uint64_t isize, in.header.nodeid == ino && in.body.read.fh == FH && in.body.read.offset == offset && - in.body.read.size == isize); + in.body.read.size == isize && + flags == -1 ? + (in.body.read.flags == O_RDONLY || + in.body.read.flags == O_RDWR) + : in.body.read.flags == (uint32_t)flags); }, Eq(true)), _) ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) { @@ -400,6 +405,8 @@ void FuseTest::expect_write(uint64_t ino, uint64_t offset, uint64_t isize, pid_ok && (wf & flags_set) == flags_set && (wf & flags_unset) == 0 && + (in.body.write.flags == O_WRONLY || + in.body.write.flags == O_RDWR) && 0 == bcmp(buf, contents, isize)); }, Eq(true)), _) diff --git a/tests/sys/fs/fusefs/utils.hh b/tests/sys/fs/fusefs/utils.hh index 9239ef39b78..b30583e748f 100644 --- a/tests/sys/fs/fusefs/utils.hh +++ b/tests/sys/fs/fusefs/utils.hh @@ -139,7 +139,7 @@ class FuseTest : public ::testing::Test { * nothing currently validates the size of the fuse_read_in struct. */ void expect_read(uint64_t ino, uint64_t offset, uint64_t isize, - uint64_t osize, const void *contents); + uint64_t osize, const void *contents, int flags = -1); /* * Create an expectation that FUSE_READIR will be called any number of diff --git a/tests/sys/fs/fusefs/write.cc b/tests/sys/fs/fusefs/write.cc index f7c7f06ed6a..95ad364edf9 100644 --- a/tests/sys/fs/fusefs/write.cc +++ b/tests/sys/fs/fusefs/write.cc @@ -636,7 +636,7 @@ TEST_F(WriteBack, rmw) FuseTest::expect_lookup(RELPATH, ino, S_IFREG | 0644, fsize, 1); expect_open(ino, 0, 1); - expect_read(ino, 0, fsize, fsize, INITIAL); + expect_read(ino, 0, fsize, fsize, INITIAL, O_WRONLY); expect_write(ino, offset, bufsize, bufsize, CONTENTS); fd = open(FULLPATH, O_WRONLY); -- 2.45.0