[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use
From: |
Pádraig Brady |
Subject: |
bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call |
Date: |
Sat, 23 Nov 2013 01:30:23 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 |
On 11/23/2013 01:02 AM, Bernhard Voelker wrote:
> On 11/22/2013 06:39 PM, Bob Proulx wrote:
>> Eric Blake wrote:
>>> Bernhard Voelker wrote:
>>>> Eric Blake wrote:
>>>>> Just noticing this context...
>>>>>> # This test is too dangerous -- if there's a bug you're wiped out!
>>>>>> # rm -fr / 2>/dev/null && fail=1
>>>>>
>>>>> What if we use chroot to create a safer /, where failing the test would
>>>>> only wipe out the chroot?
>>>>
>>>> That's not that easy.
>>
>> I think that it would be too complicated to be desired in the test suite.
>>
>>>> Alternatively, that test could be secured by "skip_if_root_"
>>>> plus intercepting the unlinkat() call via LD_PRELOAD.
>>>
>>> Indeed, LD_PRELOAD is great for this - since the test passes when no
>>> unlink/rmdir occurs, you just make the intercepts fail loudly if they
>>> are invoked.
>>
>> Please make sure that if the LD_PRELOAD functionality fails that this
>> test is not run. Since it would be a live fire exercise and if
>> LD_PRELOAD doesn't function then the test would wipe the system out.
>> I don't think it is worth the risk for this piece of functionality.
>
> Sure, I tried to make it as defensive as possible.
> WDYT?
>
> Have a nice day,
> Berny
>
>>From a87e3d0a8417648e65ee077ca6f70d5d19fa757a Mon Sep 17 00:00:00 2001
> From: Bernhard Voelker <address@hidden>
> Date: Sat, 23 Nov 2013 01:55:36 +0100
> Subject: [PATCH] tests: add a test for rm -rf "/"
>
> * tests/rm/rm-root.sh: Add a non-root test.
> * tests/local.mk (all_tests): Mention the test.
> ---
> tests/local.mk | 1 +
> tests/rm/rm-root.sh | 159
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 160 insertions(+)
> create mode 100755 tests/rm/rm-root.sh
>
> diff --git a/tests/local.mk b/tests/local.mk
> index 3c92425..1837690 100644
> --- a/tests/local.mk
> +++ b/tests/local.mk
> @@ -208,6 +208,7 @@ all_tests = \
> tests/rm/rm3.sh \
> tests/rm/rm4.sh \
> tests/rm/rm5.sh \
> + tests/rm/rm-root.sh \
> tests/rm/sunos-1.sh \
> tests/rm/unread2.sh \
> tests/rm/unread3.sh \
> diff --git a/tests/rm/rm-root.sh b/tests/rm/rm-root.sh
> new file mode 100755
> index 0000000..3882289
> --- /dev/null
> +++ b/tests/rm/rm-root.sh
> @@ -0,0 +1,159 @@
> +#!/bin/sh
> +# Try to remove '/' recursively.
> +
> +# Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
> +print_ver_ rm
> +
> +# POSIX mandates rm(1) to skip '/' arguments. This test verifies this
> mandated
> +# behavior as well as the --preserve-root and --no-preserve-root options.
> +# Especially the latter case is a live fire exercise as rm(1) is supposed to
> +# enter the unlinkat() system call. Therefore, limit the risk as much
> +# as possible -- if there's a bug this test would wipe the system out!
> +
> +# Faint-hearted: skip this test for the 'root' user.
> +skip_if_root_
good :)
> +
> +# Pull rm(1) the teeth by intercepting the unlinkat() system call via the
s/the// ?
> +# LD_PRELOAD environment variable. This requires shared libraries to work.
> +require_gcc_shared_
> +
> +cat > k.c <<'EOF' || framework_failure_
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +
> +int unlinkat(int dirfd, const char *pathname, int flags)
s/(/ (/
> +{
> + /* Prove that LD_PRELOAD works. */
> + fclose (fopen ("x", "w"));
> +
> + /* Immediately terminate. */
> + _exit(0);
s/(/ (/
> +}
> +EOF
> +
> +# Then compile/link it:
> +gcc -Wall --std=gnu99 -shared -fPIC -ldl -O2 k.c -o k.so \
> + || framework_failure_ 'failed to build shared library'
> +
> +# Verify that "rm -rf dir" basically works.
> +mkdir dir || framework_failure_
> +rm -rf dir || framework_failure_
> +test -d dir && framework_failure_
>
> +# Now verify that intercepting unlinkat() works:
> +# rm(1) must succeed as before, but this time both the evidence file "x"
> +# and the test directory "dir" must exist afterwards.
> +mkdir dir || framework_failure_
> +LD_PRELOAD=./k.so \
> +rm -rf dir || framework_failure_
> +test -d dir || skip_ "internal test failure: maybe LD_PRELOAD doesn't work?"
> +test -f x || skip_ "internal test failure: maybe LD_PRELOAD doesn't work?"
> +
> +# Remove the evidence file "x"; verify that.
> +rm x || framework_failure_
> +test -f x && framework_failure_
> +
> +#-------------------------------------------------------------------------------
> +# exercise_rm_rf_root: shell function to test "rm -rf '/'"
> +# Paranoia mode on:
> +# For the worst case where both rm(1) would fail to refuse to process the "/"
> +# argument (in the cases without the --no-preserve-root option), and
> +# intercepting the unlinkat(1) system call would fail (which actually already
> +# has been proven to work above), limit the damage to the current file system
> +# via the --one-file-system option.
> +# Furthermore, run rm(1) in the background and kill that process after
> +# a maximum of 1 second or when the evidence file appears. This also
> +# shortens the testing time.
> +exercise_rm_rf_root ()
> +{
> + local pid
> + LD_PRELOAD=./k.so \
> + rm -rfv --one-file-system $1 '/' > out 2> err & pid=$!
> +
> + # Wait for the evidence file to appear, or until the process has
> terminated.
> + for i in $(seq 10); do
> + test -f x && break
> + kill -0 $pid || break
> + sleep .1
> + done
better to use retry_delay_ here I think and just wait for x file
> + # At this point, rm(1) usually has already terminated. Kill it anyway.
> + kill -9 $pid
probably best use a timeout 10 ... on the original rm call
Oh you're trying to minimize run time. I wouldn't bother TBH,
you can't really be half dead.
> +
> + # Get the exit status.
> + wait $pid
> +
> + return $?
> +}
> +
> +# "rm -rf /" without --no-preserve-root should output the following
> +# diagnostic error message.
> +cat <<EOD > exp || framework_failure_
> +rm: it is dangerous to operate recursively on '/'
> +rm: use --no-preserve-root to override this failsafe
> +EOD
> +
> +#-------------------------------------------------------------------------------
> +# Exercise "rm -rf /" without the --preserve-root and --no-preserve-root
> option.
> +# Expect a non-Zero exist status.
> +exercise_rm_rf_root \
> + && fail=1
> +
> +# Expect nothing in 'out' and the above error diagnostic in 'err'.
> +# As rm(1) should have skipped the "/" argument, it does not call unlinkat().
> +# Therefore, the evidence file "x" should not exist.
> +compare /dev/null out || fail=1
> +compare exp err || fail=1
> +test -f x && fail=1
> +
> +# Do nothing more if this test failed.
> +test $fail = 1 && { cat out; cat err; Exit $fail; }
> +
> +#-------------------------------------------------------------------------------
> +# Exercise "rm -rf --preserve-root /" which should behave the same as above.
> +exercise_rm_rf_root --preserve-root \
> + && fail=1
> +
> +compare /dev/null out || fail=1
> +compare exp err || fail=1
> +test -f x && fail=1
> +
> +# Do nothing more if this test failed.
> +test $fail = 1 && { cat out; cat err; Exit $fail; }
> +
> +#-------------------------------------------------------------------------------
> +# Until now, it was all just fun.
> +# Now exercise the --no-preserve-root option with which rm(1) should enter
> +# the intercepted unlinkat() system call.
> +# As the interception code terminates the process immediately via _exit(0),
> +# the exist status should be 0.
s/exist/exit/
> +exercise_rm_rf_root --no-preserve-root \
> + || fail=1
> +
> +# The 'err' file should not contain the above error diagostic.
> +grep "^rm: it is dangerous to operate recursively on '/'" err \
> + && fail=1
> +
> +# Instead, rm(1) should have called the intercepted unlinkat() function,
> +# i.e. the evidence file "x" should exist.
> +test -f x || fail=1
> +
> +test $fail = 1 && { cat out; cat err; Exit $fail; }
> +
> +Exit $fail
>
So the real case where this could not be handled (and what might
actually catch users out) is when various synonyms of '/' are specified.
i.e. // //. /./ /bin/..
It would be good to have tests for those.
cheers,
Pádraig.
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, (continued)
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Jim Meyering, 2013/11/21
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Bernhard Voelker, 2013/11/22
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Eric Blake, 2013/11/22
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Bernhard Voelker, 2013/11/22
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Eric Blake, 2013/11/22
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Bob Proulx, 2013/11/22
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Bernhard Voelker, 2013/11/22
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call,
Pádraig Brady <=
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Eric Blake, 2013/11/22
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Pádraig Brady, 2013/11/23
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Pádraig Brady, 2013/11/23
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Bernhard Voelker, 2013/11/24
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Pádraig Brady, 2013/11/24
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Bernhard Voelker, 2013/11/25
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Bernhard Voelker, 2013/11/26
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Pádraig Brady, 2013/11/26
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Bernhard Voelker, 2013/11/27
- bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call, Pádraig Brady, 2013/11/28