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

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) {