All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: linux-fsdevel@vger.kernel.org, virtio-fs@redhat.com, miklos@szeredi.hu
Cc: vgoyal@redhat.com, stefanha@redhat.com, dgilbert@redhat.com
Subject: [PATCH 2/2] fuse: Enable SB_NOSEC if filesystem is not shared
Date: Tue,  1 Sep 2020 16:40:45 -0400	[thread overview]
Message-ID: <20200901204045.1250822-3-vgoyal@redhat.com> (raw)
In-Reply-To: <20200901204045.1250822-1-vgoyal@redhat.com>

We don't enable SB_NOSEC on fuse filesystems thinking filesystem is
shared and files attrs setuid/setgid/capabilities can change without
fuse knowing about it. This means on every WRITE, file_remove_privs(),
is called and that calls into fuse server to figure out if security.capability
xattr has been set on file. Most of the time this is a performance hog,
specially for small writes done at high frequency.

Enable SB_NOSEC if fuse filesystem sets flag FS_NONSHARED_FS. This means,
do not expect file attrs/xattrs to change without the knowledge of
fuse. In this case it should be possible to enable SB_NOSEC.

For the case of shared filesystems, we will have to come up with a different
mechanism to enable SB_NOSEC. I guess it will depend on invalidation
mechanisms implemented by filesystem and cache coherency guarantees.

I do clear inode S_NOSEC flag whenever file attrs are being refreshed. So
this still honors attr timeout protocol.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/fuse/inode.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 088faa3e352c..2da13fe25417 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -187,6 +187,9 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
 		inode->i_mode &= ~S_ISVTX;
 
 	fi->orig_ino = attr->ino;
+
+	/* Clear S_NOSEC whenever cached attrs are being refreshed */
+	inode->i_flags &= ~S_NOSEC;
 }
 
 void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
@@ -967,6 +970,9 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
 			}
 			if (arg->flags & FUSE_NONSHARED_FS) {
 				fc->nonshared_fs = 1;
+				down_write(&fc->sb->s_umount);
+				fc->sb->s_flags |= SB_NOSEC;
+				up_write(&fc->sb->s_umount);
 			}
 		} else {
 			ra_pages = fc->max_read / PAGE_SIZE;
-- 
2.25.4


WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com>
To: linux-fsdevel@vger.kernel.org, virtio-fs@redhat.com, miklos@szeredi.hu
Cc: vgoyal@redhat.com
Subject: [Virtio-fs] [PATCH 2/2] fuse: Enable SB_NOSEC if filesystem is not shared
Date: Tue,  1 Sep 2020 16:40:45 -0400	[thread overview]
Message-ID: <20200901204045.1250822-3-vgoyal@redhat.com> (raw)
In-Reply-To: <20200901204045.1250822-1-vgoyal@redhat.com>

We don't enable SB_NOSEC on fuse filesystems thinking filesystem is
shared and files attrs setuid/setgid/capabilities can change without
fuse knowing about it. This means on every WRITE, file_remove_privs(),
is called and that calls into fuse server to figure out if security.capability
xattr has been set on file. Most of the time this is a performance hog,
specially for small writes done at high frequency.

Enable SB_NOSEC if fuse filesystem sets flag FS_NONSHARED_FS. This means,
do not expect file attrs/xattrs to change without the knowledge of
fuse. In this case it should be possible to enable SB_NOSEC.

For the case of shared filesystems, we will have to come up with a different
mechanism to enable SB_NOSEC. I guess it will depend on invalidation
mechanisms implemented by filesystem and cache coherency guarantees.

I do clear inode S_NOSEC flag whenever file attrs are being refreshed. So
this still honors attr timeout protocol.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/fuse/inode.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 088faa3e352c..2da13fe25417 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -187,6 +187,9 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
 		inode->i_mode &= ~S_ISVTX;
 
 	fi->orig_ino = attr->ino;
+
+	/* Clear S_NOSEC whenever cached attrs are being refreshed */
+	inode->i_flags &= ~S_NOSEC;
 }
 
 void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
@@ -967,6 +970,9 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
 			}
 			if (arg->flags & FUSE_NONSHARED_FS) {
 				fc->nonshared_fs = 1;
+				down_write(&fc->sb->s_umount);
+				fc->sb->s_flags |= SB_NOSEC;
+				up_write(&fc->sb->s_umount);
 			}
 		} else {
 			ra_pages = fc->max_read / PAGE_SIZE;
-- 
2.25.4


  parent reply	other threads:[~2020-09-01 20:41 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 20:40 [RFC PATCH 0/2] fuse: Enable SB_NOSEC if filesystem is not shared Vivek Goyal
2020-09-01 20:40 ` [Virtio-fs] " Vivek Goyal
2020-09-01 20:40 ` [PATCH 1/2] fuse: Add a flag FUSE_NONSHARED_FS Vivek Goyal
2020-09-01 20:40   ` [Virtio-fs] " Vivek Goyal
2020-09-02  6:57   ` Miklos Szeredi
2020-09-02  6:57     ` [Virtio-fs] " Miklos Szeredi
2020-09-02 18:08     ` Vivek Goyal
2020-09-02 18:08       ` [Virtio-fs] " Vivek Goyal
2020-09-01 20:40 ` Vivek Goyal [this message]
2020-09-01 20:40   ` [Virtio-fs] [PATCH 2/2] fuse: Enable SB_NOSEC if filesystem is not shared Vivek Goyal
2020-09-01 20:46 ` [RFC PATCH 0/2] " Vivek Goyal
2020-09-01 20:46   ` [Virtio-fs] " Vivek Goyal
2020-09-02 19:14 ` Vivek Goyal
2020-09-02 19:14   ` [Virtio-fs] " Vivek Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200901204045.1250822-3-vgoyal@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=stefanha@redhat.com \
    --cc=virtio-fs@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.