From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/6416 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: Add login_tty Date: Sat, 1 Nov 2014 17:45:03 -0400 Message-ID: <20141101214503.GK22465@brightrain.aerifal.cx> References: <20140825185756.GA6077@euler> <20140825224333.GX12888@brightrain.aerifal.cx> <20140826165627.GA1208@euler> <20141031161907.GD22465@brightrain.aerifal.cx> <20141101211523.GA13145@euler> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1414878324 31913 80.91.229.3 (1 Nov 2014 21:45:24 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 1 Nov 2014 21:45:24 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-6429-gllmg-musl=m.gmane.org@lists.openwall.com Sat Nov 01 22:45:18 2014 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1XkgU1-0002bk-Hi for gllmg-musl@m.gmane.org; Sat, 01 Nov 2014 22:45:17 +0100 Original-Received: (qmail 32470 invoked by uid 550); 1 Nov 2014 21:45:16 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 32462 invoked from network); 1 Nov 2014 21:45:15 -0000 Content-Disposition: inline In-Reply-To: <20141101211523.GA13145@euler> User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:6416 Archived-At: On Sat, Nov 01, 2014 at 10:15:23PM +0100, Felix Janda wrote: > > But in this case they're not correct: > > > > > int forkpty(int *m, char *name, const struct termios *tio, const struct winsize *ws) > > > { > > > - int s, t, i, istmp[3]={0}; > > > + int s; > > > pid_t pid; > > > > > > if (openpty(m, &s, name, tio, ws) < 0) return -1; > > > > > > - /* Ensure before forking that we don't exceed fd limit */ > > > - for (i=0; i<3; i++) { > > > - if (fcntl(i, F_GETFL) < 0) { > > > - t = fcntl(s, F_DUPFD, i); > > > - if (t<0) break; > > > - else if (t!=i) close(t); > > > - else istmp[i] = 1; > > > - } > > > - } > > > > This loop is checking whether fd 0/1/2 are already open in the parent, > > and if not, temporarily allocating them prior to fork to detect an > > error before fork, since we can't handle errors after fork. The idea > > is that dup2 might fail when dup'ing onto an unallocated fd, but > > should never fail when atomically replacing an existing one. I'm not > > 100% sure this is correct -- the kernel might deallocate some resource > > then reallocate, rather than using in-place, in which case there would > > be a resource exhaustion leak -- but that's at least the intent of the > > code. > > I still don't understand how dup2 can fail when fd 0/1/2 are not open in > the parent. AFAIU, limits on the number of open fds are imposed by an > upper bound on the value of any fd. For the dup2 calls we know that the > newfds are certainly within the limits. Indeed, looking at the kernel code, I don't see any error paths where this operation could fail. I had figured some allocations might be needed to represent the new fd in the fd table, but it seems not. So the current code is probably unnecessary. > > > +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; > > > +} > > > > Is login_tty supposed to close the fd passed to it? > > The man page says so. OK. Surprising, but whatever. :) In that case maybe your patch is okay as-is, aside from needing to be factored into two changes -- one for removing useless code and the other for separating-out login_tty. Rich