It turns out it is also useful to be able to perform other operations on a file created while running as a different uid (eg, write things to that file), and possibly to do this to a file that already exists. This patch adds an optional hook function to the renamed (for more accuracy of purpose) virFileOperation; the hook will be called after the file has been opened (possibly created) and gid/mode checked/set, before closing it. As with the other operations on the file, if the VIR_FILE_OP_AS_UID flag is set, this hook function will be called in the context of a child process forked from the process that called virFileOperation. The implication here is that, while all data in memory is available to this hook function, any modification to that data will not be seen by the caller - the only indication in memory of what happened in the hook will be the return value (which the hook should set to 0 on success, or one of the standard errno values on failure). Another piece of making the function more flexible was to add an "openflags" argument. This arg should contain exactly the flags to be passed to open(2), eg O_RDWR | O_EXCL, etc. In the process of adding the hook to virFileOperation, I also realized that the bits to fix up file owner/group/mode settings after creation were being done in the parent process, which could fail, so I moved them to the child process where they should be. util/util.c|h: rename and rework virFileCreate-->virFileOperation, and redo flags in virDirCreate. storage/storage_backend.c, storage/storage_backend_fs.c: update the calls to virFileOperation/virDirCreate to reflect changes in the API, but don't yet take advantage of the hook. --- src/storage/storage_backend.c | 11 ++- src/storage/storage_backend_fs.c | 7 +- src/util/util.c | 156 ++++++++++++++++++++------------------ src/util/util.h | 19 ++++- 4 files changed, 106 insertions(+), 87 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index a12ddc7..07c116a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -290,10 +290,13 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } - if ((createstat = virFileCreate(vol->target.path, vol->target.perms.mode, - vol->target.perms.uid, vol->target.perms.gid, - (pool->def->type == VIR_STORAGE_POOL_NETFS - ? VIR_FILE_CREATE_AS_UID : 0))) < 0) { + if ((createstat = virFileOperation(vol->target.path, O_RDWR | O_CREAT | O_EXCL, + vol->target.perms.mode, + vol->target.perms.uid, vol->target.perms.gid, + NULL, NULL, + VIR_FILE_OP_FORCE_PERMS | + (pool->def->type == VIR_STORAGE_POOL_NETFS + ? VIR_FILE_OP_AS_UID : 0))) < 0) { virReportSystemError(createstat, _("cannot create path '%s'"), vol->target.path); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bbd5787..8279d78 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -533,9 +533,9 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, pool->def->target.perms.mode, pool->def->target.perms.uid, pool->def->target.perms.gid, - VIR_FILE_CREATE_ALLOW_EXIST | + VIR_DIR_CREATE_FORCE_PERMS | VIR_DIR_CREATE_ALLOW_EXIST | (pool->def->type == VIR_STORAGE_POOL_NETFS - ? VIR_FILE_CREATE_AS_UID : 0)) != 0)) { + ? VIR_DIR_CREATE_AS_UID : 0)) != 0)) { virReportSystemError(err, _("cannot create path '%s'"), pool->def->target.path); goto error; @@ -779,8 +779,9 @@ static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, if ((err = virDirCreate(vol->target.path, vol->target.perms.mode, vol->target.perms.uid, vol->target.perms.gid, + VIR_DIR_CREATE_FORCE_PERMS | (pool->def->type == VIR_STORAGE_POOL_NETFS - ? VIR_FILE_CREATE_AS_UID : 0))) != 0) { + ? VIR_DIR_CREATE_AS_UID : 0))) != 0) { virReportSystemError(err, _("cannot create path '%s'"), vol->target.path); return -1; diff --git a/src/util/util.c b/src/util/util.c index c3b4084..d3bbfe1 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1212,15 +1212,15 @@ int virFileExists(const char *path) } -static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t gid, - unsigned int flags) { - int open_flags = O_RDWR | O_CREAT | ((flags & VIR_FILE_CREATE_ALLOW_EXIST) - ? 0 : O_EXCL); +static int virFileOperationNoFork(const char *path, int openflags, mode_t mode, + uid_t uid, gid_t gid, + virFileOperationHook hook, void *hookdata, + unsigned int flags) { int fd = -1; int ret = 0; struct stat st; - if ((fd = open(path, open_flags, mode)) < 0) { + if ((fd = open(path, openflags, mode)) < 0) { ret = errno; virReportSystemError(errno, _("failed to create file '%s'"), path); @@ -1238,13 +1238,17 @@ static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t g path, uid, gid); goto error; } - if (fchmod(fd, mode) < 0) { + if ((flags & VIR_FILE_OP_FORCE_PERMS) + && (fchmod(fd, mode) < 0)) { ret = errno; virReportSystemError(errno, _("cannot set mode of '%s' to %04o"), path, mode); goto error; } + if ((hook) && ((ret = hook(fd, hookdata)) != 0)) { + goto error; + } if (close(fd) < 0) { ret = errno; virReportSystemError(errno, _("failed to close new file '%s'"), @@ -1259,13 +1263,13 @@ error: return ret; } -static int virDirCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t gid, +static int virDirCreateNoFork(const char *path, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) { int ret = 0; struct stat st; if ((mkdir(path, mode) < 0) - && !((errno == EEXIST) && (flags & VIR_FILE_CREATE_ALLOW_EXIST))) + && !((errno == EEXIST) && (flags & VIR_DIR_CREATE_ALLOW_EXIST))) { ret = errno; virReportSystemError(errno, _("failed to create directory '%s'"), @@ -1285,7 +1289,8 @@ static int virDirCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t gi path, uid, gid); goto error; } - if (chmod(path, mode) < 0) { + if ((flags & VIR_DIR_CREATE_FORCE_PERMS) + && (chmod(path, mode) < 0)) { ret = errno; virReportSystemError(errno, _("cannot set mode of '%s' to %04o"), @@ -1297,18 +1302,20 @@ error: } #ifndef WIN32 -int virFileCreate(const char *path, mode_t mode, - uid_t uid, gid_t gid, unsigned int flags) { +int virFileOperation(const char *path, int openflags, mode_t mode, + uid_t uid, gid_t gid, + virFileOperationHook hook, void *hookdata, + unsigned int flags) { struct stat st; pid_t pid; int waitret, status, ret = 0; int fd; - if ((!(flags & VIR_FILE_CREATE_AS_UID)) + if ((!(flags & VIR_FILE_OP_AS_UID)) || (getuid() != 0) - || ((uid == 0) && (gid == 0)) - || ((flags & VIR_FILE_CREATE_ALLOW_EXIST) && (stat(path, &st) >= 0))) { - return virFileCreateSimple(path, mode, uid, gid, flags); + || ((uid == 0) && (gid == 0))) { + return virFileOperationNoFork(path, openflags, mode, uid, gid, + hook, hookdata, flags); } /* parent is running as root, but caller requested that the @@ -1338,34 +1345,8 @@ int virFileCreate(const char *path, mode_t mode, if (!WIFEXITED(status) || (ret == EACCES)) { /* fall back to the simpler method, which works better in * some cases */ - return virFileCreateSimple(path, mode, uid, gid, flags); - } - if (ret != 0) { - goto parenterror; - } - - /* check if group was set properly by creating after - * setgid. If not, try doing it with chown */ - if (stat(path, &st) == -1) { - ret = errno; - virReportSystemError(errno, - _("stat of '%s' failed"), - path); - goto parenterror; - } - if ((st.st_gid != gid) && (chown(path, -1, gid) < 0)) { - ret = errno; - virReportSystemError(errno, - _("cannot chown '%s' to group %u"), - path, gid); - goto parenterror; - } - if (chmod(path, mode) < 0) { - ret = errno; - virReportSystemError(errno, - _("cannot set mode of '%s' to %04o"), - path, mode); - goto parenterror; + return virFileOperationNoFork(path, openflags, mode, uid, gid, + hook, hookdata, flags); } parenterror: return ret; @@ -1395,7 +1376,7 @@ parenterror: uid, path); goto childerror; } - if ((fd = open(path, O_RDWR | O_CREAT | O_EXCL, mode)) < 0) { + if ((fd = open(path, openflags, mode)) < 0) { ret = errno; if (ret != EACCES) { /* in case of EACCES, the parent will retry */ @@ -1405,6 +1386,29 @@ parenterror: } goto childerror; } + if (fstat(fd, &st) == -1) { + ret = errno; + virReportSystemError(errno, _("stat of '%s' failed"), path); + goto childerror; + } + if ((st.st_gid != gid) + && (fchown(fd, -1, gid) < 0)) { + ret = errno; + virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"), + path, uid, gid); + goto childerror; + } + if ((flags & VIR_FILE_OP_FORCE_PERMS) + && (fchmod(fd, mode) < 0)) { + ret = errno; + virReportSystemError(errno, + _("cannot set mode of '%s' to %04o"), + path, mode); + goto childerror; + } + if ((hook) && ((ret = hook(fd, hookdata)) != 0)) { + goto childerror; + } if (close(fd) < 0) { ret = errno; virReportSystemError(errno, _("child failed to close new file '%s'"), @@ -1423,11 +1427,11 @@ int virDirCreate(const char *path, mode_t mode, int waitret; int status, ret = 0; - if ((!(flags & VIR_FILE_CREATE_AS_UID)) + if ((!(flags & VIR_DIR_CREATE_AS_UID)) || (getuid() != 0) || ((uid == 0) && (gid == 0)) - || ((flags & VIR_FILE_CREATE_ALLOW_EXIST) && (stat(path, &st) >= 0))) { - return virDirCreateSimple(path, mode, uid, gid, flags); + || ((flags & VIR_DIR_CREATE_ALLOW_EXIST) && (stat(path, &st) >= 0))) { + return virDirCreateNoFork(path, mode, uid, gid, flags); } int forkRet = virFork(&pid); @@ -1451,34 +1455,11 @@ int virDirCreate(const char *path, mode_t mode, if (!WIFEXITED(status) || (ret == EACCES)) { /* fall back to the simpler method, which works better in * some cases */ - return virDirCreateSimple(path, mode, uid, gid, flags); + return virDirCreateNoFork(path, mode, uid, gid, flags); } if (ret != 0) { goto parenterror; } - - /* check if group was set properly by creating after - * setgid. If not, try doing it with chown */ - if (stat(path, &st) == -1) { - ret = errno; - virReportSystemError(errno, - _("stat of '%s' failed"), - path); - goto parenterror; - } - if ((st.st_gid != gid) && (chown(path, -1, gid) < 0)) { - ret = errno; - virReportSystemError(errno, - _("cannot chown '%s' to group %u"), - path, gid); - goto parenterror; - } - if (chmod(path, mode) < 0) { - virReportSystemError(errno, - _("cannot set mode of '%s' to %04o"), - path, mode); - goto parenterror; - } parenterror: return ret; } @@ -1513,20 +1494,45 @@ parenterror: } goto childerror; } + /* check if group was set properly by creating after + * setgid. If not, try doing it with chown */ + if (stat(path, &st) == -1) { + ret = errno; + virReportSystemError(errno, + _("stat of '%s' failed"), path); + goto childerror; + } + if ((st.st_gid != gid) && (chown(path, -1, gid) < 0)) { + ret = errno; + virReportSystemError(errno, + _("cannot chown '%s' to group %u"), + path, gid); + goto childerror; + } + if ((flags & VIR_DIR_CREATE_FORCE_PERMS) + && chmod(path, mode) < 0) { + virReportSystemError(errno, + _("cannot set mode of '%s' to %04o"), + path, mode); + goto childerror; + } childerror: _exit(ret); } #else /* WIN32 */ -int virFileCreate(const char *path, mode_t mode, - uid_t uid, gid_t gid, unsigned int flags) { - return virFileCreateSimple(path, mode, uid, gid, flags); +int virFileOperation(const char *path, int openflags, mode_t mode, + uid_t uid, gid_t gid, + virFileOperationHook hook, void *hookdata, + unsigned int flags) { + return virFileOperationNoFork(path, openflags, mode, uid, gid, + hook, hookdata, flags); } int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) { - return virDirCreateSimple(path, mode, uid, gid, flags); + return virDirCreateNoFork(path, mode, uid, gid, flags); } #endif diff --git a/src/util/util.h b/src/util/util.h index 8460100..1bc9999 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -110,13 +110,22 @@ char *virFindFileInPath(const char *file); int virFileExists(const char *path); enum { - VIR_FILE_CREATE_NONE = 0, - VIR_FILE_CREATE_AS_UID = (1 << 0), - VIR_FILE_CREATE_ALLOW_EXIST = (1 << 1), + VIR_FILE_OP_NONE = 0, + VIR_FILE_OP_AS_UID = (1 << 0), + VIR_FILE_OP_FORCE_PERMS = (1 << 1), }; +typedef int (*virFileOperationHook)(int fd, void *data); +int virFileOperation(const char *path, int openflags, mode_t mode, + uid_t uid, gid_t gid, + virFileOperationHook hook, void *hookdata, + unsigned int flags) ATTRIBUTE_RETURN_CHECK; -int virFileCreate(const char *path, mode_t mode, uid_t uid, gid_t gid, - unsigned int flags) ATTRIBUTE_RETURN_CHECK; +enum { + VIR_DIR_CREATE_NONE = 0, + VIR_DIR_CREATE_AS_UID = (1 << 0), + VIR_DIR_CREATE_FORCE_PERMS = (1 << 1), + VIR_DIR_CREATE_ALLOW_EXIST = (1 << 2), +}; int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) ATTRIBUTE_RETURN_CHECK; int virFileMakePath(const char *path) ATTRIBUTE_RETURN_CHECK; -- 1.6.6.1