GNU bug report logs - #15926
RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call

Previous Next

Full log

View this message in rfc822 format

From: Pádraig Brady <P <at> draigBrady.com>
To: Bernhard Voelker <mail <at> bernhard-voelker.de>
Cc: 15926 <15926 <at> debbugs.gnu.org>
Subject: bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call
Date: Wed, 27 Nov 2013 03:14:48 +0000

On 11/26/2013 11:08 PM, Bernhard Voelker wrote:
> +#-------------------------------------------------------------------------------
> +# Exercise "rm -r /" without and with the --preserve-root option.
> +# Also exercise the synonyms '///' and '////' which would normally go into
> +# the 'synonyms' test category below; but due to the way gnulib's fts_open()
> +# performs trimming of trailing slashes ("///" -> "/"), the error diagnostic
> +# is the same as for a bare "/", see the ROOT_DEV_INO_WARN macro.
> +# Expect a non-Zero exit status.
> +for opts in '/' '--preserve-root /' '///' '////'; do

This relies on that fts behavior, which I'm a bit wary about coupling to.
I'd be inclined to lump all the synonyms together and just
be less stringent in the error message we compare there.

> +  exercise_rm_r_root $opts \
> +    && fail=1

So this call has 2s to work or we fail the test.
That seems too short from experience of running many tests
in parallel on slow/loaded systems.
This is at odds of course with trying to get rm not run
for too long. As I said I wouldn't bother with that constraint,
and I can't think how to avoid false failures doing that.

> +  # 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_r_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 -sr /       rootlink3 || framework_failure_
> +
> +for file in    \
> +  'rootlink/'  \
> +  'rootlink2/' \
> +  'rootlink3/' \
> +  '//'         \
> +  '//.'        \
> +  '/./'        \
> +  '/../'       \
> +  '/.././'     \
> +  '/bin/..' ; do

I'm still worried about /bin being always present.
How about: test -d "$file" || continue

> +
> +  exercise_rm_r_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.
> +# Use the option --interactive=never to bypass the following prompt:
> +#   "rm: descend into write-protected directory '/'?"
> +exercise_rm_r_root  --interactive=never --no-preserve-root '/' \
> +  || fail=1

/me doubles checks unlinkat() is the only function that should be called,
and the previous checks guarantee this... OK :)

thanks!
Pádraig.

p.s. something that might be worth looking at in future,
would be to use runcon to restrict the process using selinux
if in selinux enforcing mode.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.