From owner-freebsd-security@FreeBSD.ORG Wed May 18 15:41:16 2005 Return-Path: Delivered-To: freebsd-security@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 016D316A4CE; Wed, 18 May 2005 15:41:16 +0000 (GMT) Received: from postal.sdsc.edu (postal.sdsc.edu [132.249.20.114]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5EB3B43D75; Wed, 18 May 2005 15:41:15 +0000 (GMT) (envelope-from okumoto@ucsd.edu) Received: from multivac.sdsc.edu (IDENT:eK5vQeZ5LcE+F8Etiw2/Agxi5RliIm7d@multivac.sdsc.edu [132.249.20.57]) j4IFfEP27680; Wed, 18 May 2005 08:41:14 -0700 (PDT) Received: (from okumoto@localhost)j4IFfEgZ003295; Wed, 18 May 2005 08:41:14 -0700 (PDT) X-Authentication-Warning: multivac.sdsc.edu: okumoto set sender to okumoto@ucsd.edu using -f To: Giorgos Keramidas References: <200505121545.j4CFjENu078768@repoman.freebsd.org> <20050512180743.6z1h22fldwksgw4w@netchild.homeip.net> <42897003.2090005@ucsd.edu> <20050517144446.gibxprydoosokw0k@netchild.homeip.net> <428A23A2.5080108@ucsd.edu> <20050518100548.h8r4qc59c08swoog@netchild.homeip.net> <20050518141456.GB40240@orion.daedalusnetworks.priv> From: Max Okumoto Date: Wed, 18 May 2005 08:41:14 -0700 In-Reply-To: <20050518141456.GB40240@orion.daedalusnetworks.priv> (Giorgos Keramidas's message of "Wed, 18 May 2005 17:14:56 +0300") Message-ID: User-Agent: Gnus/5.1006 (Gnus v5.10.6) Emacs/21.2 (usg-unix-v) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Mailman-Approved-At: Thu, 19 May 2005 12:33:30 +0000 cc: freebsd-security@freebsd.org cc: Alexander Leidinger Subject: Re: cvs commit: src/usr.bin/make job.c X-BeenThere: freebsd-security@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Security issues [members-only posting] List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 May 2005 15:41:16 -0000 Giorgos Keramidas writes: > On 2005-05-18 01:41, Max Okumoto wrote: >> Your idea of using mkdtemp() can be fixed by putting a loop >> around the code. Each time around the loop would be expensive >> but we wouldn't be doing that to often anyway. >> >> loop: >> mkdtemp(template) >> mkfifo(tempalte + "/fifo") >> if error remove temp directory, restore template and loop. >> >> Or better yet, if someone could create an equiv function in libc >> so I don't have to maintain it in make(1) :-) Do any other >> programs need the ability to make a temp fifo? >> >> Personally, I don't think it is a risk, but I wanted other >> peoples opinions, before I tried to fix a non-issue. :-) > > Does this really need to be of the form DIR/fifo ? > > I haven't looked at the code that uses the fifo at all, so I risk being > extremely out of topic here, but why wouldn't a temporary fifo created > with a name obtained from mkstemp() work too? I think you mean mktemp(), since mkstemp() actucally creates a file and returns a file descriptor. The reason that I rewrote the code to obtain a fifo file was that the libc generates a warnning message when you link with mktemp(). warning: mktemp() possibly used unsafely; consider using mkstemp() As part of the refactoring of the make(1) that I am doing, I am correcting all the warnings. The original code that phk committed generated this messages and had the race condition. The current code which is derived from mkstemp(), got rid of the error message and the race condition. But has the disadvantage that the code is pretty much a duplicate of the original libc code, sitting in /src/usr.bin/make/job.c:mkfifotemp() which is 80 lines. Alexander suggested that I replace that code with mkdtemp(template) mkfifo(tempalte + "/fifo") Which removed alot of the code duplication, but added the race back in. So my question is it this race something that I should worry about? IMHO removing the warning message was important, but the race isn't much of a problem, but would like some input on it. Thanks Max > > A directory won't be needed if the fifo name is created by mkstemp() and > then passed directly to mkfifo(2). > > Then there is still a (small?) possibility for a race, but a subsequent > invocation of mkstemp() is almost guaranteed to work, unless mkstemp() > is severely broken.