[libvirt] [PATCH] sanlock: Chown lease files as well

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121231/8d5481c8/attachment-0001.sig>