[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/4] New utility functions virFileCreate and virDirCreate



On Wed, Jan 20, 2010 at 02:29:41AM -0500, Laine Stump wrote:
> These functions create a new file or directory with the given
> uid/gid. If the flag VIR_FILE_CREATE_AS_UID is given, they do this by
> forking a new process, calling setuid/setgid in the new process, and
> then creating the file. This is better than simply calling open then
> fchown, because in the latter case, a root-squashing nfs server would
> create the new file as user nobody, then refuse to allow fchown.
> 
> If VIR_FILE_CREATE_AS_UID is not specified, the simpler tactic of
> creating the file/dir, then chowning is is used. This gives better
> results in cases where the parent directory isn't on a root-squashing
> NFS server, but doesn't give permission for the specified uid/gid to
> create files. (Note that if the fork/setuid method fails to create the
> file due to access privileges, the parent process will make a second
> attempt using this simpler method.)
> 
> If the bit VIR_FILE_CREATE_ALLOW_EXIST is set in the flags, an
> existing file/directory will not cause an error; in this case, the
> function will simply set the permissions of the file/directory to
> those requested. If VIR_FILE_CREATE_ALLOW_EXIST is not specified, an
> existing file/directory is considered (and reported as) an error.
> 
> Return from both of these functions is 0 on success, or the value of
> errno if there was a failure.
> ---
>  src/util/util.c |  304 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/util.h |   10 ++
>  2 files changed, 314 insertions(+), 0 deletions(-)
> 
> diff --git a/src/util/util.c b/src/util/util.c
> index 578d12b..d0f7fcd 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -1138,6 +1138,310 @@ int virFileExists(const char *path)
>      return(0);
>  }
>  
> +
> +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);
> +    int fd = -1;
> +    int ret = 0;
> +    struct stat st;
> +
> +    if ((fd = open(path, open_flags, mode)) < 0) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno, _("failed to create file '%s'"),
> +                             path);
> +        goto error;
> +    }
> +    if (fstat(fd, &st) == -1) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno, _("stat of '%s' failed"), path);

  misses a close(fd); here

> +        goto error;
> +    }
> +    if (((st.st_uid != uid) || (st.st_gid != gid))
> +        && (fchown(fd, uid, gid) < 0)) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno, _("cannot chown '%s' to (%u, %u)"),
> +                             path, uid, gid);
> +        close(fd);
> +        goto error;
> +    }
> +    if (fchmod(fd, mode) < 0) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno,
> +                             _("cannot set mode of '%s' to %04o"),
> +                             path, mode);
> +        close(fd);
> +        goto error;
> +    }
> +    if (close(fd) < 0) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno, _("failed to close new file '%s'"),
> +                             path);
> +        goto error;
> +    }
> +    fd = -1;
> +error:
> +    return ret;
> +}
[...]
> +#ifndef WIN32
> +int virFileCreate(const char *path, mode_t mode,
> +                  uid_t uid, gid_t gid, unsigned int flags) {
> +    struct stat st;
> +    pid_t pid;
> +    int waitret, status, ret = 0;
> +    int fd = -1;
> +
> +    if ((!(flags & VIR_FILE_CREATE_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);
> +    }
> +
> +    /* parent is running as root, but caller requested that the
> +     * file be created as some other user and/or group). The
> +     * following dance avoids problems caused by root-squashing
> +     * NFS servers. */
> +
> +    if ((pid = fork()) < 0) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno,
> +                             _("cannot fork o create file '%s'"), path);
> +        return ret;
> +    }
> +
> +    if (pid) { /* parent */
> +        /* wait for child to complete, and retrieve its exit code */
> +        while ((waitret = waitpid(pid, &status, 0) == -1)
> +               && (errno == EINTR));
> +        if (waitret == -1) {
> +            ret = errno;
> +            virReportSystemError(NULL, errno,
> +                                 _("failed to wait for child creating '%s'"),
> +                                 path);
> +            goto parenterror;
> +        }
> +        ret = WEXITSTATUS(status);
> +        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(NULL, errno,
> +                                 _("stat of '%s' failed"),
> +                                 path);
> +            goto parenterror;
> +        }
> +        if ((st.st_gid != gid) && (chown(path, -1, gid) < 0)) {
> +            ret = errno;
> +            virReportSystemError(NULL, errno,
> +                                 _("cannot chown '%s' to group %u"),
> +                                 path, gid);
> +            goto parenterror;
> +        }
> +        if (chmod(path, mode) < 0) {
> +            ret = errno;
> +            virReportSystemError(NULL, errno,
> +                                 _("cannot set mode of '%s' to %04o"),
> +                                 path, mode);
> +            goto parenterror;
> +        }
> +parenterror:
> +        return ret;
> +    }
> +
> +    /* child - set desired uid/gid, then attempt to create the file */
> +
> +    if ((gid != 0) && (setgid(gid) != 0)) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno,
> +                             _("cannot set gid %u creating '%s'"),
> +                             gid, path);
> +        goto childerror;
> +    }
> +    if  ((uid != 0) && (setuid(uid) != 0)) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno,
> +                             _("cannot set uid %u creating '%s'"),
> +                             uid, path);
> +        goto childerror;
> +    }
> +    if ((fd = open(path, O_RDWR | O_CREAT | O_EXCL, mode)) < 0) {
> +        ret = errno;
> +        if (ret != EACCES) {
> +            /* in case of EACCES, the parent will retry */
> +            virReportSystemError(NULL, errno,
> +                                 _("child failed to create file '%s'"),
> +                                 path);
> +        }
> +        goto childerror;
> +    }
> +    if (close(fd) < 0) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno, _("child failed to close new file '%s'"),
> +                             path);
> +        goto childerror;
> +    }
> +childerror:
> +    _exit(ret);
> +
> +}
> +
> +int virDirCreate(const char *path, mode_t mode,
> +                 uid_t uid, gid_t gid, unsigned int flags) {
> +    struct stat st;
> +    pid_t pid;
> +    int waitret;
> +    int status, ret = 0;
> +
> +    if ((!(flags & VIR_FILE_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);
> +    }
> +
> +    if ((pid = fork()) < 0) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno,
> +                             _("cannot fork to create directory '%s'"),
> +                             path);
> +        return ret;
> +    }
> +
> +    if (pid) { /* parent */
> +        /* wait for child to complete, and retrieve its exit code */
> +        while ((waitret = waitpid(pid, &status, 0) == -1)  && (errno == EINTR));
> +        if (waitret == -1) {
> +            ret = errno;
> +            virReportSystemError(NULL, errno,
> +                                 _("failed to wait for child creating '%s'"),
> +                                 path);
> +            goto parenterror;
> +        }
> +        ret = WEXITSTATUS(status);
> +        if (!WIFEXITED(status) || (ret == EACCES)) {
> +            /* fall back to the simpler method, which works better in
> +             * some cases */
> +            return virDirCreateSimple(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(NULL, errno,
> +                                 _("stat of '%s' failed"),
> +                                 path);
> +            goto parenterror;
> +        }
> +        if ((st.st_gid != gid) && (chown(path, -1, gid) < 0)) {
> +            ret = errno;
> +            virReportSystemError(NULL, errno,
> +                                 _("cannot chown '%s' to group %u"),
> +                                 path, gid);
> +            goto parenterror;
> +        }
> +        if (chmod(path, mode) < 0) {
> +            virReportSystemError(NULL, errno,
> +                                 _("cannot set mode of '%s' to %04o"),
> +                                 path, mode);
> +            goto parenterror;
> +        }
> +parenterror:
> +        return ret;
> +    }
> +
> +    /* child - set desired uid/gid, then attempt to create the directory */
> +
> +    if ((gid != 0) && (setgid(gid) != 0)) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno, _("cannot set gid %u creating '%s'"),
> +                             gid, path);
> +        goto childerror;
> +    }
> +    if  ((uid != 0) && (setuid(uid) != 0)) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno, _("cannot set uid %u creating '%s'"),
> +                             uid, path);
> +        goto childerror;
> +    }
> +    if (mkdir(path, mode) < 0) {
> +        ret = errno;
> +        if (ret != EACCES) {
> +            /* in case of EACCES, the parent will retry */
> +            virReportSystemError(NULL, errno, _("child failed to create directory '%s'"),
> +                                 path);
> +        }
> +        goto childerror;
> +    }
> +childerror:
> +    _exit(ret);
> +}

  This is fairly tortuous, but apparently we have no choice ...

> +#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 virDirCreate(const char *path, mode_t mode,
> +                 uid_t uid, gid_t gid, unsigned int flags) {
> +    return virDirCreateSimple(path, mode, uid, gid, flags);
> +}
> +#endif
> +
>  int virFileMakePath(const char *path)
>  {
>      struct stat st;
> diff --git a/src/util/util.h b/src/util/util.h
> index 5e70038..b0219cc 100644
> --- a/src/util/util.h
> +++ b/src/util/util.h
> @@ -111,6 +111,16 @@ 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),
> +};
> +
> +int virFileCreate(const char *path, mode_t mode, uid_t uid, gid_t gid,
> +                  unsigned int flags) ATTRIBUTE_RETURN_CHECK;
> +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;
>  
>  int virFileBuildPath(const char *dir,

  ACK, but there is the small fd leak to plug

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]