CHROMIUM: fuse: Set SB_NOSEC for virtiofs
Currently when caching is enabled the fuse driver sends a getxattr
request for every write request to see if the security.capability xattr
is set. This adds a significant amount of latency to every write
operation. However, the kernel provides a flag that can be set on the
file system superblock (SB_NOSEC) that effectively allows the kernel to
cache the value of the xattr. This is disabled by default for fuse.
Enable SB_NOSEC for virtiofs. This requires making some assumptions
that we know are true for virtiofs but may not be true for generic fuse
servers. The proper fix is being worked on upstream (the latest patch
series is [1]) so this is just a temporary solution until the upstream
fix can be backported.
BUG=b:163383485
TEST=`fio --ioengine=libaio --direct=1 --name=test --filename=$FILENAME
--bs=4k --iodepth=64 --size=4G --readwrite=randwrite`
Without this change the fio benchmark gives a write bandwidth of
26 MiB/s. With this change the fio benchmark gives a write
bandwidth of 194 MiB/s, which is a very significant improvement.
[1]: https://lore.kernel.org/linux-fsdevel/20201009181512.65496-1-vgoyal@redhat.com/
Change-Id: I074b009c122a3abeccdb7e23beae284b6ec3d3ec
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2486004
Reviewed-by: Suleiman Souhlal <suleiman@chromium.org>
Reviewed-by: Lepton Wu <lepton@chromium.org>
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index ab4fc12..d5a8c6d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1274,16 +1274,21 @@
ssize_t written = 0;
ssize_t written_buffered = 0;
struct inode *inode = mapping->host;
+ struct fuse_conn *fc = get_fuse_conn(inode);
ssize_t err;
loff_t endbyte = 0;
- if (get_fuse_conn(inode)->writeback_cache) {
+ if (fc->writeback_cache) {
/* Update size (EOF optimization) and mode (SUID clearing) */
err = fuse_update_attributes(mapping->host, file);
if (err)
return err;
- return generic_file_write_iter(iocb, from);
+ if (!fc->handle_killpriv ||
+ !should_remove_suid(file->f_path.dentry))
+ return generic_file_write_iter(iocb, from);
+
+ /* Fall back to unbuffered write to remove suid/sgid bits */
}
inode_lock(inode);
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 4ee3df2..46b7914 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -8,6 +8,7 @@
#include "fuse_i.h"
+#include <linux/fs.h>
#include <linux/pagemap.h>
#include <linux/slab.h>
#include <linux/file.h>
@@ -187,6 +188,13 @@
inode->i_mode &= ~S_ISVTX;
fi->orig_ino = attr->ino;
+
+ /*
+ * Reset S_NOSEC since the file server observed that the suid/sgid bit
+ * is set.
+ */
+ if (IS_NOSEC(inode) && is_sxid(inode->i_mode))
+ inode->i_flags &= ~S_NOSEC;
}
void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 7505f81..6e38b57 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1127,6 +1127,22 @@
/* Previous unmount will stop all queues. Start these again */
virtio_fs_start_all_queues(fs);
fuse_send_init(fc);
+
+ /*
+ * We set the SB_NOSEC flag for virtiofs to improve small write
+ * performance. This requires that the server kills the suid/sgid bits
+ * as well as the security.capability xattr on chown() as well as on
+ * truncate() and write() if the caller does not have CAP_FSETID.
+ * We know this is true for our virtiofs implementation but we cannot
+ * make this assumption for general fuse servers. The proper fix is
+ * being worked on upstream so this is just a temporary hack until the
+ * upstream solution can be merged.
+ *
+ * TODO(b/163383485): Revert this once the proper fix has landed
+ * upstream and can be backported.
+ */
+ sb->s_flags |= SB_NOSEC;
+
mutex_unlock(&virtio_fs_mutex);
return 0;
