From owner-freebsd-current@FreeBSD.ORG Fri Dec 4 18:02:27 2009 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3A454106566B for ; Fri, 4 Dec 2009 18:02:27 +0000 (UTC) (envelope-from scf@FreeBSD.org) Received: from mail.farley.org (mail.farley.org [IPv6:2001:470:1f0f:20:2::11]) by mx1.freebsd.org (Postfix) with ESMTP id EBA6B8FC14 for ; Fri, 4 Dec 2009 18:02:26 +0000 (UTC) Received: from thor.farley.org (HPooka@thor.farley.org [IPv6:2001:470:1f0f:20:1::5]) by mail.farley.org (8.14.3/8.14.3) with ESMTP id nB4I2PSN042664; Fri, 4 Dec 2009 12:02:25 -0600 (CST) (envelope-from scf@FreeBSD.org) Date: Fri, 4 Dec 2009 12:02:25 -0600 (CST) From: "Sean C. Farley" To: Jilles Tjoelker In-Reply-To: <20091204161031.GA37372@stack.nl> Message-ID: References: <20091204161031.GA37372@stack.nl> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Spam-Status: No, score=-2.8 required=4.0 tests=AWL,BAYES_00,NO_RELAYS autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on mail.farley.org Cc: freebsd-current@FreeBSD.org Subject: Re: environ function patch for review X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Dec 2009 18:02:27 -0000 On Fri, 4 Dec 2009, Jilles Tjoelker wrote: > On Thu, Dec 03, 2009 at 06:04:47PM -0600, Sean C. Farley wrote: >> Regarding the recent security issue with the unsetenv() calls in >> rtld, I have made a patch[1] I would like reviewed prior to commit. >> It changes the behavior of all the *env() routines that cause an >> internal environment to be created. This is putenv(), setenv() and >> unsetenv(). getenv() will not cause an internal environment to be >> created. I have tested the patch without the rltd fix, and it >> prevents the security issue. > >> Instead of returning an error when tripping upon a corrupt >> environment, it will return an error when the caller passes bad >> argument(s) (EINVAL) or if unable to allocate memory (ENOMEM). >> Except for the possibility for ENOMEM, this should make the behavior >> the same as FreeBSD 6 and below. > > It would be very nice to avoid ENOMEM on unsetenv() somehow, such as > by rewriting environ in place. Neither FreeBSD6 nor POSIX mention the > possibility of ENOMEM on unsetenv(). The only error listed by POSIX is > an invalid variable name (unsetting a variable that does not exist is > not an error), so it seems reasonable to assume unsetenv() is > successful if passed a valid constant variable name. I agree that they do not mention the possibility of ENOMEM with unsetenv() but only in the unsetenv() man pages. From OpenGroup's getenv() page[1], there is a constraint of a conforming application not altering environ. The "rationale" section states: This constraint allows the implementation to properly manage the memory it allocates, either by using allocated storage for all variables (copying them on the first invocation of setenv() or unsetenv()), ... >From this, it appears that unsetenv() is allowed to set errno to ENOMEM. At least, it should be able to do that from that rationale. > If unsetenv() has to copy the environment, there cannot be any > setenv() or putenv() strings in it. So it seems correct to remove the > pointer from environ without doing anything else, instead. I can add this, however, I was trying to keep the complexity down a bit. Since the copying would normally only occur once in the lifetime of an application and not necessarily by unsetenv(), most of the time unsetenv() will take that code path one time if at all. > If the environment has already been copied, the unsetenv() should > proceed without needing to allocate any memory. This seems to be the > case. Yes, unsetenv() only allocates memory upon its first call using an environ that has not been previously copied. How did the patches look otherwise, or are you suggesting this change to unsetenv() in place of the patch to continue even after discovering a corrupt environ? Sean 1. http://opengroup.org/onlinepubs/009695399/functions/getenv.html -- scf@FreeBSD.org