Re: [Virtio-fs] [PATCH v2 2/2] viriofsd: Add support for FUSE_HANDLE_KILLPRIV_V2

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3489 bytes --]

On Thu, Nov 12, 2020 at 01:24:18PM -0500, Vivek Goyal wrote:
> This patch adds basic support for FUSE_HANDLE_KILLPRIV_V2. virtiofsd
> can enable/disable this by specifying option "-o killpriv_v2/no_killpriv_v2".
> By default this is enabled as long as client supports it
> 
> Enabling this option helps with performance in write path. Without this
> option, currently every write is first preceeded with a getxattr() operation
> to find out if security.capability is set. (Write is supposed to clear
> security.capability). With this option enabled, server is signing up for
> clearing security.capability on every WRITE and also clearing suid/sgid
> subject to certain rules. This gets rid of extra getxattr() call for every
> WRITE and improves performance. This is true when virtiofsd is run with
> option -o xattr.
> 
> What does enabling FUSE_HANDLE_KILLPRIV_V2 mean for file server implementation.
> It needs to adhere to following rules. Thanks to Miklos for this summary.
> 
> - clear "security.capability" on write, truncate and chown unconditionally
> - clear suid/sgid in case of following. Note, sgid is cleared only if
>   group executable bit is set.
>     o setattr has FATTR_SIZE and FATTR_KILL_SUIDGID set.
>     o setattr has FATTR_UID or FATTR_GID
>     o open has O_TRUNC and FUSE_OPEN_KILL_SUIDGID
>     o create has O_TRUNC and FUSE_OPEN_KILL_SUIDGID flag set.
>     o write has FUSE_WRITE_KILL_SUIDGID
> 
> >From Linux VFS client perspective, here are the requirements.
> 
> - caps are always cleared on chown/write/truncate
> - suid is always cleared on chown, while for truncate/write it is cleared
>   only if caller does not have CAP_FSETID.
> - sgid is always cleared on chown, while for truncate/write it is cleared
>   only if caller does not have CAP_FSETID as well as file has group execute
>   permission.
> 
> virtiofsd implementation has not changed much to adhere to above ruls. And
> reason being that current assumption is that we are running on Linux
> and on top of filesystems like ext4/xfs which already follow above rules.
> On write, truncate, chown, seucurity.capability is cleared. And virtiofsd
> drops CAP_FSETID if need be and that will lead to clearing of suid/sgid.
> 
> But if virtiofsd is running on top a filesystem which breaks above assumptions,
> then it will have to take extra actions to emulate above. That's a TODO
> for later when need arises.
> 
> Note: create normally is supposed to be called only when file does not
>       exist. So generally there should not be any question of clearing
>       setuid/setgid. But it is possible that after client checks that
>       file is not present, some other client creates file on server
>       and this race can trigger sending FUSE_CREATE. In that case, if
>       O_TRUNC is set, we should clear suid/sgid if FUSE_OPEN_KILL_SUIDGID
>       is also set.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  include/standard-headers/linux/fuse.h |  28 ++++++-
>  tools/virtiofsd/fuse_common.h         |  15 ++++
>  tools/virtiofsd/fuse_lowlevel.c       |  11 ++-
>  tools/virtiofsd/fuse_lowlevel.h       |   1 +
>  tools/virtiofsd/passthrough_ll.c      | 108 +++++++++++++++++++++++---
>  5 files changed, 148 insertions(+), 15 deletions(-)

Looks complicated but the implementation is simple since we rely on the
host file system to honor the semantics.

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]