From owner-freebsd-current@FreeBSD.ORG Mon Jul 16 14:30:11 2007 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id B732116A40D for ; Mon, 16 Jul 2007 14:30:11 +0000 (UTC) (envelope-from barnaclewes@gmail.com) Received: from ik-out-1112.google.com (ik-out-1112.google.com [66.249.90.180]) by mx1.freebsd.org (Postfix) with ESMTP id 3708713C4AC for ; Mon, 16 Jul 2007 14:30:08 +0000 (UTC) (envelope-from barnaclewes@gmail.com) Received: by ik-out-1112.google.com with SMTP id c21so651139ika for ; Mon, 16 Jul 2007 07:30:07 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=quRFjnXaS5cN4CP9iFWxxiVD7+M9Zujbpb+oS7RrTiD8/GS7J70KJXjaIGRk7NvwUlRqKVaToO3BqkliHCqEiSMvAsZVkfykdZD/PMxBIjQQEoFlZ3Nh2K40+3NYNeXakb9zhy8dqD2ADwBQWgRizoGoJICViaD/CUiKLgSe44s= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=ZtaOKJqeKsw92GzAx30YPgj4dsu4oqFUm9j9YIxx+Xj4Fxu/FDlz4im82xeVFwOByKdae44H7wQAGec/jMYpKMdtYZXf0p+VOci7KuYPNJx44IZjku3PWyXUE1IIQz8+W8NbJbSSOIVuhHxKsf0spoKEWSwzLh1nRZBlBjMfmgk= Received: by 10.78.176.20 with SMTP id y20mr1180935hue.1184594548491; Mon, 16 Jul 2007 07:02:28 -0700 (PDT) Received: by 10.78.201.4 with HTTP; Mon, 16 Jul 2007 07:02:28 -0700 (PDT) Message-ID: Date: Mon, 16 Jul 2007 07:02:28 -0700 From: "Wes Peters" To: "Andrey Chernov" , "Sean C. Farley" , "Robert Watson" , freebsd-current , "Michal Mertl" In-Reply-To: <20070713162742.GA16260@nagual.pp.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20070704215154.O77978@thor.farley.org> <20070705105922.F98700@thor.farley.org> <20070707130859.GA96605@nagual.pp.ru> <20070707131359.GB96605@nagual.pp.ru> <20070707133102.C14065@thor.farley.org> <20070707191835.GA4368@nagual.pp.ru> <20070707205410.B14065@thor.farley.org> <20070708020940.GA80166@nagual.pp.ru> <20070708171727.GA90490@nagual.pp.ru> <20070713162742.GA16260@nagual.pp.ru> Cc: Subject: Re: Environment handling broken in /bin/sh with changes to {get, set, put}env() 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: Mon, 16 Jul 2007 14:30:11 -0000 On 7/13/07, Andrey Chernov wrote: > On Sun, Jul 08, 2007 at 09:17:27PM +0400, Andrey Chernov wrote: > > Hmm. I just think a bit more and feel worry about that place in the merge > > code: > > > > *equals = '\0'; > > if (setenv(*env, equals + 1, 1) == -1) > > return (-1); > > *equals = '='; > > because it modifies memory which may be treated like const one. > > > > Consider following scenario: getenv() is not thread-safe, but may be > [snip] > > I found another breakage case not covered by your last getenv() fix. > Take this simple program: > > -- a.c ------------------------------------------------------------------- > #include > extern char **environ; > > main () { > > static char *nenv[2]; > > nenv[0] = "PATH=/bin"; > nenv[1] = NULL; > > /* > environ = nenv; > unsetenv("PATH"); or somethig like > which touch '=' char in nenv[0] > */ > > nenv[0][4] = '\0'; > > } > -- a.c ------------------------------------------------------------------- > > Look at assembler code first: > > cc -S a.c > cat a.s > .file "a.c" > .local nenv.1948 > .comm nenv.1948,8,4 > .section .rodata > .LC0: > .string "PATH=/bin" > .text > > [skipped] > > As you may see, compiler puts "PATH=/bin" to the program's .rodata section > which is placed to read only memory. > > If later you'll modify this single "PATH=/bin" (comes from "nenv" now) by > *equals = '\0'; > ... > *equals = '='; > core dump happens, which simulated in my simple a.c example by > nenv[0][4] = '\0'; > > Just run it and got code dump. The default setting for 'writable-string' changed in gcc; strings are now NOT writable by default. While it may seem reasonable, setting the programs environment to non-writable strings by manipulating the environ pointer, then attempting to modify those strings, is bound to fail. Since you've directly set the environment, there isn't much that {set,put}env() could do, unless you want to get clever and have them detect strings in .rodata and work around that. The simplest solution may be a caution in the man page about stuffing read-only strings into the environment. -- Against stupidity the very gods Themselves contend in vain. Friedrich Schiller