[libvirt] [PATCH 3/3] Create storage volumes directly with desired uid/gid.

In order to avoid problems trying to chown files that were created by root on a root-squashing nfs server, fork a new process that setuid's to the desired uid before creating the file. (It's only done this way if the pool containing the new volume is of type 'netfs', otherwise the old method of creating the file followed by chown() is used.) This changes the semantics of the "create_func" slightly - previously it was assumed that this function just created the file, then the caller would chown it to the desired uid. Now, create_func does both operations. There are multiple functions that can take on the role of create_func: createFileDir - previously called mkdir(), now calls virDirCreate(). virStorageBackendCreateRaw - previously called open(), now calls virFileCreate(). virStorageBackendCreateQemuImg - use virRunWithHook() to setuid/gid. virStorageBackendCreateQcowCreate - same. virStorageBackendCreateBlockFrom - preserve old behavior. --- src/storage/storage_backend.c | 115 ++++++++++++++++++++++++++++++++------ src/storage/storage_backend.h | 4 + src/storage/storage_backend_fs.c | 48 ++++++++++------ 3 files changed, 132 insertions(+), 35 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 9dc801c..8a312ca 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -229,6 +229,15 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn, goto cleanup; } + if (getuid() == 0) { + if (fchown(fd, vol->target.perms.uid, vol->target.perms.gid) < 0) { + virReportSystemError(conn, errno, + _("cannot chown %s to (%u, %u)"), + vol->target.path, vol->target.perms.uid, + vol->target.perms.gid); + goto cleanup; + } + } if (close(fd) < 0) { virReportSystemError(conn, errno, _("cannot close file '%s'"), @@ -249,10 +258,11 @@ int virStorageBackendCreateRaw(virConnectPtr conn, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { int fd = -1; int ret = -1; + int createstat; unsigned long long remain; char *buf = NULL; @@ -263,14 +273,23 @@ virStorageBackendCreateRaw(virConnectPtr conn, return -1; } - if ((fd = open(vol->target.path, O_RDWR | O_CREAT | O_EXCL, - vol->target.perms.mode)) < 0) { - virReportSystemError(conn, errno, + if ((createstat = virFileCreate(vol->target.path, vol->target.perms.mode, + vol->target.perms.uid, vol->target.perms.gid, + (flags & VIR_STORAGE_BUILD_NETFS + ? VIR_FILE_CREATE_AS_UID : 0))) < 0) { + virReportSystemError(conn, createstat, _("cannot create path '%s'"), vol->target.path); goto cleanup; } + if ((fd = open(vol->target.path, O_RDWR | O_EXCL)) < 0) { + virReportSystemError(conn, errno, + _("cannot open new path '%s'"), + vol->target.path); + goto cleanup; + } + /* Seek to the final size, so the capacity is available upfront * for progress reporting */ if (ftruncate(fd, vol->capacity) < 0) { @@ -453,12 +472,81 @@ cleanup: return ret; } +static int virStorageBuildSetUIDHook(void *data) { + virStorageVolDefPtr vol = data; + + if ((vol->target.perms.gid != 0) + && (setgid(vol->target.perms.gid) != 0)) { + virReportSystemError(NULL, errno, + _("Cannot set gid to %u before creating %s"), + vol->target.perms.gid, vol->target.path); + return -1; + } + if ((vol->target.perms.uid != 0) + && (setuid(vol->target.perms.uid) != 0)) { + virReportSystemError(NULL, errno, + _("Cannot set uid to %u before creating %s"), + vol->target.perms.uid, vol->target.path); + return -1; + } + return 0; +} + +static int virStorageBackendCreateExecCommand(virConnectPtr conn, + virStorageVolDefPtr vol, + const char **cmdargv, + unsigned int flags) { + struct stat st; + gid_t gid; + uid_t uid; + int filecreated = 0; + + if ((flags & VIR_STORAGE_BUILD_NETFS) + && (getuid() == 0) + && ((vol->target.perms.uid != 0) || (vol->target.perms.gid != 0))) { + if (virRunWithHook(conn, cmdargv, + virStorageBuildSetUIDHook, vol, NULL) == 0) { + /* command was successfully run, check if the file was created */ + if (stat(vol->target.path, &st) >=0) + filecreated = 1; + } + } + if (!filecreated) { + if (virRun(conn, cmdargv, NULL) < 0) { + virReportSystemError(NULL, errno, + _("Cannot run %s to create %s"), + cmdargv[0], vol->target.path); + return -1; + } + if (stat(vol->target.path, &st) < 0) { + virReportSystemError(NULL, errno, + _("%s failed to create %s"), + cmdargv[0], vol->target.path); + return -1; + } + } + + gid = (vol->target.perms.gid != 0) ? vol->target.perms.gid : -1; + uid = (vol->target.perms.uid != 0) ? vol->target.perms.uid : -1; + if (((gid != -1) && (gid != st.st_gid)) + || ((uid != -1) && (uid != st.st_uid))) { + /* either uid or gid wasn't properly set during creation */ + if (chown(vol->target.path, uid, gid) < 0) { + virReportSystemError(NULL, errno, _("cannot chown %s to (%u, %u)"), + vol->target.path, uid, gid); + return -1; + } + } + return 0; +} + static int virStorageBackendCreateQemuImg(virConnectPtr conn, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { + int ret; char size[100]; char *create_tool; short use_kvmimg; @@ -616,14 +704,10 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, /* Size in KB */ snprintf(size, sizeof(size), "%lluK", vol->capacity/1024); - if (virRun(conn, imgargv, NULL) < 0) { - VIR_FREE(imgargv[0]); - return -1; - } - + ret = virStorageBackendCreateExecCommand(conn, vol, imgargv, flags); VIR_FREE(imgargv[0]); - return 0; + return ret; } /* @@ -636,6 +720,7 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags ATTRIBUTE_UNUSED) { + int ret; char size[100]; const char *imgargv[4]; @@ -672,14 +757,10 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn, imgargv[2] = vol->target.path; imgargv[3] = NULL; - if (virRun(conn, imgargv, NULL) < 0) { - VIR_FREE(imgargv[0]); - return -1; - } - + ret = virStorageBackendCreateExecCommand(conn, vol, imgargv, flags); VIR_FREE(imgargv[0]); - return 0; + return ret; } virStorageBackendBuildVolFrom diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 88c6161..1e33308 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -40,6 +40,10 @@ typedef int (*virStorageBackendRefreshVol)(virConnectPtr conn, virStoragePoolObj typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, virStorageVolDefPtr origvol, virStorageVolDefPtr newvol, unsigned int flags); +enum { + VIR_STORAGE_BUILD_NETFS = (1 << 0), +}; + /* File creation/cloning functions used for cloning between backends */ int virStorageBackendCreateRaw(virConnectPtr conn, virStorageVolDefPtr vol, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 4fe40b3..8fdf8b7 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -739,7 +739,7 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn, static int createFileDir(virConnectPtr conn, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, - unsigned int flags ATTRIBUTE_UNUSED) { + unsigned int flags) { if (inputvol) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", @@ -747,7 +747,9 @@ static int createFileDir(virConnectPtr conn, return -1; } - if (mkdir(vol->target.path, vol->target.perms.mode) < 0) { + if (virDirCreate(vol->target.path, vol->target.perms.mode, + vol->target.perms.uid, vol->target.perms.gid, + flags & VIR_STORAGE_BUILD_NETFS ? VIR_FILE_CREATE_AS_UID : 0) != 0) { virReportSystemError(conn, errno, _("cannot create path '%s'"), vol->target.path); @@ -760,7 +762,8 @@ static int createFileDir(virConnectPtr conn, static int _virStorageBackendFileSystemVolBuild(virConnectPtr conn, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) + virStorageVolDefPtr inputvol, + unsigned int flags) { int fd; virStorageBackendBuildVolFrom create_func; @@ -794,7 +797,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, return -1; } - if (create_func(conn, vol, inputvol, 0) < 0) + if (create_func(conn, vol, inputvol, flags) < 0) return -1; if ((fd = open(vol->target.path, O_RDONLY)) < 0) { @@ -804,16 +807,6 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, return -1; } - /* We can only chown/grp if root */ - if (getuid() == 0) { - if (fchown(fd, vol->target.perms.uid, vol->target.perms.gid) < 0) { - virReportSystemError(conn, errno, - _("cannot set file owner '%s'"), - vol->target.path); - close(fd); - return -1; - } - } if (fchmod(fd, vol->target.perms.mode) < 0) { virReportSystemError(conn, errno, _("cannot set file mode '%s'"), @@ -840,7 +833,15 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, static int virStorageBackendFileSystemVolBuild(virConnectPtr conn, virStorageVolDefPtr vol) { - return _virStorageBackendFileSystemVolBuild(conn, vol, NULL); + return _virStorageBackendFileSystemVolBuild(conn, vol, NULL, 0); +} + +/* version for vols created in netfs-based pools */ +static int +virStorageBackendFileSystemNetVolBuild(virConnectPtr conn, + virStorageVolDefPtr vol) { + return _virStorageBackendFileSystemVolBuild(conn, vol, NULL, + VIR_STORAGE_BUILD_NETFS); } /* @@ -851,9 +852,20 @@ virStorageBackendFileSystemVolBuildFrom(virConnectPtr conn, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags ATTRIBUTE_UNUSED) { - return _virStorageBackendFileSystemVolBuild(conn, vol, inputvol); + return _virStorageBackendFileSystemVolBuild(conn, vol, inputvol, 0); +} + +/* version for vols created in netfs-based pools */ +static int +virStorageBackendFileSystemNetVolBuildFrom(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags ATTRIBUTE_UNUSED) { + return _virStorageBackendFileSystemVolBuild(conn, vol, inputvol, + VIR_STORAGE_BUILD_NETFS); } + /** * Remove a volume - just unlinks for now */ @@ -959,8 +971,8 @@ virStorageBackend virStorageBackendNetFileSystem = { .refreshPool = virStorageBackendFileSystemRefresh, .stopPool = virStorageBackendFileSystemStop, .deletePool = virStorageBackendFileSystemDelete, - .buildVol = virStorageBackendFileSystemVolBuild, - .buildVolFrom = virStorageBackendFileSystemVolBuildFrom, + .buildVol = virStorageBackendFileSystemNetVolBuild, + .buildVolFrom = virStorageBackendFileSystemNetVolBuildFrom, .createVol = virStorageBackendFileSystemVolCreate, .refreshVol = virStorageBackendFileSystemVolRefresh, .deleteVol = virStorageBackendFileSystemVolDelete, -- 1.6.6