On Fri, Nov 06, 2020 at 03:39:29PM +0100, Miklos Szeredi wrote: > On Fri, Oct 9, 2020 at 8:16 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > > > If fc->handle_killpriv_v2 is enabled, we expect file server to clear > > suid/sgid/security.capbility upon chown/truncate/write as appropriate. > > > > Upon truncate (ATTR_SIZE), suid/sgid is cleared only if caller does > > not have CAP_FSETID. File server does not know whether caller has > > CAP_FSETID or not. Hence set FATTR_KILL_PRIV upon truncate to let > > file server know that caller does not have CAP_FSETID and it should > > kill suid/sgid as appropriate. > > > > We don't have to send this information for chown (ATTR_UID/ATTR_GID) > > as that always clears suid/sgid irrespective of capabilities of > > calling process. > > I'm undecided on this. Would it hurt to set it on chown? That > might make the logic in some servers simpler, no? > > What would be the drawback of setting FATTR_KILL_PRIV for chown as well? Hi Miklos, Thinking loud. So these are the rules we expect from VFS point of view. - 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. >From server point of view, these rules become. - caps are always cleared on chown/write/truncate - suid is always cleared on chown, while for truncate/write it is cleared only if client set appropriate flag. - For truncate, this flag will either be FUSE_OPEN_KILL_PRIV or FATTR_KILL_PRIV. - For write, FUSE_WRITE_KILL_PRIV will be set. - sgid is always cleared on chown, while for truncate/write it is cleared only if caller has set a flag as well as file has group execute permission. - For truncate, this flag will either be FUSE_OPEN_KILL_PRIV or FATTR_KILL_PRIV. - For write, FUSE_WRITE_KILL_PRIV will be set. Above rules assumes that chown() will always clear caps/suid/sgid and server does not have to rely on any flags. 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). So anything is fine. We just need to document it well. I think I will write it very clearly in qemu patch depending on what goes in kernel. Thanks Vivek
