[libvirt] [PATCH 1/1] lxc: use our own hand-rolled code in place of unlockpt and grantpt (v2)

[Serge E. Hallyn]

New version, compile-tested only tonight.  I followed the suggestion
about using posix_openpt(), though its manpage worries me - does libvirt
need to compile on any platforms that don't have that fn?  (In which case
we can add the trivial define if we need to, but...)

Subject: [PATCH 1/1] lxc: use our own hand-rolled code in place of unlockpt
 and grantpt

The glibc ones cannot handle ptys opened in a devpts not mounted at
/dev/pts.

Changelog:
  Oct 17: Per Eli Qiao, use VIR_ALLOC_N when appropriate, make
          sure to check return values, and follow coding style
	  convention.
	  Change lxcGetTtyGid() to return -1 on error, otherwise
	  return gid in passed-in argument.
  Oct 18: Per Eric Blake, consolidate virFileOpenTtyAt() into
          virFileOpenTty(), and use posix_openpt().
	  Per Eric Blake, don't set gid on opened pty since we
	  ask the kernel to do so with 'gid=5' mount option.

Signed-off-by: Serge Hallyn <serge.hallyn at canonical.com>
---
 src/lxc/lxc_controller.c |   67 +++++++++++++++++++++++++++++++++++++++++++---
 src/util/util.c          |   24 +----------------
 2 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 89ce7f5..464bfe8 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -41,6 +41,8 @@
 #include <locale.h>
 #include <linux/loop.h>
 #include <dirent.h>
+#include <grp.h>
+#include <sys/stat.h>
 
 #if HAVE_CAPNG
 # include <cap-ng.h>
@@ -780,6 +782,62 @@ static int lxcSetPersonality(virDomainDefPtr def)
 # define MS_SLAVE              (1<<19)
 #endif
 
+#define ACCESSPERMS (S_IRWXU|S_IRWXG|S_IRWXO) /* 0777 */
+
+/* heavily borrowed from glibc, but don't assume devpts == "/dev/pts" */
+static int
+lxcCreateTty(char *ptmx, int *ttymaster, char **ttyName)
+{
+    int rc = -1;
+    int ret;
+    int ptyno;
+    uid_t uid;
+    struct stat st;
+    int unlock = 0;
+
+    if ((*ttymaster = open(ptmx, O_RDWR|O_NOCTTY|O_NONBLOCK)) < 0)
+        goto cleanup;
+
+    if (ioctl(*ttymaster, TIOCSPTLCK, &unlock) < 0)
+        goto cleanup;
+
+    if (ioctl(*ttymaster, TIOCGPTN, &ptyno) < 0)
+        goto cleanup;
+
+    if (fstat(*ttymaster, &st) < 0)
+        goto cleanup;
+
+    uid = getuid();
+    if (st.st_uid != uid)
+        if (fchown(*ttymaster, uid, -1) < 0)
+            goto cleanup;
+
+    if ((st.st_mode & ACCESSPERMS) != (S_IRUSR|S_IWUSR|S_IWGRP))
+        if (fchmod(*ttymaster, S_IRUSR|S_IWUSR|S_IWGRP) < 0)
+            goto cleanup;
+
+    /*
+     * ptyno shouldn't currently be anything other than 0, but let's
+     * play it safe
+     */
+    if (virAsprintf(ttyName, "/dev/pts/%d", ptyno) < 0) {
+        virReportOOMError();
+        errno = ENOMEM;
+        goto cleanup;
+    }
+
+
+    rc = 0;
+
+cleanup:
+    if (rc != 0) {
+        VIR_FORCE_CLOSE(*ttymaster);
+        VIR_FREE(*ttyName)
+    }
+
+    return rc;
+}
+
 static int
 lxcControllerRun(virDomainDefPtr def,
                  unsigned int nveths,
@@ -877,6 +935,10 @@ lxcControllerRun(virDomainDefPtr def,
             goto cleanup;
         }
 
+	/*
+	 * todo - should we support gid=X for X!=5 for distros which
+	 * use a different gid for tty?
+	 */
         VIR_DEBUG("Mounting 'devpts' on %s", devpts);
         if (mount("devpts", devpts, "devpts", 0,
                   "newinstance,ptmxmode=0666,mode=0620,gid=5") < 0) {
@@ -894,10 +956,7 @@ lxcControllerRun(virDomainDefPtr def,
 
     if (devptmx) {
         VIR_DEBUG("Opening tty on private %s", devptmx);
-        if (virFileOpenTtyAt(devptmx,
-                             &containerPty,
-                             &containerPtyPath,
-                             0) < 0) {
+        if (lxcCreateTty(devptmx, &containerPty, &containerPtyPath) < 0) {
             virReportSystemError(errno, "%s",
                                  _("Failed to allocate tty"));
             goto cleanup;
diff --git a/src/util/util.c b/src/util/util.c
index dac616b..1ec36f1 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1104,21 +1104,9 @@ int virFileOpenTty(int *ttymaster,
                    char **ttyName,
                    int rawmode)
 {
-    return virFileOpenTtyAt("/dev/ptmx",
-                            ttymaster,
-                            ttyName,
-                            rawmode);
-}
-
-#ifdef __linux__
-int virFileOpenTtyAt(const char *ptmx,
-                     int *ttymaster,
-                     char **ttyName,
-                     int rawmode)
-{
     int rc = -1;
 
-    if ((*ttymaster = open(ptmx, O_RDWR|O_NOCTTY|O_NONBLOCK)) < 0)
+    if ((*ttymaster = posix_openpt(O_RDWR|O_NOCTTY|O_NONBLOCK)) < 0)
         goto cleanup;
 
     if (unlockpt(*ttymaster) < 0)
@@ -1159,16 +1147,6 @@ cleanup:
     return rc;
 
 }
-#else
-int virFileOpenTtyAt(const char *ptmx ATTRIBUTE_UNUSED,
-                     int *ttymaster ATTRIBUTE_UNUSED,
-                     char **ttyName ATTRIBUTE_UNUSED,
-                     int rawmode ATTRIBUTE_UNUSED)
-{
-    return -1;
-}
-#endif
-
 
 /*
  * Creates an absolute path for a potentially relative path.
-- 
1.7.5.4