On 12/28/2012 08:59 AM, Michal Privoznik wrote:
> Since sanlock doesn't run under root:root, we have chown()'ed the
> __LIBVIRT__DISKS__ lease file to the user:group defined in the
> sanlock config. However, when writing the patch I've forgot about
> lease files for each disk (this is the
> /var/lib/libvirt/sanlock/<md5>) file.
> ---
> src/locking/lock_driver_sanlock.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
> index 75ced84..c955003 100644
> --- a/src/locking/lock_driver_sanlock.c
> +++ b/src/locking/lock_driver_sanlock.c
> @@ -679,6 +679,17 @@ static int virLockManagerSanlockCreateLease(struct sanlk_resource *res)
> }
> VIR_DEBUG("Someone else just created lockspace %s", res->disks[0].path);
> } else {
> + /* chown() the path to make sure sanlock can access it */
> + if ((driver->user != -1 || driver->group != -1) &&
I'm late in my review, but this needs to be:
(driver->user != (uid_t) -1 || driver->group != (gid_t) -1)
Why? Because POSIX makes no requirements on whether [ug]id_t are signed
or unsigned, nor whether they are smaller, equal, or wider than int.
Therefore, without the cast, it is possible to hit a platform where the
comparison fails to do what you want, if you don't also have the cast in
place (that is, it is possible to have '(uid_t)-1 != -1' evaluate to
true instead of the intended false, due to integer promotion rules).
Thankfully, it's only theoretical at this point (sanlock has only been
ported to Linux, where uid_t is sane), but we might as well make the fix.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list