[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 6/7] 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 mutually exclusive access to the PTYs.

If mutualy exclusive access is not used, two clients may open the same
console, which results in 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 internally 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 in an
        error.
---
Diff to v2:
- moved new files to src/conf/
- tweaked configure messages and output
- renamed functions to shorter and nicer variants
- fixed license and year
- fixed typos and grammar mistakes
- added use of virPidFileReadPathIfAlive to avoid redundant code
- cleaned up other parts to cope with Eric's review
 configure.ac             |   39 ++++-
 po/POTFILES.in           |    1 +
 src/Makefile.am          |    6 +-
 src/conf/virconsole.c    |  396 ++++++++++++++++++++++++++++++++++++++++++++++
 src/conf/virconsole.h    |   36 ++++
 src/libvirt_private.syms |    6 +
 6 files changed, 475 insertions(+), 9 deletions(-)
 create mode 100644 src/conf/virconsole.c
 create mode 100644 src/conf/virconsole.h

diff --git a/configure.ac b/configure.ac
index 9fb7bfc..3254105 100644
--- a/configure.ac
+++ b/configure.ac
@@ -327,6 +327,12 @@ 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)
+                  @<:@default=disabled@:>@]),
+  [],[with_console_lock_files=disabled])

 dnl
 dnl in case someone want to build static binaries
@@ -1197,6 +1203,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 containing UUCP pty lock files])
+fi
+AM_CONDITIONAL([VIR_PTY_LOCK_FILE_PATH], [test "$with_console_lock_files" != "disabled"])
+

 dnl SELinux
 AC_ARG_WITH([selinux],
@@ -2751,14 +2773,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 locks: $with_console_lock_files])
 AC_MSG_NOTICE([])
 AC_MSG_NOTICE([Privileges])
 AC_MSG_NOTICE([])
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 0126320..d2c4e21 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -17,6 +17,7 @@ src/conf/nwfilter_params.c
 src/conf/secret_conf.c
 src/conf/storage_conf.c
 src/conf/storage_encryption_conf.c
+src/conf/virconsole.c
 src/cpu/cpu.c
 src/cpu/cpu_generic.c
 src/cpu/cpu_map.c
diff --git a/src/Makefile.am b/src/Makefile.am
index a44446f..19aca7c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -180,6 +180,9 @@ ENCRYPTION_CONF_SOURCES =					\

 CPU_CONF_SOURCES =						\
 		conf/cpu_conf.c conf/cpu_conf.h
+# Safe console handling helper APIs
+CONSOLE_CONF_SOURCES =						\
+		conf/virconsole.c conf/virconsole.h

 CONF_SOURCES =							\
 		$(NETDEV_CONF_SOURCES)				\
@@ -192,7 +195,8 @@ CONF_SOURCES =							\
 		$(ENCRYPTION_CONF_SOURCES)			\
 		$(INTERFACE_CONF_SOURCES)			\
 		$(SECRET_CONF_SOURCES)				\
-		$(CPU_CONF_SOURCES)
+		$(CPU_CONF_SOURCES)				\
+		$(CONSOLE_CONF_SOURCES)

 # The remote RPC driver, covering domains, storage, networks, etc
 REMOTE_DRIVER_GENERATED = \
diff --git a/src/conf/virconsole.c b/src/conf/virconsole.c
new file mode 100644
index 0000000..f411293
--- /dev/null
+++ b/src/conf/virconsole.c
@@ -0,0 +1,396 @@
+/**
+ * virconsole.c: api to guarantee mutualy exclusive
+ * access to domain's consoles
+ *
+ * Copyright (C) 2011-2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Peter Krempa <pkrempa redhat com>
+ */
+
+#include <config.h>
+
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/types.h>
+
+#include "virconsole.h"
+#include "hash.h"
+#include "fdstream.h"
+#include "internal.h"
+#include "threads.h"
+#include "memory.h"
+#include "virpidfile.h"
+#include "logging.h"
+#include "virterror_internal.h"
+#include "virfile.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+#define virConsoleError(code, ...)                                      \
+    virReportErrorHelper(VIR_FROM_THIS, code, __FILE__,                  \
+                         __FUNCTION__, __LINE__, __VA_ARGS__)
+
+/* structure holding information about consoles
+ * open in a given domain */
+struct _virConsoles {
+    virMutex lock;
+    virHashTablePtr hash;
+};
+
+typedef struct _virConsoleStreamInfo virConsoleStreamInfo;
+typedef virConsoleStreamInfo *virConsoleStreamInfoPtr;
+struct _virConsoleStreamInfo {
+    virConsolesPtr 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 *virConsoleLockFilePath(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/ anywhere in the path*/
+
+    /* substitute path forward slashes for underscores */
+    p = filename;
+    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 virConsoleLockFileCreate(const char *pty)
+{
+    char *path = NULL;
+    int ret = -1;
+    int lockfd = -1;
+    char *pidStr = NULL;
+    int pid;
+    /* build lock file path */
+    if (!(path = virConsoleLockFilePath(pty)))
+        goto cleanup;
+
+
+
+    /* check if a log file and  process holding the lock still exists */
+    if (virPidFileReadPathIfAlive(path, &pid, NULL) == 0 && pid >= 0) {
+        /* the process exists, the lockfile is valid */
+        virConsoleError(VIR_ERR_OPERATION_FAILED,
+                        _("Requested console pty '%s' is locked by "
+                          "lock file '%s' held by process %d"),
+                        pty, path, pid);
+        goto cleanup;
+    } else {
+        /* clean up the stale/corrupted/nonexistent lockfile */
+        unlink(path);
+    }
+    /* lockfile doesn't (shouldn't) exist */
+
+    /* ensure correct format according to filesystem hierarchy standard */
+    /* http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLOCKLOCKFILES */
+    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 virConsoleLockFileRemove(const char *pty)
+{
+    char *path = virConsoleLockFilePath(pty);
+    if (path)
+        unlink(path);
+    VIR_FREE(path);
+}
+#else /* #ifdef VIR_PTY_LOCK_FILE_PATH */
+/* file locking for console devices is disabled */
+static int virConsoleLockFileCreate(const char *pty ATTRIBUTE_UNUSED)
+{
+    return 0;
+}
+
+static void virConsoleLockFileRemove(const char *pty 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 information about the console
+ * @name Path of the pty.
+ */
+static void virConsoleHashEntryFree(void *data,
+                                    const void *name)
+{
+    const char *pty = name;
+    virStreamPtr st = data;
+
+    /* free stream reference */
+    virStreamFree(st);
+
+    /* delete lock file */
+    virConsoleLockFileRemove(pty);
+}
+
+/**
+ * Frees opaque data provided for the stream closing callback
+ *
+ * @opaque Data to be freed.
+ */
+static void virConsoleFDStreamCloseCbFree(void *opaque)
+{
+    virConsoleStreamInfoPtr 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 virConsoleFDStreamCloseCb(virStreamPtr st ATTRIBUTE_UNUSED,
+                                      void *opaque)
+{
+    virConsoleStreamInfoPtr 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
+ */
+virConsolesPtr virConsoleAlloc(void)
+{
+    virConsolesPtr 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, virConsoleHashEntryFree)))
+        goto error;
+
+    if (virMutexInit(&cons->lock) < 0)
+        goto error;
+
+    return cons;
+error:
+    virConsoleFree(cons);
+    return NULL;
+}
+
+/**
+ * Free structures for handling open console streams.
+ *
+ * @cons Pointer to the private structure.
+ */
+void virConsoleFree(virConsolesPtr 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 ensures 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). Returns -1 on
+ * error and 1 if the console stream is open and busy.
+ */
+int virConsoleOpen(virConsolesPtr cons,
+                   const char *pty,
+                   virStreamPtr st,
+                   bool force)
+{
+    virConsoleStreamInfoPtr 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 = virConsoleLockFileCreate(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;
+
+    cbdata->cons = cons;
+    if (!(cbdata->pty = strdup(pty)))
+        goto error;
+
+    /* open the console pty */
+    if (virFDStreamOpenFile(st, pty, 0, 0, O_RDWR) < 0)
+        goto error;
+
+    savedStream = st;
+    st = NULL;
+
+    /* add cleanup callback */
+    virFDStreamSetInternalCloseCb(savedStream,
+                                  virConsoleFDStreamCloseCb,
+                                  cbdata,
+                                  virConsoleFDStreamCloseCbFree);
+    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/conf/virconsole.h b/src/conf/virconsole.h
new file mode 100644
index 0000000..9f7fb04
--- /dev/null
+++ b/src/conf/virconsole.h
@@ -0,0 +1,36 @@
+/**
+ * virconsole.h: api to guarantee mutualy exclusive
+ * access to domain's consoles
+ *
+ * Copyright (C) 2011-2012 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Peter Krempa <pkrempa redhat com>
+ */
+#ifndef __VIR_CONSOLE_H__
+# define __VIR_CONSOLE_H__
+
+# include "internal.h"
+
+typedef struct _virConsoles virConsoles;
+typedef virConsoles *virConsolesPtr;
+
+virConsolesPtr virConsoleAlloc(void);
+void virConsoleFree(virConsolesPtr cons);
+
+int virConsoleOpen(virConsolesPtr cons, const char *pty,
+                   virStreamPtr st, bool force);
+#endif /*__VIR_DOMAIN_CONSOLE_LOCK_H__*/
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 924ec16..ff85ace 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1165,6 +1165,12 @@ virAuditOpen;
 virAuditSend;


+# virconsole.h
+virConsoleAlloc;
+virConsoleFree;
+virConsoleOpen;
+
+
 # virfile.h
 virFileClose;
 virFileDirectFdClose;
-- 
1.7.3.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]