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

2011-07-08 Thread Jiri Denemark
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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2011-07-08 Thread Eric Blake
On 07/08/2011 07:30 AM, Jiri Denemark wrote:
 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(-)

ACK.

 +/* 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) {

Style nit - add a newline between the declaration of st and the first
statement (the nested if).

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list