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

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



Quoting Serge E. Hallyn (serue us ibm com):
> Quoting Laine Stump (laine laine org):
> > 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.)
> > 
> > Return from both of these functions is 0 on success, or the value of
> > errno if there was a failure.
> > ---
> >  src/util/util.c |  247 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/util/util.h |    9 ++
> >  2 files changed, 256 insertions(+), 0 deletions(-)
> > 
> > diff --git a/src/util/util.c b/src/util/util.c
> > index 1d493de..1cb29f4 100644
> > --- a/src/util/util.c
> > +++ b/src/util/util.c
> > @@ -1126,6 +1126,253 @@ int virFileExists(const char *path)
> >      return(0);
> >  }
> > 
> > +
> > +static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t gid) {
> > +    int fd = -1;
> > +    int ret = 0;
> > +
> > +    if ((fd = open(path, O_RDWR | O_CREAT | O_EXCL, mode)) < 0) {
> > +        ret = errno;
> > +        virReportSystemError(NULL, errno, _("failed to create file '%s'"),
> > +                             path);
> > +        goto error;
> > +    }
> > +    if ((getuid() == 0) && ((uid != 0) || (gid != 0))) {
> 
> How about checking for CAP_CHOWN instead of getuid()==0?  Otherwise
> if I try installing this certain ways, virFileCreateSimple() will refuse
> to try the chown even though it could succeed.

I guess for certain netfs's the uid might matter more, so checking for
either condition - or in fact just trying the fchown, then doing a stat,
then making sure st.st_{ug}id are correct after the fact - seems useful.

-serge


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