Re: [Virtio-fs] [PATCH v3 3/6] fuse: setattr should set FATTR_KILL_PRIV upon size change

[Vivek Goyal <vgoyal@redhat.com>]

On Wed, Nov 11, 2020 at 02:54:43PM +0100, Miklos Szeredi wrote:
> On Fri, Nov 6, 2020 at 6:18 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> 
> > I think it does not hurt to start passing FATTR_KILL_PRIV for chown()
> > as well. In that case, server will always clear caps on chown but
> > clear suid/sgid only if FATTR_KILL_PRIV is set. (Which will always
> > be set).
> 
> Okay.
> 
> More thoughts for FUSE_HANDLE_KILLPRIV_V2:
> 
>  - clear "security.capability" on write, truncate and chown unconditionally
>  - clear suid/sgid if
>     o setattr has FATTR_SIZE and  FATTR_KILL_PRIV
>     o setattr has FATTR_UID or FATTR_GID
>     o open has O_TRUNC and FUSE_OPEN_KILL_PRIV
>     o write has FUSE_WRITE_KILL_PRIV
> 
> Kernel has:
> ATTR_KILL_PRIV -> clear "security.capability"
> ATTR_KILL_SUID -> clear S_ISUID
> ATTR_KILL_SGID -> clear S_ISGID if executable
> 
> Fuse has:
> FUSE_*KILL_PRIV -> clear S_ISUID and S_ISGID if executable
> 
> So the fuse meaning of FUSE_*KILL_PRIV has a complementary meaning to
> that of ATTR_KILL_PRIV, which is somewhat confusing.  Also "PRIV"
> implies all privileges, including "security.capability" but the fuse
> ones relate to suid/sgid only.
> 
> How about FUSE_*KILL_SUIDGID (FUSE_WRITE_KILL_SUIDGID being an alias
> for FUSE_WRITE_KILL_PRIV)?

Hi Miklos,

Renaming FUSE_*KILL_PRIV to FUSE_*KILL_SUIDSGID sounds good. For a
breif moment I was also thinking that these FUSE_*KILL_PRIV and
and ATTR_KILL_PRIV are not exactly mapping. Glad you caught it
and made the situation better.

Thanks
Vivek