When virFileOpenAs is called with VIR_FILE_OPEN_AS_UID flag and uid/gid different from root/root while libvirtd is running as root, we fork a new child, change its effective UID/GID to uid/gid and run virFileOpenAsNoFork. It doesn't make any sense to fchown() the opened file in this case since we already know that uid/gid can access the file when open succeeds and one of the following situations may happen: - the file is already owned by uid/gid and we skip fchown even before this patch - the file is owned by uid but not gid because it was created in a directory with SETGID set, in which case it is desirable not to change the group - the file may be owned by a completely different user and/or group because it was created on a root-squashed or even all-squashed NFS filesystem, in which case fchown would most likely fail anyway --- src/util/util.c | 31 +++++++++++++++++++------------ 1 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 3d0ceea..8ff25da 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -652,7 +652,6 @@ virFileOpenAsNoFork(const char *path, int openflags, mode_t mode, { int fd = -1; int ret = 0; - struct stat st; if ((fd = open(path, openflags, mode)) < 0) { ret = -errno; @@ -660,18 +659,25 @@ virFileOpenAsNoFork(const char *path, int openflags, mode_t mode, path); goto error; } - if (fstat(fd, &st) == -1) { - ret = -errno; - virReportSystemError(errno, _("stat of '%s' failed"), path); - goto error; - } - if (((st.st_uid != uid) || (st.st_gid != gid)) - && (fchown(fd, uid, gid) < 0)) { - ret = -errno; - virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"), - path, (unsigned int) uid, (unsigned int) gid); - goto error; + + /* VIR_FILE_OPEN_AS_UID in flags means we are running in a child process + * owned by uid and gid */ + if (!(flags & VIR_FILE_OPEN_AS_UID)) { + struct stat st; + if (fstat(fd, &st) == -1) { + ret = -errno; + virReportSystemError(errno, _("stat of '%s' failed"), path); + goto error; + } + if (((st.st_uid != uid) || (st.st_gid != gid)) + && (fchown(fd, uid, gid) < 0)) { + ret = -errno; + virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"), + path, (unsigned int) uid, (unsigned int) gid); + goto error; + } } + if ((flags & VIR_FILE_OPEN_FORCE_PERMS) && (fchmod(fd, mode) < 0)) { ret = -errno; @@ -757,6 +763,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, if ((!(flags & VIR_FILE_OPEN_AS_UID)) || (getuid() != 0) || ((uid == 0) && (gid == 0))) { + flags &= ~VIR_FILE_OPEN_AS_UID; return virFileOpenAsNoFork(path, openflags, mode, uid, gid, flags); } -- 1.7.6