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@xxxxxxxxxxxxx> --- 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list