Re: multiple devpts mounts not supported insysdeps/unix/sysv/linux/ptsname.c:_ptsname_internal()

["Serge E. Hallyn" <serge dot hallyn at canonical dot com>]

Quoting Mike Frysinger (vapier@gentoo.org):
> On Tuesday 11 October 2011 13:15:37 Serge E. Hallyn wrote:
> > grantpt, ptsname, and ptsname_r make use of _ptsname_internal().  That
> > function works by appending the TIOCGPTN result to "/dev/pts".  The
> > path /dev/pts is hardcoded.  That means that if you call grantpt on an
> > fd from /chroot/dev/pts, and /dev/pts/0 (for instance) does not exist
> > on the host, you'll (wrongly) get back an error.  This has been seen with
> > libvirt (and is easly to verify with a simple testcase).
> 
> `mount --bind /dev/pts /chroot/dev/pts` ?

Sorry, do you mean as a testcase?  You can just
	mkdir e
	mount -t devpts -o newinstance,ptmxmode=0666,mode=0620,gid=5 e
and run granpt on a pty under e/ for which the host doesn't have a
corresponding /dev/pts/N.  I'm attaching a nice simple little test program
by Scott Moser - though it requires that you first make sure /dev/pts/0
doesn't exist on the host.  (For instance, log into desktop, start two
terminals, close the first one, run the program in the second terminal).

> > I'm not sure of the best way to fix this in a way palatable to glibc.
> > Follow the /proc/self/fd/N symlink?  Update the manpages to say the
> > devpts used must be mounted under /dev/pts by the caller?  (Caller would
> > need privilege, but can fork+unshare+bind-mount).
> 
> doesn't the caller already need privs to chroot in the first place ?

The caller hasn't chrooted (if he had, then glibc would find /dev/pts/0 :).
Now someone needed privs to mount the new devpts, but it's easy to conceive
a separation of duties such that someone is calling ptsname or grantpt
after having dropped the privileges.  At which point the problem IMO is
mainly that there is no indication of *why* there is a failure.

For libvirt, however, it should be able to do work around it if it has
to.

> it's been this way for years which means i would have expected all relevant 
> code to have accounted for this already (not saying that it wouldn't be nice 
> to have things "just work" when possible) ...
> -mike

It rarely shows up in libvirt, but that's purely by accident since
/dev/pts/0 is usually in use, and if it isn't the driver creates a pty
on the host first, which will take /dev/pts/0.  So you have to do
something to keep /dev/pts/0 from being re-used on the host, but it's
doable (and has been done and reported).

So if the answer is just don't do that, then the manpage should be updated,
right?  The problem is that unless you look at the implementation, you have
no idea why grantpt shouldn't work on an fd opened in another ptsns.

It'd still be neat if we could fix it for real, but the only way I can
think of is to follow /proc/self/fd/<fd>, and while i've not thought it
through i have a feeling that's not a good idea.

thanks,
-serge

/*
  test-virFileOpen.c
  This mimics libvirt/src/lxc/lxc_controller.c in lxcControllerRun

  gcc -o test-virFileOpen test-virFileOpen.c
  mkdir d
  sudo ./test-virFileOpen d

  We get varied results.  Sometimes this passes, sometimes it fails.
*/
#define _XOPEN_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <sys/mount.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <termios.h>
#include <unistd.h>
#include <errno.h>
#define PATH_MAX 4096



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) {
        printf("open failed\n");
        goto cleanup;
    }
    printf("opened %s to %i ptsname=%s\n", ptmx, *ttymaster, ptsname(*ttymaster));

    if (unlockpt(*ttymaster) < 0) {
        printf("unlockpt failed\n");
        goto cleanup;
    }

    if (grantpt(*ttymaster) < 0) {
        printf("grantpt failed (%i)\n", errno);
        perror("grantpt");
        goto cleanup;
    }

    if (rawmode) {
        struct termios ttyAttr;
        if (tcgetattr(*ttymaster, &ttyAttr) < 0) {
            printf("tcgetattr failed\n");
            goto cleanup;
        }

        cfmakeraw(&ttyAttr);

        if (tcsetattr(*ttymaster, TCSADRAIN, &ttyAttr) < 0) {
            printf("tcsetattr failed\n");
            goto cleanup;
        }
    }

    if (ttyName) {
        // if (VIR_ALLOC_N(*ttyName, PATH_MAX) < 0) {
        if ((ttyName = malloc(PATH_MAX)) == 0) {
            printf("failed malloc\n");
            errno = ENOMEM;
            goto cleanup;
        }

        if (ptsname_r(*ttymaster, *ttyName, PATH_MAX) < 0) {
            printf("ptsname_r failed\n");
            goto cleanup;
        }
    }

    rc = 0;

cleanup:
    if (rc != 0)
        close(*ttymaster);
        // VIR_FORCE_CLOSE(*ttymaster);

    return rc;

}

int main(int argc, char* argv[]) {
   char *devpts = argv[1];
   char devptmx[4096];
   int containerPty = -1;
   char *containerPtyPath = NULL;

   sprintf(devptmx, "%s/ptmx", devpts);
   printf("devptmx=%s devpts=%s\n", devptmx, devpts);
   if (mount("devpts", devpts, "devpts", 0,
      "newinstance,ptmxmode=0666,mode=0620,gid=5") < 0) {
      printf("mount of %s failed\n", devpts);
      return(1);
   }
   
   if (virFileOpenTtyAt(devptmx,
                        &containerPty,
                        &containerPtyPath,
                        0) < 0) {
       printf("virFileOpenTtyAt failed\n");
       close(containerPty);
       if(umount(devpts) != 0) { perror("umount"); printf("umount failed\n"); }
       return(1);
   }
   close(containerPty);
   if(umount(devpts) != 0) { perror("umount"); printf("umount failed\n"); }

   printf("Everything seems good\n");
}