Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 6 Jan 2016 13:48:36 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Ian Lepore <ian@freebsd.org>
Cc:        Warner Losh <imp@freebsd.org>, src-committers@freebsd.org,  svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r293227 - head/etc
Message-ID:  <20160106125617.E968@besplex.bde.org>
In-Reply-To: <1452038404.1320.46.camel@freebsd.org>
References:  <201601052120.u05LKlQw074919@repo.freebsd.org> <1452038404.1320.46.camel@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160106125617.E968>