rm (without -f) could hang unnecessarily when attempting to

remove a symlink to a file on an off-line NFS-mounted partition.
Reported by David Howells in https://bugzilla.redhat.com/124699.

(write_protected_non_symlink): New function.
Don't invoke euidaccess on symlinks.
(prompt): Use write_protected_non_symlink rather than using
euidaccess directly, being careful not to call lstat twice for a file.

diff --git a/src/remove.c b/src/remove.c
index 4bb90d08948e89971b255fb07680f725df0248c3..b1b5c0fa99d167f81d1cf76233d97b9e0af15d6a 100644 (file)
--- a/src/remove.c
+++ b/src/remove.c
@@ -539,6 +539,23 @@ is_empty_dir (char const *dir)
   return saved_errno == 0 ? true : false;
 }
 
+/* Return true if FILE is not a symbolic link and it is not writable.
+   Also return true if FILE cannot be lstat'ed.  Otherwise, return false.
+   If lstat succeeds, set *BUF_P to BUF.
+   This is to avoid calling euidaccess when FILE is a symlink.  */
+static bool
+write_protected_non_symlink (char const *file,
+                            struct stat **buf_p,
+                            struct stat *buf)
+{
+  if (lstat (file, buf) != 0)
+    return false;
+  *buf_p = buf;
+  if (S_ISLNK (buf->st_mode))
+    return false;
+  return euidaccess (file, W_OK) == -1 && errno == EACCES;
+}
+
 /* 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,
@@ -558,23 +575,30 @@ prompt (Dirstack_state const *ds, char const *filename,
        Ternary *is_dir, Ternary *is_empty)
 {
   int write_protected = 0;
+  struct stat *sbuf = NULL;
+  struct stat buf;
+
   *is_empty = T_UNKNOWN;
   *is_dir = T_UNKNOWN;
 
   if ((!x->ignore_missing_files && (x->interactive || x->stdin_tty)
-       && (write_protected = (euidaccess (filename, W_OK) && errno == EACCES)))
+       && (write_protected = write_protected_non_symlink (filename,
+                                                         &sbuf, &buf)))
       || x->interactive)
     {
-      struct stat sbuf;
-      if (lstat (filename, &sbuf))
+      if (sbuf == NULL)
        {
-         /* lstat failed.  This happens e.g., with `rm '''.  */
-         error (0, errno, _("cannot lstat %s"),
-                quote (full_filename (filename)));
-         return RM_ERROR;
+         sbuf = &buf;
+         if (lstat (filename, sbuf))
+           {
+             /* lstat failed.  This happens e.g., with `rm '''.  */
+             error (0, errno, _("cannot lstat %s"),
+                    quote (full_filename (filename)));
+             return RM_ERROR;
+           }
        }
 
-      if (S_ISDIR (sbuf.st_mode) && !x->recursive)
+      if (S_ISDIR (sbuf->st_mode) && !x->recursive)
        {
          error (0, EISDIR, _("cannot remove directory %s"),
                 quote (full_filename (filename)));
@@ -582,7 +606,7 @@ prompt (Dirstack_state const *ds, char const *filename,
        }
 
       /* Using permissions doesn't make sense for symlinks.  */
-      if (S_ISLNK (sbuf.st_mode))
+      if (S_ISLNK (sbuf->st_mode))
        {
          if ( ! x->interactive)
            return RM_OK;
@@ -593,12 +617,12 @@ prompt (Dirstack_state const *ds, char const *filename,
       {
        char const *quoted_name = quote (full_filename (filename));
 
-       *is_dir = (S_ISDIR (sbuf.st_mode) ? T_YES : T_NO);
+       *is_dir = (S_ISDIR (sbuf->st_mode) ? T_YES : T_NO);
 
        /* FIXME: use a variant of error (instead of fprintf) that doesn't
           append a newline.  Then we won't have to declare program_name in
           this file.  */
-       if (S_ISDIR (sbuf.st_mode)
+       if (S_ISDIR (sbuf->st_mode)
            && x->recursive
            && mode == PA_DESCEND_INTO_DIR
            && ((*is_empty = (is_empty_dir (filename) ? T_YES : T_NO))
@@ -618,7 +642,7 @@ prompt (Dirstack_state const *ds, char const *filename,
                     (write_protected
                      ? _("%s: remove write-protected %s %s? ")
                      : _("%s: remove %s %s? ")),
-                    program_name, file_type (&sbuf), quoted_name);
+                    program_name, file_type (sbuf), quoted_name);
          }
 
        if (!yesno ())