1
0
mirror of git://git.sv.gnu.org/coreutils.git synced 2026-04-18 17:56:54 +02:00

rm: fix diagnostics on I/O error

I ran into this problem when attempting to recursively
remove a directory in a filesystem on flaky hardware.
Although the underlying readdir syscall failed with errno == EIO,
rm issued no diagnostic about the I/O error.

Without this patch I see this behavior:

  $ rm -fr baddir
  rm: cannot remove 'baddir': Directory not empty
  $ rm -ir baddir
  rm: descend into directory 'baddir'? y
  rm: remove directory 'baddir'? y
  rm: cannot remove 'baddir': Directory not empty

With this patch I see the following behavior, which
lets the user know about the I/O error when rm tries
to read baddir's directory entries:

  $ rm -fr baddir
  rm: cannot remove 'baddir': Input/output error
  $ rm -ir baddir
  rm: cannot remove 'baddir': Input/output error

* src/remove.c (Ternary): Remove.  All uses removed.
(get_dir_status): New static function.
(prompt): Last arg is now directory status, not ternary.
Return RM_USER_ACCEPTED if user explicitly accepted.
All uses changed.
Report any significant error in directory status right away.
(prompt, rm_fts): Use get_dir_status to get directory status lazily.
(excise): Treat any FTS_DNR errno as being more descriptive, not
just EPERM and EACCESS.  For example, EIO is more descriptive.
(rm_fts): Distinguish more clearly between explicit and implied
user OK.
* src/remove.h (RM_USER_ACCEPTED): New constant.
(VALID_STATUS): Treat it as valid.
* src/system.h (is_empty_dir): Remove, replacing with ...
(directory_status): ... this more-general function.
All uses changed.  Avoid undefined behavior of looking at
a non-null readdir pointer after corresponding closedir.
* tests/rm/rm-readdir-fail.sh: Adjust test of internals
to match current behavior.
This commit is contained in:
Paul Eggert
2022-09-21 14:05:49 -07:00
parent 5a14ccad48
commit eb7841426c
5 changed files with 58 additions and 55 deletions

View File

@@ -33,14 +33,6 @@
#include "xfts.h"
#include "yesno.h"
enum Ternary
{
T_UNKNOWN = 2,
T_NO,
T_YES
};
typedef enum Ternary Ternary;
/* The prompt function may be called twice for a given directory.
The first time, we ask whether to descend into it, and the
second time, we ask whether to remove it. */
@@ -168,9 +160,23 @@ write_protected_non_symlink (int fd_cwd,
}
}
/* Prompt whether to remove FILENAME (ent->, if required via a combination of
/* Return the status of the directory identified by FTS and ENT.
This is -1 if the directory is empty, 0 if it is nonempty,
and a positive error number if there was trouble determining the status,
e.g., it is not a directory, or permissions problems, or I/O errors.
Use *DIR_STATUS is a cache for the status. */
static int
get_dir_status (FTS const *fts, FTSENT const *ent, int *dir_status)
{
if (*dir_status < -1)
*dir_status = directory_status (fts->fts_cwd_fd, ent->fts_accpath);
return *dir_status;
}
/* Prompt whether to remove FILENAME, if required via a combination of
the options specified by X and/or file attributes. If the file may
be removed, return RM_OK. If the user declines to remove the file,
be removed, return RM_OK or RM_USER_ACCEPTED, the latter if the user
was prompted and accepted. If the user declines to remove the file,
return RM_USER_DECLINED. If not ignoring missing files and we
cannot lstat FILENAME, then return RM_ERROR.
@@ -178,20 +184,16 @@ write_protected_non_symlink (int fd_cwd,
Depending on MODE, ask whether to 'descend into' or to 'remove' the
directory FILENAME. MODE is ignored when FILENAME is not a directory.
Set *IS_EMPTY_P to T_YES if FILENAME is an empty directory, and it is
appropriate to try to remove it with rmdir (e.g. recursive mode).
Don't even try to set *IS_EMPTY_P when MODE == PA_REMOVE_DIR. */
Use and update *DIR_STATUS as needed, via the conventions of
get_dir_status. */
static enum RM_status
prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
struct rm_options const *x, enum Prompt_action mode,
Ternary *is_empty_p)
int *dir_status)
{
int fd_cwd = fts->fts_cwd_fd;
char const *full_name = ent->fts_path;
char const *filename = ent->fts_accpath;
if (is_empty_p)
*is_empty_p = T_UNKNOWN;
struct stat st;
struct stat *sbuf = &st;
cache_stat_init (sbuf);
@@ -199,13 +201,6 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
int dirent_type = is_dir ? DT_DIR : DT_UNKNOWN;
int write_protected = 0;
bool is_empty = false;
if (is_empty_p)
{
is_empty = is_empty_dir (fd_cwd, filename);
*is_empty_p = is_empty ? T_YES : T_NO;
}
/* When nonzero, this indicates that we failed to remove a child entry,
either because the user declined an interactive prompt, or due to
some other failure, like permissions. */
@@ -257,10 +252,12 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
/* Unless we're either deleting directories or deleting
recursively, we want to raise an EISDIR error rather than
prompting the user */
if ( ! (x->recursive || (x->remove_empty_directories && is_empty)))
if ( ! (x->recursive
|| (x->remove_empty_directories
&& get_dir_status (fts, ent, dir_status) < 0)))
{
write_protected = -1;
wp_errno = EISDIR;
wp_errno = *dir_status <= 0 ? EISDIR : *dir_status;
}
break;
}
@@ -276,12 +273,17 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
/* Issue the prompt. */
if (dirent_type == DT_DIR
&& mode == PA_DESCEND_INTO_DIR
&& !is_empty)
&& get_dir_status (fts, ent, dir_status) == 0)
fprintf (stderr,
(write_protected
? _("%s: descend into write-protected directory %s? ")
: _("%s: descend into directory %s? ")),
program_name, quoted_name);
else if (0 < *dir_status)
{
error (0, *dir_status, _("cannot remove %s"), quoted_name);
return RM_ERROR;
}
else
{
if (cache_fstatat (fd_cwd, filename, sbuf, AT_SYMLINK_NOFOLLOW) != 0)
@@ -302,8 +304,7 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
program_name, file_type (sbuf), quoted_name);
}
if (!yesno ())
return RM_USER_DECLINED;
return yesno () ? RM_USER_ACCEPTED : RM_USER_DECLINED;
}
return RM_OK;
}
@@ -405,13 +406,12 @@ excise (FTS *fts, FTSENT *ent, struct rm_options const *x, bool is_dir)
/* When failing to rmdir an unreadable directory, we see errno values
like EISDIR or ENOTDIR (or, on Solaris 10, EEXIST), but they would be
meaningless in a diagnostic. When that happens and the errno value
from the failed open is EPERM or EACCES, use the earlier, more
meaningless in a diagnostic. When that happens, use the earlier, more
descriptive errno value. */
if (ent->fts_info == FTS_DNR
&& (errno == ENOTEMPTY || errno == EISDIR || errno == ENOTDIR
|| errno == EEXIST)
&& (ent->fts_errno == EPERM || ent->fts_errno == EACCES))
&& ent->fts_errno != 0)
errno = ent->fts_errno;
error (0, errno, _("cannot remove %s"), quoteaf (ent->fts_path));
mark_ancestor_dirs (ent);
@@ -427,12 +427,14 @@ excise (FTS *fts, FTSENT *ent, struct rm_options const *x, bool is_dir)
static enum RM_status
rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
{
int dir_status = -2;
switch (ent->fts_info)
{
case FTS_D: /* preorder directory */
if (! x->recursive
&& !(x->remove_empty_directories
&& is_empty_dir (fts->fts_cwd_fd, ent->fts_accpath)))
&& get_dir_status (fts, ent, &dir_status) < 0))
{
/* This is the first (pre-order) encounter with a directory
that we cannot delete.
@@ -507,11 +509,10 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
}
{
Ternary is_empty_directory;
enum RM_status s = prompt (fts, ent, true /*is_dir*/, x,
PA_DESCEND_INTO_DIR, &is_empty_directory);
PA_DESCEND_INTO_DIR, &dir_status);
if (s == RM_OK && is_empty_directory == T_YES)
if (s == RM_USER_ACCEPTED && dir_status == -1)
{
/* When we know (from prompt when in interactive mode)
that this is an empty directory, don't prompt twice. */
@@ -520,7 +521,7 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
fts_skip_tree (fts, ent);
}
if (s != RM_OK)
if (! (s == RM_OK || s == RM_USER_ACCEPTED))
{
mark_ancestor_dirs (ent);
fts_skip_tree (fts, ent);
@@ -553,8 +554,9 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
}
bool is_dir = ent->fts_info == FTS_DP || ent->fts_info == FTS_DNR;
enum RM_status s = prompt (fts, ent, is_dir, x, PA_REMOVE_DIR, NULL);
if (s != RM_OK)
enum RM_status s = prompt (fts, ent, is_dir, x, PA_REMOVE_DIR,
&dir_status);
if (! (s == RM_OK || s == RM_USER_ACCEPTED))
return s;
return excise (fts, ent, x, is_dir);
}

View File

@@ -79,13 +79,15 @@ enum RM_status
{
/* These must be listed in order of increasing seriousness. */
RM_OK = 2,
RM_USER_ACCEPTED,
RM_USER_DECLINED,
RM_ERROR,
RM_NONEMPTY_DIR
};
# define VALID_STATUS(S) \
((S) == RM_OK || (S) == RM_USER_DECLINED || (S) == RM_ERROR)
((S) == RM_OK || (S) == RM_USER_ACCEPTED || (S) == RM_USER_DECLINED \
|| (S) == RM_ERROR)
# define UPDATE_STATUS(S, New_value) \
do \

View File

@@ -101,8 +101,7 @@ ignorable_failure (int error_number, char const *dir)
return (ignore_fail_on_non_empty
&& (errno_rmdir_non_empty (error_number)
|| (errno_may_be_non_empty (error_number)
&& ! is_empty_dir (AT_FDCWD, dir)
&& errno == 0 /* definitely non empty */)));
&& directory_status (AT_FDCWD, dir) == 0)));
}
/* Remove any empty parent directories of DIR.

View File

@@ -287,37 +287,36 @@ readdir_ignoring_dot_and_dotdot (DIR *dirp)
}
}
/* Return true if DIR is determined to be an empty directory.
Return false with ERRNO==0 if DIR is a non empty directory.
Return false if not able to determine if directory empty. */
static inline bool
is_empty_dir (int fd_cwd, char const *dir)
/* Return -1 if DIR is an empty directory,
0 if DIR is a nonempty directory,
and a positive error number if there was trouble determining
whether DIR is an empty or nonempty directory. */
static inline int
directory_status (int fd_cwd, char const *dir)
{
DIR *dirp;
struct dirent const *dp;
bool no_direntries;
int saved_errno;
int fd = openat (fd_cwd, dir,
(O_RDONLY | O_DIRECTORY
| O_NOCTTY | O_NOFOLLOW | O_NONBLOCK));
if (fd < 0)
return false;
return errno;
dirp = fdopendir (fd);
if (dirp == NULL)
{
saved_errno = errno;
close (fd);
return false;
return saved_errno;
}
errno = 0;
dp = readdir_ignoring_dot_and_dotdot (dirp);
no_direntries = !readdir_ignoring_dot_and_dotdot (dirp);
saved_errno = errno;
closedir (dirp);
errno = saved_errno;
if (dp != NULL)
return false;
return saved_errno == 0 ? true : false;
return no_direntries && saved_errno == 0 ? -1 : saved_errno;
}
/* Factor out some of the common --help and --version processing code. */

View File

@@ -112,6 +112,7 @@ done
# (with ENOENT in this case but it could be anything).
cat <<EOF > exp
rm: cannot remove 'dir'
Failed to get dirent
rm: traversal failed: dir
EOF
sed 's/\(rm:.*\):.*/\1/' errt > err || framework_failure_