From: Nishant Nayan <nishant.na...@oracle.com> When removing a directory fails for some reason, and that directory is empty, the rm_fts code gets the return value of the excise call confused with the return value of its earlier call to prompt, causing fts_skip_tree to be called repeatedly and the next file that rm would otherwise have deleted to survive.
[nca: added commit log, test] * src/remove.c (rm_fts): Only ever carry out one branch of the rm -i failure path for directories, never both (a user cannot both answer y and n to an rm -i prompt). * tests/rm/empty-immutable-skip.sh: New root-only test. * tests/local.mk: Add it. * NEWS: Mention the bug fix. --- NEWS | 3 +++ src/remove.c | 3 +-- tests/local.mk | 1 + tests/rm/empty-immutable-skip.sh | 46 ++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 2 deletions(-) create mode 100755 tests/rm/empty-immutable-skip.sh diff --git a/NEWS b/NEWS index 61b5d42f6..ce40fc4ff 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,9 @@ GNU coreutils NEWS -*- outline -*- invalid combinations of case character classes. [bug introduced in coreutils-8.6] + rm no longer skips an extra file when the removal of an empty directory fails. + [introduced by the rewrite to make rm use fts in coreutils-8.0] + ** Changes in behavior cp and install now default to copy-on-write (COW) if available. diff --git a/src/remove.c b/src/remove.c index 2d40c55cd..1150cf179 100644 --- a/src/remove.c +++ b/src/remove.c @@ -508,8 +508,7 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x) s = excise (fts, ent, x, true); fts_skip_tree (fts, ent); } - - if (s != RM_OK) + else if (s != RM_OK) { mark_ancestor_dirs (ent); fts_skip_tree (fts, ent); diff --git a/tests/local.mk b/tests/local.mk index e1c4675c2..297760cc5 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -136,6 +136,7 @@ all_root_tests = \ tests/rm/no-give-up.sh \ tests/rm/one-file-system.sh \ tests/rm/read-only.sh \ + tests/rm/empty-immutable-skip.sh \ tests/tail-2/append-only.sh \ tests/tail-2/end-of-device.sh \ tests/touch/now-owned-by-other.sh diff --git a/tests/rm/empty-immutable-skip.sh b/tests/rm/empty-immutable-skip.sh new file mode 100755 index 000000000..e31417073 --- /dev/null +++ b/tests/rm/empty-immutable-skip.sh @@ -0,0 +1,46 @@ +#!/bin/sh +# Ensure that rm does not skip extra files after hitting an empty immutable dir. +# Requires root access to do chattr +i, as well as an ext[23] or xfs file system + +# Copyright (C) 2006-2020 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 <https://www.gnu.org/licenses/>. + +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ tail +require_root_ + +# These simple one-file operations are expected to work even in the +# presence of this bug, and we need them to set up the rest of the test. +chattr_i_works=1 +touch f +chattr +i f 2>/dev/null || chattr_i_works=0 +rm f 2>/dev/null +test -f f || chattr_i_works=0 +chattr -i f 2>/dev/null || chattr_i_works=0 +rm f 2>/dev/null || chattr_i_works=0 +test -f f && chattr_i_works=0 + +if test $chattr_i_works = 0; then + skip_ "chattr +i doesn't work on this file system" +fi + +mkdir empty +touch x y +chattr +i empty +rm -rf empty x y +{ test -f x || test -f y || test -f z; } && fail=1 +chattr -i empty + +Exit $fail -- 2.29.0.249.g249b51256f