[libvirt] [PATCH] Add call to sanlock_restrict() in QEMU lock driver

[Daniel Veillard]

On Thu, Jun 02, 2011 at 11:52:57AM +0100, Daniel P. Berrange wrote:
> In between fork and exec, a connection to sanlock is acquired
> and the socket file descriptor is intionally leaked to the
> child process. sanlock watches this FD for POLL_HANGUP to
> detect when QEMU has exited. We don't want a rogus/compromised
> QEMU from issuing sanlock RPC calls on the leaked FD though,
> since that could be used to DOS other guests. By calling
> sanlock_restrict() on the socket before exec() we can lock
> it down.
> 
> * configure.ac: Check for sanlock_restrict API
> * src/locking/domain_lock.c: Restrict lock acquired in
>   process startup phase
> * src/locking/lock_driver.h: Add VIR_LOCK_MANAGER_ACQUIRE_RESTRICT
> * src/locking/lock_driver_sanlock.c: Add call to sanlock_restrict
>   when requested by VIR_LOCK_MANAGER_ACQUIRE_RESTRICT flag
> ---
>  configure.ac                      |    2 +-
>  src/locking/domain_lock.c         |    8 +++++---
>  src/locking/lock_driver.h         |    4 +++-
>  src/locking/lock_driver_sanlock.c |   15 ++++++++++++++-
>  4 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index a1bd64d..25669cf 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -972,7 +972,7 @@ if test "x$with_sanlock" != "xno"; then
>          fail=1
>      fi])
>    if test "x$with_sanlock" != "xno" ; then
> -    AC_CHECK_LIB([sanlock], [sanlock_acquire],[
> +    AC_CHECK_LIB([sanlock], [sanlock_restrict],[
>        SANLOCK_LIBS="$SANLOCK_LIBS -lsanlock"
>        with_sanlock=yes
>      ],[
> diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c
> index 85352e2..f0a11b7 100644
> --- a/src/locking/domain_lock.c
> +++ b/src/locking/domain_lock.c
> @@ -159,10 +159,12 @@ int virDomainLockProcessStart(virLockManagerPluginPtr plugin,
>  {
>      virLockManagerPtr lock = virDomainLockManagerNew(plugin, dom, true);
>      int ret;
> +    int flags = VIR_LOCK_MANAGER_ACQUIRE_RESTRICT;
> +
>      if (paused)
> -        ret = virLockManagerAcquire(lock, NULL, VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY);
> -    else
> -        ret = virLockManagerAcquire(lock, NULL, 0);
> +        flags |= VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY;
> +
> +    ret = virLockManagerAcquire(lock, NULL, flags);
>  
>      virLockManagerFree(lock);
>  
> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
> index 40a55f6..2e71113 100644
> --- a/src/locking/lock_driver.h
> +++ b/src/locking/lock_driver.h
> @@ -59,7 +59,9 @@ typedef enum {
>  
>  typedef enum {
>      /* Don't acquire the resources, just register the object PID */
> -    VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY = (1 << 0)
> +    VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY = (1 << 0),
> +    /* Prevent further lock/unlock calls from this process */
> +    VIR_LOCK_MANAGER_ACQUIRE_RESTRICT = (1 << 1),
>  } virLockManagerAcquireFlags;
>  
>  enum {
> diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
> index 7e0610d..a60d7ce 100644
> --- a/src/locking/lock_driver_sanlock.c
> +++ b/src/locking/lock_driver_sanlock.c
> @@ -240,7 +240,8 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock,
>      int rv;
>      int i;
>  
> -    virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY, -1);
> +    virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_RESTRICT |
> +                  VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY, -1);
>  
>      if (priv->res_count == 0 &&
>          priv->hasRWDisks) {
> @@ -327,6 +328,18 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock,
>          virSetInherit(sock, true) < 0)
>          goto error;
>  
> +    if (flags & VIR_LOCK_MANAGER_ACQUIRE_RESTRICT) {
> +        if ((rv = sanlock_restrict(sock, SANLK_RESTRICT_ALL)) < 0) {
> +            if (rv <= -200)
> +                virLockError(VIR_ERR_INTERNAL_ERROR,
> +                             _("Failed to restrict process: error %d"), rv);
> +            else
> +                virReportSystemError(-rv, "%s",
> +                                     _("Failed to restrict process"));
> +            goto error;
> +        }
> +    }
> +
>      VIR_DEBUG("Acquire completed fd=%d", sock);
>  
>      if (res_free) {

  Qualify as security bug fix, ACK, lease push :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/