On 11/23/2013 02:30 AM, Pádraig Brady wrote:
> On 11/23/2013 01:02 AM, Bernhard Voelker wrote:
>> >From a87e3d0a8417648e65ee077ca6f70d5d19fa757a Mon Sep 17 00:00:00 2001
>> From: Bernhard Voelker <[email protected]>
>> Date: Sat, 23 Nov 2013 01:55:36 +0100
>> Subject: [PATCH] tests: add a test for rm -rf "/"
Hi Padraig & Eric,
sorry for the late reply - my system had a few problems after
running this test. no, just kidding ;-)
First of all, thank you for the nice hints.
>> diff --git a/tests/rm/rm-root.sh b/tests/rm/rm-root.sh
>> new file mode 100755
>> index 0000000..3882289
>> [...]
>> +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.
I wanted to be as conservative as possible in this test.
Therefore I think in the worst case we shouldn't give 'rm' more
than 1 second to destroy possibly valuable user data.
Regarding half dead: I tried it: on a system where /home is on a separate
partition, "rm -rf --one-file-system --no-preserve-root /" did not remove
more than /var/spool/mail/$USER and the files in /tmp owned by that USER.
That's not too bad. ;-)
> 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.
I've added tests like this - which partially run into the "refusing to
remove '.' or '..'" case, as you pointed out in the other mails.
Regarding Cygwin:
'rm' - the version which is currently shipped there - behaves the same
on this platform: it skips /, //, ///, //// etc.
LD_PRELOAD seems to be defined on Cygwin, too, but as 'rm' invokes
a different system call there, this test will be skipped.
On 11/23/2013 03:28 AM, Eric Blake wrote:
> Maybe you should favor 'rm -r /' rather than 'rm -rf /'.
Fixed.
> Also, probably worth testing:
>
> rm -r / a
>
> to make sure that 'a' DOES get deleted, even though we skipped over the
> / argument, and that we still get the final exit status representing
> failure to remove /.
Added a test.
Attached is the new patch.
One question (which is also as a FIXME in the patch):
for /, rm outputs the diagnostic
rm: it is dangerous to operate recursively on '/'
while for // it outputs
rm: it is dangerous to operate recursively on '//' (same as '/')
However, for ///, //// etc it output again the former message.
Why?
Thanks & have a nice day,
Berny
>From 9f262b7e16b702262230514b3eda7fd98c8117be Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <[email protected]>
Date: Mon, 25 Nov 2013 00:24:24 +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 | 248 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 249 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..cfce279
--- /dev/null
+++ b/tests/rm/rm-root.sh
@@ -0,0 +1,248 @@
+#!/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_
+
+# Pull the teeth from rm(1) by intercepting the unlinkat() system call via the
+# LD_PRELOAD environment variable. This requires shared libraries to work.
+require_gcc_shared_
+
+cat > k.c <<'EOF' || framework_failure_
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+int unlinkat (int dirfd, const char *pathname, int flags)
+{
+ /* Prove that LD_PRELOAD works: create the evidence file "x". */
+ fclose (fopen ("x", "w"));
+
+ /* Immediately terminate, unless indicated otherwise. */
+ if (! getenv("CU_TEST_SKIP_EXIT"))
+ _exit (0);
+
+ /* Pretend success. */
+ return 0;
+}
+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 -r dir" basically works.
+mkdir dir || framework_failure_
+rm -r 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 -r 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?"
+
+#-------------------------------------------------------------------------------
+# exercise_rm_rf_root: shell function to test "rm -r '/'"
+# The caller must provide the FILE to remove as well as any options
+# which should be passed to 'rm'.
+# 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 ()
+{
+ # Remove the evidence file "x"; verify that.
+ rm -f x || framework_failure_
+ test -f x && framework_failure_
+
+ local pid
+ if [ "$CU_TEST_SKIP_EXIT" = 1 ]; then
+ # Pass on this variable into 'rm's environment.
+ LD_PRELOAD=./k.so CU_TEST_SKIP_EXIT=1 rm \
+ -rv --one-file-system "$@" > out 2> err & pid=$!
+ else
+ LD_PRELOAD=./k.so rm -rv --one-file-system "$@" > out 2> err & pid=$!
+ fi
+
+ # 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
+
+ # At this point, rm(1) usually has already terminated. Kill it anyway.
+ kill -9 $pid
+
+ # Get the exit status.
+ wait $pid
+
+ return $?
+}
+
+# "rm -r /" 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 -r /" without and with the --preserve-root option.
+# Expect a non-Zero exit status.
+for opt in '' '--preserve-root'; do
+ exercise_rm_rf_root $opt '/' \
+ && 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; }
+done
+
+#-------------------------------------------------------------------------------
+# Exercise "rm -r file1 / file2".
+# Expect a non-Zero exit status representing failure to remove "/",
+# yet 'file1' and 'file2' should be removed.
+: > file1 || framework_failure_
+: > file2 || framework_failure_
+
+# Now that we know that 'rm' won't call the unlinkat() system function for "/",
+# we could probably execute it without the LD_PRELOAD'ed safety net.
+# Nevertheless, it's still better to use it for this test.
+# Tell the unlinkat() replacement function to not _exit(0) immediately
+# by setting the following variable.
+CU_TEST_SKIP_EXIT=1
+
+exercise_rm_rf_root --preserve-root file1 '/' file2 \
+ && fail=1
+
+unset CU_TEST_SKIP_EXIT
+
+cat <<EOD > out_removed
+removed 'file1'
+removed 'file2'
+EOD
+
+# The above error diagnostic should appear in 'err'.
+# Both 'file1' and 'file2' should be removed. Simply verify that in the
+# "out" file, as the replacement unlinkat() dummy did not remove them.
+# Expect the evidence file "x" to exist.
+compare out_removed 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 various synonyms of "/" including symlinks to it.
+# The error diagnostic slightly differs from that of the basic "/" case above.
+cat <<EOD > exp_same || framework_failure_
+rm: it is dangerous to operate recursively on 'FILE' (same as '/')
+rm: use --no-preserve-root to override this failsafe
+EOD
+
+# Some combinations have a trailing "." or "..". This triggers another check
+# in the code first and therefore leads to a different diagnostic. However,
+# we want to test anyway to protect against future reordering of the checks
+# in the code.
+cat <<EOD > exp_dot || framework_failure_
+rm: refusing to remove '.' or '..' directory: skipping 'FILE'
+EOD
+
+# Prepare a few symlinks to "/".
+ln -s / rootlink || framework_failure_
+ln -s rootlink rootlink2 || framework_failure_
+ln -s /bin/.. rootlink3 || framework_failure_
+
+# FIXME: for '///', '////', and more, "rm -r" outputs the error diagnostic
+# as if the bare "/" was given. For '//' not. Why?!?
+
+for file in \
+ 'rootlink/' \
+ 'rootlink2/' \
+ 'rootlink3/' \
+ '//' \
+ '//.' \
+ '/./' \
+ '/../' \
+ '/.././' \
+ '/bin/..' ; do
+
+ exercise_rm_rf_root --preserve-root "$file" \
+ && fail=1
+
+ sed "s,FILE,$file," exp_same > exp2 || framework_failure_
+ sed "s,FILE,$file," exp_dot > exp_dot2 || framework_failure_
+
+ # Check against the "refusing to remove '.' or '..'" diagnostic.
+ compare exp_dot2 err \
+ && continue
+
+ compare /dev/null out || fail=1
+ compare exp2 err || fail=1
+ test -f x && fail=1
+
+ # Do nothing more if this test failed.
+ test $fail = 1 && { cat out; cat err; Exit $fail; }
+done
+
+#-------------------------------------------------------------------------------
+# 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 exit status should be 0.
+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
--
1.8.3.1
