|
Date: Sat, 1 Nov 2014 18:43:25 -0400 From: Rich Felker <dalias@...c.org> To: musl@...ts.openwall.com Subject: Re: Add login_tty One more issue: On Sat, Nov 01, 2014 at 11:27:29PM +0100, Felix Janda wrote: > int forkpty(int *m, char *name, const struct termios *tio, const struct winsize *ws) > { > @@ -13,12 +12,7 @@ int forkpty(int *m, char *name, const struct termios *tio, const struct winsize > pid = fork(); > if (!pid) { > close(*m); > - setsid(); > - ioctl(s, TIOCSCTTY, (char *)0); > - dup2(s, 0); > - dup2(s, 1); > - dup2(s, 2); > - if (s>2) close(s); > + login_tty(s); Here there's no checking for failure of login_tty. Note that checking for failure would not be useful, because there's no valid response, but: > diff --git a/src/misc/login_tty.c b/src/misc/login_tty.c > new file mode 100644 > index 0000000..f0be0a0 > --- /dev/null > +++ b/src/misc/login_tty.c > @@ -0,0 +1,14 @@ > +#include <utmp.h> > +#include <sys/ioctl.h> > +#include <unistd.h> > + > +int login_tty(int fd) > +{ > + setsid(); > + if (ioctl(fd, TIOCSCTTY, (char *)0)) return -1; > + dup2(fd, 0); > + dup2(fd, 1); > + dup2(fd, 2); > + if (fd>2) close(fd); > + return 0; > +} Unlike the code moved out of forkpty, this code errors out early if the ioctl fails, thereby skipping the dup2's and close. In the case of forkpty, this leaves the child process in a very messed-up state with regard to its file descriptors; it will clobber the parent's stdin/out/err without knowing anything is wrong. In the case of forkpty, the error just needs to be ignored, I think, like it is now. I'm not sure what login_tty should do, though. Reporting the error is good, but leaving the process and its file descriptors in an unpredictable state is not good. Is there any documentation for how they're left on error? Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.