Re: [libvirt] [PATCH 3/3] util: virfile: Only setuid for virFileRemove if on NFS

NFS with root-squash is the only reason we need to do setuid/setgid crazyness in virFileRemove, so limit that behavior to the NFS case. --- I'm not sure though if NFS is the only case we care about this here, or if we want to conditionalize this path on NFS since that makes it more of a pain to test... It's not required to fix the initial bug src/util/virfile.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index cea2674..3d1b118 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2322,7 +2322,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, * owned by the passed uid/gid pair. Needed for NFS with root-squash */ static bool -virFileRemoveNeedsSetuid(uid_t uid, gid_t gid) +virFileRemoveNeedsSetuid(const char *path, uid_t uid, gid_t gid)

{ /* If running unprivileged, setuid isn't going to work */ if (geteuid() != 0) @@ -2336,6 +2336,12 @@ virFileRemoveNeedsSetuid(uid_t uid, gid_t gid) if (uid == geteuid() && gid == getegid()) return false; + /* Only perform the setuid stuff for NFS, which is the only case + that may actually need it. This can error, but just be safe and + only check for a clear negative result. */ + if (virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS) == 0) + return false; +

return true; } @@ -2361,7 +2367,7 @@ virFileRemove(const char *path, gid_t *groups; int ngroups; - if (!virFileRemoveNeedsSetuid(uid, gid)) { + if (!virFileRemoveNeedsSetuid(path, uid, gid)) { if (virFileIsDir(path)) return rmdir(path); else