From owner-svn-src-head@freebsd.org Wed Jan 6 02:48:46 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 59677A6352A; Wed, 6 Jan 2016 02:48:46 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id 237D91B81; Wed, 6 Jan 2016 02:48:45 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id CE3D11A3008; Wed, 6 Jan 2016 13:48:37 +1100 (AEDT) Date: Wed, 6 Jan 2016 13:48:36 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ian Lepore cc: Warner Losh , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r293227 - head/etc In-Reply-To: <1452038404.1320.46.camel@freebsd.org> Message-ID: <20160106125617.E968@besplex.bde.org> References: <201601052120.u05LKlQw074919@repo.freebsd.org> <1452038404.1320.46.camel@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=R4L+YolX c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=JzwRw_2MAAAA:8 a=kj9zAlcOel0A:10 a=F8TLbMH91mOxZ-GJiaoA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 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, 06 Jan 2016 02:48:46 -0000 On Tue, 5 Jan 2016, Ian Lepore wrote: >> Log: >> Use the more proper -f. Leave /bin/rm in place since that's what >> other rc scripts have, though it isn't strictly necessary. "proper -f" is hard to parse. I think you mean: Use 'rm -f' to turn off -i in case rm is broken and is an alias which has -i (and perhaps actually even something resembling rm) in it. More precisely, use 'rm -f /usr/bin' to partly defend against the same bug in /bin/rm (where it would be larger). Keep using /usr/rm instead of restoring the use of plain rm since that is what other rc scripts have. The previous change to use /bin/rm instead of plain rm was neither necessary nor sufficient for fixing the bug. Neither is this one, but it gets closer. It is a little-known bug in aliases that even absolute pathnames can be aliased. So /bin/rm might be aliased to 'rm -ri /'. Appending -f would accidentally help for that too, by turning it into a syntax error, instead of accidentally making it more forceful by turning -ri into -rf. Hopefully this is all FUD. Non-interactive scripts shouldn't source any files that are not mentioned in the script. /etc/rc depends on a secure environment being set up by init and probably gets it since init doesn't set up much. sh(1) documents closing the security hole of sourcing the script in $ENV for non-interactive shells, but was never a problem for /etc/rc since init must be trusted to not put security holes in $ENV. But users could put security holes in a sourced config file like /etc/rc.conf.local. >> Modified: head/etc/rc >> ===================================================================== >> ========= >> --- head/etc/rc Tue Jan 5 21:20:46 2016 (r293226) >> +++ head/etc/rc Tue Jan 5 21:20:47 2016 (r293227) >> @@ -132,9 +132,9 @@ done >> # Remove the firstboot sentinel, and reboot if it was requested. >> if [ -e ${firstboot_sentinel} ]; then >> [ ${root_rw_mount} = "yes" ] || mount -uw / >> - /bin/rm ${firstboot_sentinel} >> + /bin/rm -f ${firstboot_sentinel} >> if [ -e ${firstboot_sentinel}-reboot ]; then >> - /bin/rm ${firstboot_sentinel}-reboot >> + /bin/rm -f ${firstboot_sentinel}-reboot >> [ ${root_rw_mount} = "yes" ] || mount -ur / >> kill -INT 1 >> fi > > Using rm -f to suppress an error message seems like a bad idea here -- > if the sentinel file can't be removed that implies it's going to do > firstboot behavior every time it boots, and that's the sort of error > that should be in-your-face. Especially on the reboot one because > you're going to be stuck in a reboot loop with no error message. Er, -f on rm only turns off -i and supresses the warning message for failing to remove nonexistent files. But we just tested that the file exists, and in the impossible even of a race making it not exist by the time that it runs, we have more problems than the failure of rm since we use the file's existence as a control for other things. So the only effect of this -f is to turn off -i, which can only be set if the FUD was justified. The correct fix seems to be 'unalias -a'. Bruce