From owner-svn-src-head@FreeBSD.ORG Wed Dec 2 15:33:34 2009 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B99D5106566B; Wed, 2 Dec 2009 15:33:34 +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 72FE48FC19; Wed, 2 Dec 2009 15:33:34 +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 nB2FXTqC032102; Wed, 2 Dec 2009 09:33:30 -0600 (CST) (envelope-from scf@FreeBSD.org) Date: Wed, 2 Dec 2009 09:33:29 -0600 (CST) From: "Sean C. Farley" To: "M. Warner Losh" In-Reply-To: <20091201.193518.387188323.imp@bsdimp.com> Message-ID: References: <200912010504.nB154VnS053167@svn.freebsd.org> <4B14B32C.3060409@freebsd.org> <20091201.193518.387188323.imp@bsdimp.com> 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.6 required=4.0 tests=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: green@FreeBSD.org, src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, rwatson@FreeBSD.org, cperciva@FreeBSD.org, svn-src-head@FreeBSD.org Subject: Re: svn commit: r199983 - in head: lib/libc/stdlib tools/regression/environ X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 Dec 2009 15:33:34 -0000 On Tue, 1 Dec 2009, M. Warner Losh wrote: > In message: > Robert Watson writes: > : On Mon, 30 Nov 2009, Colin Percival wrote: > : > : > Brian Feldman wrote: > : >> Do not gratuitously fail *env(3) operations due to corrupt ('='-less) > : >> **environ entries. This puts non-getenv(3) operations in line with > : >> getenv(3) in that bad environ entries do not cause all operations to > : >> fail. There is still some inconsistency in that getenv(3) in the > : >> absence of any environment-modifying operation does not emit corrupt > : >> environ entry warnings. > : >> > : >> I also fixed another inconsistency in getenv(3) where updating the > : >> global environ pointer would not be reflected in the return values. > : >> It would have taken an intermediary setenv(3)/putenv(3)/unsetenv(3) > : >> in order to see the change. > : > > : > The FreeBSD Security Team is currently dealing with a security > : > issue relating to this code. Please back out your change (at > : > least to getenv.c; I don't particularly care about the regression > : > tests) until we've finished, and then submit the patch to us for > : > review along with a detailed explanation of what it does. > : > > : > We've already had two major security issues arising out of > : > getenv.c in the past year, and I'd like to make sure we don't have > : > a third. > : > : I think it's fair to say that the POSIXization of the environment > : code has been an unmitigated disaster, and speaks to the necessity > : for careful review of those sorts of code changes. > > Why we're not just reverting the whole thing as a bad idea is beyond > me. Clearly the tiny incremental benefits have been far overshadowed > by this fiasco. Which "whole thing"? The code or the POSIX-compliance? Technically, it is not pure compliance because the code has a few BSD requirements in it such as keeping old name=value entries even when new ones are created. It was my fault for not checking how unsetenv() was used in all of base. The change to use unsetenv() in rtld.c was committed just prior to my change which introduced a version of unsetenv() that returned an int to allow checking. I am testing a change to have unsetenv() not stop in its attempt to unset a variable which should mimic the old behavior. One difference between our man page and IEEE Std 1003.1-2008 is the part concerning that the "environment shall be unchanged" which I will introduce to the man page: Upon successful completion, zero shall be returned. Otherwise, -1 shall be returned, errno set to indicate the error, and the environment shall be unchanged. After my change, unsetenv() will not return an error if environ is corrupt. It will wipe the variable. The only time errors will be returned are when the name passed to unsetenv() is invalid or when memory allocation fails. One question is whether the code should abort() when it detects a corrupt environ array or do its best to complete the request from the caller. Sean -- scf@FreeBSD.org