From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/6414 Path: news.gmane.org!not-for-mail From: Felix Janda Newsgroups: gmane.linux.lib.musl.general Subject: Re: Add login_tty Date: Sat, 1 Nov 2014 22:15:23 +0100 Message-ID: <20141101211523.GA13145@euler> References: <20140825185756.GA6077@euler> <20140825224333.GX12888@brightrain.aerifal.cx> <20140826165627.GA1208@euler> <20141031161907.GD22465@brightrain.aerifal.cx> 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 1414876562 6493 80.91.229.3 (1 Nov 2014 21:16:02 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 1 Nov 2014 21:16:02 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-6427-gllmg-musl=m.gmane.org@lists.openwall.com Sat Nov 01 22:15:55 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 1Xkg1a-0007By-UD for gllmg-musl@m.gmane.org; Sat, 01 Nov 2014 22:15:55 +0100 Original-Received: (qmail 15652 invoked by uid 550); 1 Nov 2014 21:15:53 -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 15644 invoked from network); 1 Nov 2014 21:15:53 -0000 X-Virus-Scanned: amavisd-new at posteo.de Mail-Followup-To: musl@lists.openwall.com Content-Disposition: inline In-Reply-To: <20141031161907.GD22465@brightrain.aerifal.cx> User-Agent: Mutt/1.5.22 (2013-10-16) Xref: news.gmane.org gmane.linux.lib.musl.general:6414 Archived-At: Rich Felker wrote: [..] > > >From f1d88438a6d00defcf96562ef536a4af71827ee7 Mon Sep 17 00:00:00 2001 > > From: Felix Janda > > Date: Tue, 26 Aug 2014 18:36:23 +0200 > > Subject: [PATCH] split off login_tty() from forkpty() and clean up the latter > > > > since after calling openpty() no new fds are needed, an fd limit > > causes no problems. > > I assume this remark is about the other code removals in forkpty. Even > if these were correct, they should not be part of an unrelated patch. Ok. > 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. > > 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 > > +#include > > +#include > > + > > +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. Felix