[PATCH] util: Don't try to fchown files opened as non-root

[Jiri Denemark <jdenemar@xxxxxxxxxx>]

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list