On Mon, Feb 27, 2017 at 10:19:28AM -0700, Theo de Raadt wrote:
> > On Mon, Feb 27, 2017 at 10:55:31AM +0100, Reyk Floeter wrote:
> > > The following diff is not really needed without just yet, but:
> > > - openening /dev/ptm in advance might allow better pledge in the future
> > > - customizing "openpty" will allow to do what we need next
> > > Since openpty(4) is libutil and not libc, it should be fine not using it.
> > > 
> > > OK?
> > > 
> > >     Replace openpty(3) with local function that uses pre-opened /dev/ptm 
> > > fd
> > >     
> > >     This allows more flexibility for upcoming changes and better pledge.
> > >     We also didn't use half of the features of libutil's openpty function.
> > >     Additionally, make sure that the ttys are closed correctly on 
> > > shutdown.
> 
> This is related to the change that happened in tmux, when PTM ioctl was
> locked down in pledge ioctl.  The primary purpose of the change, was that
> review of the PTM ioctl codepaths showed it was a lot of risk to keep in
> pledge "tty" programs, so it makes sense that some of them get restructured.
> 
> That means the libutil functions aren't always used.  We should consider
> whether raw ioctl PTMGET in programs is "good" or "bad".  vmd is openbsd
> only, but tmux now has this in the openbsd-specific code.
> 
> If this goes PTMGET approach goes any further, we should think about a
> new interface in libutil which hides the ioctl in a differnet way, and
> convert programs to it.  Just a thought..
> 

I had a diff for libutil but millert@ (or was it nicm@?) told me that
it is better to keep it as a local change.  I cannot remember and
unfortunately I cannot find the mail where it was mentioned.

I attached the old diff for libutil as a reference.

To fully abstract /dev/ptm in libutil, the API below would have to be
extended to have another function to open /dev/ptm in libutil as well,
eg. (better names would be desired):

        fd = openptmfd()
        pledge()
        fdopenpty(fd, ...)
        fdopenpty(fd, ...)
        fdopenpty(fd, ...)
        fdopenpty(fd, ...)

Reyk

Index: lib/libutil/openpty.3
===================================================================
RCS file: /cvs/src/lib/libutil/openpty.3,v
retrieving revision 1.17
diff -u -p -u -p -r1.17 openpty.3
--- lib/libutil/openpty.3       28 Aug 2015 19:54:06 -0000      1.17
+++ lib/libutil/openpty.3       25 Jan 2017 08:40:46 -0000
@@ -44,6 +44,8 @@
 .Ft int
 .Fn openpty "int *amaster" "int *aslave" "char *name" "struct termios *termp" 
"struct winsize *winp"
 .Ft int
+.Fn fdopenpty "int ptmfd" "int *amaster" "int *aslave" "char *name" "struct 
termios *termp" "struct winsize *winp"
+.Ft int
 .Fn login_tty "int fd"
 .Ft pid_t
 .Fn forkpty "int *amaster" "char *name" "struct termios *termp" "struct 
winsize *winp"
@@ -88,6 +90,15 @@ the caller, permissions are set to corre
 uses of that device are revoked (see
 .Xr revoke 2
 for details).
+.Pp
+The
+.Fn fdopenpty
+function works like
+.Fn openpty
+but expects a pre-opened
+.Pa /dev/ptm
+file descriptor
+.Fa ptmfd .
 .Pp
 The
 .Fn login_tty
Index: lib/libutil/pty.c
===================================================================
RCS file: /cvs/src/lib/libutil/pty.c,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 pty.c
--- lib/libutil/pty.c   30 Aug 2016 14:44:45 -0000      1.20
+++ lib/libutil/pty.c   25 Jan 2017 08:40:46 -0000
@@ -57,11 +57,30 @@ openpty(int *amaster, int *aslave, char 
        fd = open(PATH_PTMDEV, O_RDWR|O_CLOEXEC);
        if (fd == -1)
                return (-1);
-       if ((ioctl(fd, PTMGET, &ptm) == -1)) {
+
+       if (fdopenpty(fd, amaster, aslave, name, termp, winp) == -1) {
                close(fd);
                return (-1);
        }
        close(fd);
+
+       return (0);
+}
+
+int
+fdopenpty(int ptmfd, int *amaster, int *aslave, char *name,
+    struct termios *termp, struct winsize *winp)
+{
+       int master, slave;
+       struct ptmget ptm;
+
+       /*
+        * Use /dev/ptm and the PTMGET ioctl to get a properly set up and
+        * owned pty/tty pair.
+        */
+       if ((ioctl(ptmfd, PTMGET, &ptm) == -1))
+               return (-1);
+
        master = ptm.cfd;
        slave = ptm.sfd;
        if (name) {
Index: lib/libutil/util.h
===================================================================
RCS file: /cvs/src/lib/libutil/util.h,v
retrieving revision 1.34
diff -u -p -u -p -r1.34 util.h
--- lib/libutil/util.h  3 Jun 2013 21:07:02 -0000       1.34
+++ lib/libutil/util.h  25 Jan 2017 08:40:46 -0000
@@ -99,6 +99,7 @@ void  pw_copy(int, int, const struct pass
 int    pw_scan(char *, struct passwd *, int *);
 void   pw_error(const char *, int, int);
 int    openpty(int *, int *, char *, struct termios *, struct winsize *);
+int    fdopenpty(int, int *, int *, char *, struct termios *, struct winsize 
*);
 int    opendisk(const char *, int, char *, size_t, int);
 pid_t  forkpty(int *, char *, struct termios *, struct winsize *);
 int    getmaxpartitions(void);

Reply via email to