[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: 'rm -fr' mishandles unreadable empty directories
From: |
Jim Meyering |
Subject: |
Re: 'rm -fr' mishandles unreadable empty directories |
Date: |
Mon, 26 Jun 2006 15:02:43 +0200 |
Paul Eggert <address@hidden> wrote:
> (I found this while testing a new version of 'install-sh'.)
>
> Here's a scenario illustrating the problem.
>
> $ mkdir -m a-r -p a/b
> $ rm -fr a
> rm: cannot open directory `a/b': Permission denied
> rm: cannot remove directory `a': Directory not empty
> $ ls -ld a a/b
> drwxr-xr-x 3 eggert eggert 4096 Jun 26 00:51 a
> d-wx-wx-wx 2 eggert eggert 4096 Jun 26 00:51 a/b
>
> My reading of POSIX is that, though a diagnostic is allowed here and
> rm can exit with nonzero status because a/b is unreadable, "rm" is
> still required to rmdir both a/b and a here, so this is a
> POSIX-conformance issue.
>
> Solaris 10 'rm' gets it right: when it discovers that "a/b" is
> unreadable (openat fails) it tries rmdir("a/b"), and this works. No
> diagnostics are issued.
>
> This problem is in both coreutils 5.97 and CVS trunk.
Thanks for the report and test case.
I've fixed it on the trunk with the patch below.
Will look at the branch later.
2006-06-26 Jim Meyering <address@hidden>
* NEWS: rm no longer fails to remove an empty, unreadable directory
* src/remove.c (remove_cwd_entries): If we can't open a directory,
and the failure is not being ignored, try to remove the directory
with rmdir (aka unlinkat-with-AT_REMOVEDIR), in case it's empty.
Problem report and test case from Paul Eggert in
<http://article.gmane.org/gmane.comp.gnu.core-utils.bugs/7425>.
* tests/rm/empty-inacc: New test, for the above.
Index: NEWS
===================================================================
RCS file: /fetish/cu/NEWS,v
retrieving revision 1.386
diff -u -p -r1.386 NEWS
--- NEWS 25 Jun 2006 18:26:09 -0000 1.386
+++ NEWS 26 Jun 2006 12:10:08 -0000
@@ -72,6 +72,8 @@ GNU coreutils NEWS
rm --interactive now takes an optional argument, although the
default of using no argument still acts like -i.
+ rm no longer fails to remove an empty, unreadable directory
+
sort now reports incompatible options (e.g., -i and -n) rather than
silently ignoring one of them.
Index: src/remove.c
===================================================================
RCS file: /fetish/cu/src/remove.c,v
retrieving revision 1.150
diff -u -p -r1.150 remove.c
--- src/remove.c 25 Jun 2006 19:27:19 -0000 1.150
+++ src/remove.c 26 Jun 2006 12:39:34 -0000
@@ -1125,13 +1125,28 @@ remove_cwd_entries (DIR **dirp,
x, errno, subdir_sb, ds, NULL);
if (subdir_dirp == NULL)
{
+ status = RM_ERROR;
+
/* CAUTION: this test and diagnostic are identical to
those following the other use of fd_to_subdirp. */
- if (errno != ENOENT || !x->ignore_missing_files)
- error (0, errno,
- _("cannot remove %s"), quote (full_filename (f)));
- AD_mark_as_unremovable (ds, f);
- status = RM_ERROR;
+ if (errno == ENOENT && x->ignore_missing_files)
+ {
+ /* With -f, don't report "file not found". */
+ }
+ else
+ {
+ /* Upon fd_to_subdirp failure, try to remove F directly,
+ in case it's just an empty directory. */
+ int saved_errno = errno;
+ if (unlinkat (dirfd (*dirp), f, AT_REMOVEDIR) == 0)
+ status = RM_OK;
+ else
+ error (0, saved_errno,
+ _("cannot remove %s"), quote (full_filename (f)));
+ }
+
+ if (status == RM_ERROR)
+ AD_mark_as_unremovable (ds, f);
break;
}
@@ -1214,6 +1229,9 @@ remove_dir (int fd_cwd, Dirstack_state *
{
/* If fd_to_subdirp fails due to permissions, then try to
remove DIR via rmdir, in case it's just an empty directory. */
+ /* This use of rmdir just works, at least in the sole test I
+ have that exercises this code, but it'll soon go, to be
+ replaced by a use of unlinkat-with-AT_REMOVEDIR. */
if (rmdir (dir) == 0)
return RM_OK;
Index: tests/rm/empty-inacc
===================================================================
RCS file: /fetish/cu/tests/rm/empty-inacc,v
retrieving revision 1.1
diff -u -p -r1.1 empty-inacc
--- tests/rm/empty-inacc 11 Feb 2006 18:03:29 -0000 1.1
+++ tests/rm/empty-inacc 26 Jun 2006 12:12:19 -0000
@@ -18,6 +18,10 @@ mkdir -p $tmp || framework_failure=1
cd $tmp || framework_failure=1
mkdir -m0 inacc || framework_failure=1
+# Also exercise the different code path that's taken for a directory
+# that is empty (hence removable) and unreadable.
+mkdir -m a-r -p a/unreadable
+
if test $framework_failure = 1; then
echo "$0: failure in testing framework" 1>&2
(exit 1); exit 1
@@ -29,4 +33,8 @@ fail=0
rm -rf inacc || fail=1
test -d inacc && fail=1
+# This would fail for e.g., coreutils-5.97.
+rm -rf a || fail=1
+test -d a && fail=1
+
(exit $fail); exit $fail