[libvirt] [PATCH v2 5/6] util: Add helpers for safe domain console operations

This patch adds a set of functions used in creating console streams for domains using PTYs and ensures mutualy exculsive access to the PTYs. If mutualy exclusive access is not used, two clients may open the same console, which results into corruption on both clients as both of them race to read data from the PTY. Two approaches are used to ensure this: 1) Internal data structure holding open PTYs. This is used internaly and enables the user to forcibly terminate another console connection eg. when somebody leaves the console open on another host. 2) UUCP style lock files: This uses UUCP lock files according to the FHS ( http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLOCKLOCKFILES ) to check if other programs (like minicom) are not using the pty device of the console. This feature is disabled by default and may be enabled using configure parameter --with-console-lock-files=/path/to/lock/file/directory or --with-console-lock-files=auto (which tries to infer the location from OS used (currently only linux). On usual linux systems, normal users may not write to the /var/lock directory containing the locks. This poses problems while in session mode. If the current user has no access to the lockfile directory, check for presence of the file is still done, but no lock file is created. This does NOT result into an error. * configure.ac - add option to enable UUCP style PTY file locks * src/Makefile.am - add new files to be built with util module * src/libvirt_private.syms - add private symbol definition * src/util/domain_safe_console.c - implementation of safe console handling * src/util/domain_safe_console.h - header files --- configure.ac | 37 +++- src/Makefile.am | 5 +- src/libvirt_private.syms | 6 + src/util/domain_safe_console.c | 399 ++++++++++++++++++++++++++++++++++++++++ src/util/domain_safe_console.h | 28 +++ 5 files changed, 466 insertions(+), 9 deletions(-) create mode 100644 src/util/domain_safe_console.c create mode 100644 src/util/domain_safe_console.h diff --git a/configure.ac b/configure.ac index 77e5cc9..d427a9e 100644 --- a/configure.ac +++ b/configure.ac @@ -329,6 +329,10 @@ AC_ARG_WITH([remote], AC_HELP_STRING([--with-remote], [add remote driver support @<:@default=yes@:>@]),[],[with_remote=yes]) AC_ARG_WITH([libvirtd], AC_HELP_STRING([--with-libvirtd], [add libvirtd support @<:@default=yes@:>@]),[],[with_libvirtd=yes]) +AC_ARG_WITH([console-lock-files], + AC_HELP_STRING([--with-console-lock-files], + [location for UUCP style lock files for console PTYs (use auto for default paths on some platforms)@<:@defult=disabled@:>@]), + [],[with_console_lock_files=disabled]) dnl dnl in case someone want to build static binaries @@ -1198,6 +1202,22 @@ AM_CONDITIONAL([HAVE_AUDIT], [test "$with_audit" = "yes"]) AC_SUBST([AUDIT_CFLAGS]) AC_SUBST([AUDIT_LIBS]) +dnl UUCP style file locks for PTY consoles +if test "$with_console_lock_files" != "disabled"; then + if test "$with_console_lock_files" = "auto"; then + dnl Default locations for platforms + if test "$with_linux" = "yes"; then + with_console_lock_files=/var/lock + fi + fi + if test "$with_console_lock_files" = "auto"; then + AC_MSG_ERROR([You must specify path for the lock files on this platform]) + fi + AC_DEFINE_UNQUOTED([VIR_PTY_LOCK_FILE_PATH], "$with_console_lock_files", + [path to directory containig UUCP pty lock files ]) +fi +AM_CONDITIONAL([VIR_PTY_LOCK_FILE_PATH], [test "$with_console_lock_files" != "disabled"]) + dnl SELinux AC_ARG_WITH([selinux], @@ -2726,14 +2746,15 @@ AC_MSG_NOTICE([ Alloc OOM: $enable_oom]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Miscellaneous]) AC_MSG_NOTICE([]) -AC_MSG_NOTICE([ Debug: $enable_debug]) -AC_MSG_NOTICE([ Warnings: $enable_compile_warnings]) -AC_MSG_NOTICE([Warning Flags: $WARN_CFLAGS]) -AC_MSG_NOTICE([ Readline: $lv_use_readline]) -AC_MSG_NOTICE([ Python: $with_python]) -AC_MSG_NOTICE([ DTrace: $with_dtrace]) -AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) -AC_MSG_NOTICE([ Init script: $with_init_script]) +AC_MSG_NOTICE([ Debug: $enable_debug]) +AC_MSG_NOTICE([ Warnings: $enable_compile_warnings]) +AC_MSG_NOTICE([ Warning Flags: $WARN_CFLAGS]) +AC_MSG_NOTICE([ Readline: $lv_use_readline]) +AC_MSG_NOTICE([ Python: $with_python]) +AC_MSG_NOTICE([ DTrace: $with_dtrace]) +AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) +AC_MSG_NOTICE([ Init script: $with_init_script]) +AC_MSG_NOTICE([Console PTY lock path: $with_console_lock_files]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Privileges]) AC_MSG_NOTICE([]) diff --git a/src/Makefile.am b/src/Makefile.am index 93bf54c..3b04096 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -101,6 +101,9 @@ UTIL_SOURCES = \ util/virsocketaddr.h util/virsocketaddr.c \ util/virtime.h util/virtime.c +SAFE_CONSOLE_SOURCES = \ + util/domain_safe_console.c util/domain_safe_console.h + EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeycode-mapgen.py @@ -555,7 +558,7 @@ noinst_LTLIBRARIES = libvirt_util.la libvirt_la_LIBADD = $(libvirt_la_BUILT_LIBADD) libvirt_la_BUILT_LIBADD = libvirt_util.la libvirt_util_la_SOURCES = \ - $(UTIL_SOURCES) + $(UTIL_SOURCES) $(SAFE_CONSOLE_SOURCES) libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ $(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 99a1099..082248d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -537,6 +537,12 @@ virDomainConfNWFilterTeardown; virDomainConfVMNWFilterTeardown; +# domain_safe_console.h +virDomainSafeConsoleAlloc; +virDomainSafeConsoleFree; +virDomainSafeConsoleOpen; + + # ebtables.h ebtablesAddForwardAllowIn; ebtablesAddForwardPolicyReject; diff --git a/src/util/domain_safe_console.c b/src/util/domain_safe_console.c new file mode 100644 index 0000000..4d5639d --- /dev/null +++ b/src/util/domain_safe_console.c @@ -0,0 +1,399 @@ +/** + * domain_safe_console.c: api to guarantee mutualy exclusive + * access to domain's consoles + * + * Copyright (C) 2011 Red Hat, Inc. + * + * See COPYING.LIB for the License of this software + * + * Author: Peter Krempa <pkrempa(a)redhat.com&gt; + */ + +#include <config.h> + +#include <fcntl.h> +#include <unistd.h> +#include <signal.h> +#include <sys/types.h> + +#include "domain_safe_console.h" +#include "hash.h" +#include "fdstream.h" +#include "internal.h" +#include "threads.h" +#include "memory.h" +#include "logging.h" +#include "virterror_internal.h" +#include "virfile.h" + +#define VIR_FROM_THIS VIR_FROM_NONE +# define consoleReportError(code, ...) \ + virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + +/* structure holding information about consoles + * open in a given domain */ +struct _virDomainSafeConsoles { + virMutex lock; + virHashTablePtr hash; +}; + +typedef struct _virDomainSafeConsoleStreamInfo virDomainSafeConsoleStreamInfo; +typedef virDomainSafeConsoleStreamInfo *virDomainSafeConsoleStreamInfoPtr; +struct _virDomainSafeConsoleStreamInfo { + virDomainSafeConsolesPtr cons; + const char *pty; +}; + +#ifdef VIR_PTY_LOCK_FILE_PATH +/** + * Create a full filename with path to the lock file based on + * name/path of corresponding pty + * + * @pty path of the console device + * + * Returns a modified name, that the caller has to free or NULL + * on error. + */ +static char *virDomainSafeConsoleLockFilePath(const char *pty) +{ + char *path = NULL; + char *sanitizedPath = NULL; + char *ptyCopy; + char *filename; + char *p; + + if (!(ptyCopy = strdup(pty))) { + virReportOOMError(); + goto cleanup; + } + + if ((filename = strstr(ptyCopy, "/dev/"))) { + filename += 5; /* skip /dev/ */ + p = filename; + /* substitute path forward slashes for underscores */ + while (*p) { + if (*p == '/') + *p = '_'; + ++p; + } + } + + if (virAsprintf(&path, "%s/LCK..%s", VIR_PTY_LOCK_FILE_PATH, filename) < 0) + goto cleanup; + + sanitizedPath = virFileSanitizePath(path); + +cleanup: + VIR_FREE(path); + VIR_FREE(ptyCopy); + + return sanitizedPath; +} + +/** + * Verify and create a lock file for a console pty + * + * @pty Path of the console device + * + * Returns 0 on success, -1 on error + */ +static int virDomainSafeConsoleLockFileCreate(const char *pty) +{ + char *path = NULL; + int ret = -1; + int lockfd = -1; + char *pidStr = NULL; + int pid; + ssize_t read_n; + /* build lock file path */ + if (!(path = virDomainSafeConsoleLockFilePath(pty))) + goto cleanup; + + /* check if lock file exists */ + if ((lockfd = open(path, O_RDONLY)) >= 0) { + if ((read_n = virFileReadLimFD(lockfd, 100, &pidStr) > 0) && + (sscanf(pidStr, "%d", &pid) == 1)) { + /* lock file is valid, let's check if the pid is alive */ + if (pid > 1 && kill((pid_t) pid, 0) == 0) { + /* lock file is valid and a process with pid corresponding + * to the pid stored in the lock file exists */ + consoleReportError(VIR_ERR_OPERATION_FAILED, + _("Requested console pty '%s' is locked by " + "lock file '%s' held by process %d"), + pty, path, pid); + goto cleanup; + } + /* lock file is stale */ + VIR_DEBUG("Cleaning up stale lockfile '%s'", path); + VIR_FORCE_CLOSE(lockfd); + unlink(path); + } else { + /* lock file is corrupted */ + VIR_DEBUG("Cleaning up corrupted lockfile '%s'", path); + VIR_FORCE_CLOSE(lockfd); + unlink(path); + /* this might actualy fail, but we get an error while + * creating a new lockfile. Let the user handle the mess. */ + } + VIR_FREE(pidStr); + } + /* lockfile doesn't (shouldn't) exist */ + + /* ensure correct format according to filesystem hierarchy standard */ + if (virAsprintf(&pidStr, "%10d\n", (int) getpid()) < 0) + goto cleanup; + + /* create the lock file */ + if ((lockfd = open(path, O_WRONLY | O_CREAT | O_EXCL, 00644)) < 0) { + /* If we run in session mode, we might have no access to the lock + * file directory. We have to check for an permission denied error + * and see if we can reach it. This should cause an error only if + * we run in daemon mode and thus privileged. + */ + if (errno == EACCES && geteuid() != 0) { + VIR_DEBUG("Skipping lock file creation for pty '%s in path '%s'.", + pty, path); + ret = 0; + goto cleanup; + } + virReportSystemError(errno, + _("Couldn't create lock file for " + "pty '%s' in path '%s'"), + pty, path); + goto cleanup; + } + + /* write the pid to the file */ + if (safewrite(lockfd, pidStr, strlen(pidStr)) < 0) { + virReportSystemError(errno, + _("Couldn't write to lock file for " + "pty '%s' in path '%s'"), + pty, path); + VIR_FORCE_CLOSE(lockfd); + unlink(path); + goto cleanup; + } + + /* we hold the lock */ + ret = 0; + +cleanup: + VIR_FORCE_CLOSE(lockfd); + VIR_FREE(path); + VIR_FREE(pidStr); + + return ret; +} + +/** + * Remove a lock file for a pty + * + * @pty Path of the pty device + */ +static void virDomainSafeConsoleLockFileRemove(const char *pty) +{ + char *path = virDomainSafeConsoleLockFilePath(pty); + if (path) + unlink(path); + VIR_FREE(path); +} +#else /* #ifdef VIR_PTY_LOCK_FILE_PATH */ +/* file locking for console devices is disabled */ +static int virDomainSafeConsoleLockFileCreate(const char *pty ATTRIBUTE_UNUSED) +{ + return 0; +} + +static void virDomainSafeConsoleLockFileRemove(const char *py ATTRIBUTE_UNUSED) +{ + return; +} +#endif /* #ifdef VIR_PTY_LOCK_FILE_PATH */ + +/** + * Frees an entry from the hash containing domain's active consoles + * + * @data Opaque data, struct holding informations about the console + * @name Path of the pty. + */ +static void virDomainSafeConsoleHashEntryFree(void *data, + const void *name) +{ + const char *pty = name; + virStreamPtr st = data; + + /* free stream reference */ + virStreamFree(st); + + /* delete lock file */ + virDomainSafeConsoleLockFileRemove(pty); +} + +/** + * Frees opaque data provided for the stream closing callback + * + * @opaque Data to be freed. + */ +static void virDomainSafeConsoleFDStreamCloseCbFree(void *opaque) +{ + virDomainSafeConsoleStreamInfoPtr priv = opaque; + + VIR_FREE(priv->pty); + VIR_FREE(priv); +} + +/** + * Callback being called if a FDstream is closed. Frees console entries + * from data structures and removes lockfiles. + * + * @st Pointer to stream being closed. + * @opaque Domain's console information structure. + */ +static void virDomainSafeConsoleFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED, + void *opaque) +{ + virDomainSafeConsoleStreamInfoPtr priv = opaque; + virMutexLock(&priv->cons->lock); + + /* remove entry from hash */ + virHashRemoveEntry(priv->cons->hash, priv->pty); + + virMutexUnlock(&priv->cons->lock); +} + +/** + * Allocate structures for storing information about active console streams + * in domain's private data section. + * + * Returns pointer to the allocated structure or NULL on error + */ +virDomainSafeConsolesPtr virDomainSafeConsoleAlloc(void) +{ + virDomainSafeConsolesPtr cons; + if (VIR_ALLOC(cons) < 0) + return NULL; + + /* there will hardly be any consoles most of the time, the hash + * does not have to be huge */ + if (!(cons->hash = virHashCreate(3, virDomainSafeConsoleHashEntryFree))) + goto error; + + if (virMutexInit(&cons->lock) < 0) + goto error; + + return cons; +error: + virDomainSafeConsoleFree(cons); + return NULL; +} + +/** + * Free structures for handling open console streams. + * + * @cons Pointer to the private structure. + */ +void virDomainSafeConsoleFree(virDomainSafeConsolesPtr cons) +{ + if (!cons || !cons->hash) + return; + + virMutexLock(&cons->lock); + virHashFree(cons->hash); + cons->hash = NULL; + virMutexUnlock(&cons->lock); + virMutexDestroy(&cons->lock); + + VIR_FREE(cons); +} + +/** + * Open a console stream for a domain ensuring that other streams are + * not using the console, nor any lockfiles exist. This ensuers that + * the console stream does not get corrupted due to a race on reading + * same FD by two processes. + * + * @cons Pointer to private structure holding data about console streams. + * @pty Path to the pseudo tty to be opened. + * @st Stream the client wishes to use for the console connection. + * @force On true, close active console streams for the selected console pty + * before opening this connection. + * + * Returns 0 on success and st is connected to the selected pty and + * corresponding lock file is created (if configured with). Returns -1 on + * error and 1 if the console stream is open and busy. + */ +int virDomainSafeConsoleOpen(virDomainSafeConsolesPtr cons, + const char *pty, + virStreamPtr st, + bool force) +{ + virDomainSafeConsoleStreamInfoPtr cbdata = NULL; + virStreamPtr savedStream; + int ret; + + virMutexLock(&cons->lock); + + if ((savedStream = virHashLookup(cons->hash, pty))) { + if (!force) { + /* entry found, console is busy */ + virMutexUnlock(&cons->lock); + return 1; + } else { + /* terminate existing connection */ + virFDStreamSetInternalCloseCb(savedStream, NULL, NULL, NULL); + virStreamAbort(savedStream); + virHashRemoveEntry(cons->hash, pty); + /* continue adding a new stream connection */ + } + } + + /* create the lock file */ + if ((ret = virDomainSafeConsoleLockFileCreate(pty)) < 0) { + virMutexUnlock(&cons->lock); + return ret; + } + + /* obtain a reference to the stream */ + if (virStreamRef(st) < 0) { + virMutexUnlock(&cons->lock); + return -1; + } + + if (VIR_ALLOC(cbdata) < 0) + goto error; + + if (virHashAddEntry(cons->hash, pty, st) < 0) + goto error; + + savedStream = st; + st = NULL; + + cbdata->cons = cons; + if (!(cbdata->pty = strdup(pty))) + goto error; + + /* open the console pty */ + if (virFDStreamOpenFile(savedStream, pty, 0, 0, O_RDWR) < 0) + goto error; + + + /* add cleanup callback */ + virFDStreamSetInternalCloseCb(savedStream, + virDomainSafeConsoleFDStreamCloseCb, + cbdata, + virDomainSafeConsoleFDStreamCloseCbFree); + cbdata = NULL; + + virMutexUnlock(&cons->lock); + return 0; + +error: + virStreamFree(st); + virHashRemoveEntry(cons->hash, pty); + if (cbdata) + VIR_FREE(cbdata->pty); + VIR_FREE(cbdata); + virMutexUnlock(&cons->lock); + return -1; +} diff --git a/src/util/domain_safe_console.h b/src/util/domain_safe_console.h new file mode 100644 index 0000000..39536f2 --- /dev/null +++ b/src/util/domain_safe_console.h @@ -0,0 +1,28 @@ +/** + * domain_console_lock.h: api to guarantee mutualy exclusive + * access to domain consoles + * + * Copyright (C) 2011 Red Hat, Inc. + * + * See COPYING.LIB for the License of this software + * + * Author: Peter Krempa <pkrempa(a)redhat.com&gt; + */ + +#ifndef __VIR_DOMAIN_CONSOLE_LOCK_H__ +# define __VIR_DOMAIN_CONSOLE_LOCK_H__ + +# include "internal.h" + +typedef struct _virDomainSafeConsoles virDomainSafeConsoles; +typedef virDomainSafeConsoles *virDomainSafeConsolesPtr; + + +virDomainSafeConsolesPtr virDomainSafeConsoleAlloc(void); +void virDomainSafeConsoleFree(virDomainSafeConsolesPtr cons); + +int virDomainSafeConsoleOpen(virDomainSafeConsolesPtr cons, + const char *pty, + virStreamPtr st, + bool force); +#endif /*__VIR_DOMAIN_CONSOLE_LOCK_H__*/ -- 1.7.3.4