joshua stein <[email protected]> wrote: > On Thu, 06 Feb 2020 at 11:21:11 -0700, Todd C. Miller wrote: > > On Thu, 06 Feb 2020 10:45:44 -0700, "Theo de Raadt" wrote: > > > > > That feels better, and will be more atomic. > > > > Unfortunately, we can't do this in ptmioctl() since we don't have > > the index of the open ptm device to use to check for the presence > > of UF_EXCLOSE. So it has to be done in libc instead. > > Alright, so like this then? > > > Index: stdlib/posix_openpt.3 > =================================================================== > RCS file: /cvs/src/lib/libc/stdlib/posix_openpt.3,v > retrieving revision 1.4 > diff -u -p -u -p -r1.4 posix_openpt.3 > --- stdlib/posix_openpt.3 25 Jan 2019 00:19:25 -0000 1.4 > +++ stdlib/posix_openpt.3 6 Feb 2020 17:34:15 -0000 > @@ -45,7 +45,7 @@ argument is formed by bitwise-inclusive > .Tn OR Ns 'ing > the following values defined in > .In fcntl.h : > -.Bl -tag -width O_NOCTTY -offset indent > +.Bl -tag -width O_CLOEXEC -offset indent > .It Dv O_RDWR > Open for reading and writing. > .It Dv O_NOCTTY > @@ -53,6 +53,8 @@ Prevent the device from being made the c > This flag has no effect on > .Ox > and is included for compatibility with other systems. > +.It Dv O_CLOEXEC > +Set the close-on-exec flag for the new file descriptor. > .El > .Pp > The > @@ -95,6 +97,10 @@ The > .Fn posix_openpt > function conforms to > .St -p1003.1-2001 . > +.Pp > +The ability to use > +.Dv O_CLOEXEC > +is an extension to that standard. > .Sh HISTORY > The > .Fn posix_openpt > Index: stdlib/posix_pty.c > =================================================================== > RCS file: /cvs/src/lib/libc/stdlib/posix_pty.c,v > retrieving revision 1.3 > diff -u -p -u -p -r1.3 posix_pty.c > --- stdlib/posix_pty.c 25 Jan 2019 00:19:25 -0000 1.3 > +++ stdlib/posix_pty.c 6 Feb 2020 17:34:15 -0000 > @@ -36,7 +36,7 @@ posix_openpt(int oflag) > > /* User must specify O_RDWR in oflag. */ > if ((oflag & O_ACCMODE) != O_RDWR || > - (oflag & ~(O_ACCMODE | O_NOCTTY)) != 0) { > + (oflag & ~(O_ACCMODE | O_NOCTTY | O_CLOEXEC)) != 0) { > errno = EINVAL; > return -1; > } > @@ -46,7 +46,11 @@ posix_openpt(int oflag) > if (fd != -1) { > if (ioctl(fd, PTMGET, &ptm) != -1) { > close(ptm.sfd); > - mfd = ptm.cfd; > + if ((oflag & O_CLOEXEC) && > + fcntl(ptm.cfd, F_SETFD, FD_CLOEXEC) == -1) > + close(ptm.cfd); > + else > + mfd = ptm.cfd; > } > close(fd);
Only on the child? The master does not get it? I suggest an earlier return from fcntl failure. As it is now, this can return an errno from close(fd), rather than from the fcntl, hiding the actual facts. Not that either will error out, but the fallthrough seems odd.
