On 01/13/2010 09:32 AM, Serge E. Hallyn wrote:
Quoting Serge E. Hallyn (serue@xxxxxxxxxx):
Quoting Laine Stump (laine@xxxxxxxxx):
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.
That was actually just a duplication of existing functionality from the
code that will now call virFileCreate() (right down to the lack of any
error reporting if you're not running as root and the caller requests
the file to be created with a different uid).
The same check/chown is done in a couple other places in this patch
series, so your comment applies to those as well.
If I were to remove the check of current uid, should a failure of chown
then be reported as an error, or ignored (making it closer to current
behavior for non-root users)? And does this need to be added to the
current patch, or can it be considered a follow-on? (I guess the
question behind that is do people commonly use libvirt in a manner that
would be affected by this, or is this theorizing about something someone
*might* do?)
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list