External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-23 06:05:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

maint: avoid potential promotion issues with [ug]id_t

Browse Source 
POSIX does not guarantee whether uid_t and gid_t are signed or
unsigned, nor does it guarantee whether they are smaller, same
size, or larger than int (or even the same size as one another).
Therefore, it is possible to have platforms where '(uid_t)-1==-1'
is false or where 'uid = gid = -1' sets uid to the wrong value,
thanks to integer promotion rules.  The only portable way to use
the placeholder value of these two types is to always use a cast.
Thankfully, the issue is mostly theoretical - sanlock only
compiles on Linux for now, and on Linux, these types do not
suffer from strange promotion problems.

* src/locking/lock_driver_sanlock.c
(virLockManagerSanlockSetupLockspace, virLockManagerSanlockInit)
(virLockManagerSanlockCreateLease): Cast -1 to proper type before
comparing with uid_t or gid_t.
This commit is contained in:
Eric Blake 2013-01-07 15:50:41 -07:00
parent 3aa34af7a6
commit 798ff66790
1 changed files with 9 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/locking/lock_driver_sanlock.c
View File

@ -1,7 +1,7 @@
/* /*
* lock_driver_sanlock.c: A lock driver for Sanlock * lock_driver_sanlock.c: A lock driver for Sanlock
* *
* Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2010-2013 Red Hat, Inc.
* *
* This library is free software; you can redistribute it and/or * This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public * modify it under the terms of the GNU Lesser General Public
@ -236,7 +236,7 @@ static int virLockManagerSanlockSetupLockspace(void)
goto error; goto error;
} }
if (driver->group != -1) if (driver->group != (gid_t) -1)
perms |= 0060; perms |= 0060;
if ((fd = open(path, O_WRONLY|O_CREAT|O_EXCL, perms)) < 0) { if ((fd = open(path, O_WRONLY|O_CREAT|O_EXCL, perms)) < 0) {
@ -249,7 +249,7 @@ static int virLockManagerSanlockSetupLockspace(void)
VIR_DEBUG("Someone else just created lockspace %s", path); VIR_DEBUG("Someone else just created lockspace %s", path);
} else { } else {
/* chown() the path to make sure sanlock can access it */ /* chown() the path to make sure sanlock can access it */
if ((driver->user != -1 || driver->group != -1) && if ((driver->user != (uid_t) -1 || driver->group != (gid_t) -1) &&
(fchown(fd, driver->user, driver->group) < 0)) { (fchown(fd, driver->user, driver->group) < 0)) {
virReportSystemError(errno, virReportSystemError(errno,
_("cannot chown '%s' to (%u, %u)"), _("cannot chown '%s' to (%u, %u)"),
@ -303,8 +303,8 @@ static int virLockManagerSanlockSetupLockspace(void)
} }
} else if (S_ISREG(st.st_mode)) { } else if (S_ISREG(st.st_mode)) {
/* okay, the lease file exists. Check the permissions */ /* okay, the lease file exists. Check the permissions */
if (((driver->user != -1 && driver->user != st.st_uid) || if (((driver->user != (uid_t) -1 && driver->user != st.st_uid) ||
(driver->group != -1 && driver->group != st.st_gid)) && (driver->group != (gid_t) -1 && driver->group != st.st_gid)) &&
(chown(path, driver->user, driver->group) < 0)) { (chown(path, driver->user, driver->group) < 0)) {
virReportSystemError(errno, virReportSystemError(errno,
_("cannot chown '%s' to (%u, %u)"), _("cannot chown '%s' to (%u, %u)"),
@ -314,7 +314,7 @@ static int virLockManagerSanlockSetupLockspace(void)
goto error; goto error;
} }
if ((driver->group != -1 && (st.st_mode & 0060) != 0060) && if ((driver->group != (gid_t) -1 && (st.st_mode & 0060) != 0060) &&
chmod(path, 0660) < 0) { chmod(path, 0660) < 0) {
virReportSystemError(errno, virReportSystemError(errno,
_("cannot chmod '%s' to 0660"), _("cannot chmod '%s' to 0660"),
@ -397,7 +397,8 @@ static int virLockManagerSanlockInit(unsigned int version,
driver->requireLeaseForDisks = true; driver->requireLeaseForDisks = true;
driver->hostID = 0; driver->hostID = 0;
driver->autoDiskLease = false; driver->autoDiskLease = false;
driver->user = driver->group = -1; driver->user = (uid_t) -1;
driver->group = (gid_t) -1;
if (!(driver->autoDiskLeasePath = strdup(LOCALSTATEDIR "/lib/libvirt/sanlock"))) { if (!(driver->autoDiskLeasePath = strdup(LOCALSTATEDIR "/lib/libvirt/sanlock"))) {
VIR_FREE(driver); VIR_FREE(driver);
virReportOOMError(); virReportOOMError();
@ -680,7 +681,7 @@ static int virLockManagerSanlockCreateLease(struct sanlk_resource *res)
VIR_DEBUG("Someone else just created lockspace %s", res->disks[0].path); VIR_DEBUG("Someone else just created lockspace %s", res->disks[0].path);
} else { } else {
/* chown() the path to make sure sanlock can access it */ /* chown() the path to make sure sanlock can access it */
if ((driver->user != -1 || driver->group != -1) && if ((driver->user != (uid_t) -1 || driver->group != (gid_t) -1) &&
(fchown(fd, driver->user, driver->group) < 0)) { (fchown(fd, driver->user, driver->group) < 0)) {
virReportSystemError(errno, virReportSystemError(errno,
_("cannot chown '%s' to (%u, %u)"), _("cannot chown '%s' to (%u, %u)"),