8:39 p.m.
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(a)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