Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20141102222818.GR22465@brightrain.aerifal.cx>
Date: Sun, 2 Nov 2014 17:28:18 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Add login_tty

On Sun, Nov 02, 2014 at 07:56:38PM +0100, Felix Janda wrote:
> Rich Felker wrote:
> [..]
> > There's an approach I use in posix_spawn that could be copied here. My
> > hope was to keep forkpty simpler but it's not too bad and it would
> > make it so we could handle other arbitrary errors in the child if we
> > ever need to, so maybe it's a good idea.
> 
> So how about the following?
> (I'm not sure whether error checking is necessary for read.)
> 
> 
> #include <pty.h>
> #include <utmp.h>
> #include <unistd.h>
> 
> int forkpty(int *m, char *name, const struct termios *tio, const struct winsize *ws)
> {
> 	int s, ec, p[2];
> 	pid_t pid;
> 
> 	if (pipe2(p, O_CLOEXEC)) return -1;
> 	if (openpty(m, &s, name, tio, ws) < 0) return -1;
> 
> 	pid = fork();
> 	if (!pid) {
> 		close(*m);
> 		close(p[0]);
> 		ec = login_tty(s);
> 		while (write(p[1], &ec, sizeof ec) < 0);
> 		if (ec) _exit(127);
> 		close(p[1]);
> 		return 0;
> 	}
> 	close(s);
> 	close(p[1]);
> 	if (pid > 0) read(p[0], &ec, sizeof ec);
> 	close(p[0]);
> 	if (pid < 0 || ec < 0) {
> 		close(*m);
> 		return -1;
> 	}
> 	return pid;
> }

The main omission I see is that it's leaking pids (zombies) on failure
-- the pid is never returned to the caller and you don't wait on it
here, so there's no way it will ever get reaped. This is easily fixed
by calling waitpid in the failure path.

You're also leaking the pipe fds when openpty fails, etc.

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.