summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--ChangeLog16
-rw-r--r--NEWS11
-rw-r--r--src/c99-to-c89.diff14
-rw-r--r--src/remove.c136
4 files changed, 114 insertions, 63 deletions
diff --git a/ChangeLog b/ChangeLog
index 1c3b257b0..eb8763a02 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2007-10-08 Jim Meyering <meyering@redhat.com>
+
+ rm could malfunction under unusual circumstances:
+ When operating on a relative name longer than 511 bytes,
+ and (when either processing a directory that is neither writable
+ nor readable (but still searchable) or when determining whether
+ to prompt), and encountering an ENOMEM error while forming the
+ file name, rm would operate on a truncated-to-511-byte name
+ starting with "[...]" rather than the intended one.
+ * NEWS: Describe the bugs.
+ * src/remove.c: Correct two misuses of full_filename:
+ (full_filename0, xfull_filename): New functions.
+ (full_filename_): Rewrite to use full_filename0.
+ (AD_pop_and_chdir): Use xfull_filename, not full_filename.
+ (write_protected_non_symlink): Likewise.
+
2007-10-07 Jim Meyering <meyering@redhat.com>
Don't let a helper function modify errno.
diff --git a/NEWS b/NEWS
index 295ef737f..beda7f860 100644
--- a/NEWS
+++ b/NEWS
@@ -199,6 +199,17 @@ GNU coreutils NEWS -*- outline -*-
pwd and "readlink -e ." no longer fail unnecessarily when a parent
directory is unreadable.
+ rm (without -f) could prompt when it shouldn't, or fail to prompt
+ when it should, when operating on a full name longer than 511 bytes
+ and getting an ENOMEM error while trying to form the long name.
+
+ rm could mistakenly traverse into the wrong directory under unusual
+ conditions: when a full name longer than 511 bytes specifies a search-only
+ directory, and when forming that name fails with ENOMEM, rm would attempt
+ to open a truncated-to-511-byte name with the first five bytes replaced
+ with "[...]". If such a directory were to actually exist, rm would attempt
+ to remove it.
+
"rm -rf /etc/passwd" (run by non-root) now prints a diagnostic.
Before it would print nothing.
diff --git a/src/c99-to-c89.diff b/src/c99-to-c89.diff
index 8a3737247..f6c7664ed 100644
--- a/src/c99-to-c89.diff
+++ b/src/c99-to-c89.diff
@@ -1,7 +1,7 @@
diff -upr src/remove.c src/remove.c
--- src/remove.c 2007-07-23 12:56:20.000000000 +0200
+++ src/remove.c 2007-07-23 13:03:12.000000000 +0200
-@@ -257,9 +257,10 @@ pop_dir (Dirstack_state *ds)
+@@ -262,9 +262,10 @@ pop_dir (Dirstack_state *ds)
{
size_t n_lengths = obstack_object_size (&ds->len_stack) / sizeof (size_t);
size_t *length = obstack_base (&ds->len_stack);
@@ -13,7 +13,7 @@ diff -upr src/remove.c src/remove.c
assert (top_len >= 2);
/* Pop the specified length of file name. */
-@@ -391,10 +392,11 @@ AD_stack_top (Dirstack_state const *ds)
+@@ -419,10 +420,11 @@ AD_stack_top (Dirstack_state const *ds)
static void
AD_stack_pop (Dirstack_state *ds)
{
@@ -26,7 +26,7 @@ diff -upr src/remove.c src/remove.c
if (top->unremovable)
hash_free (top->unremovable);
obstack_blank (&ds->Active_dir, -(int) sizeof (struct AD_ent));
-@@ -876,6 +878,7 @@ prompt (int fd_cwd, Dirstack_state const
+@@ -904,6 +906,7 @@ prompt (int fd_cwd, Dirstack_state const
break;
}
@@ -34,7 +34,7 @@ diff -upr src/remove.c src/remove.c
char const *quoted_name = quote (full_filename (filename));
if (0 < write_protected)
-@@ -915,6 +918,7 @@ prompt (int fd_cwd, Dirstack_state const
+@@ -943,6 +946,7 @@ prompt (int fd_cwd, Dirstack_state const
: _("%s: remove %s %s? ")),
program_name, file_type (sbuf), quoted_name);
}
@@ -42,7 +42,7 @@ diff -upr src/remove.c src/remove.c
if (!yesno ())
return RM_USER_DECLINED;
-@@ -1534,6 +1538,7 @@ rm_1 (Dirstack_state *ds, char const *fi
+@@ -1562,6 +1566,7 @@ rm_1 (Dirstack_state *ds, char const *fi
return RM_ERROR;
}
@@ -50,7 +50,7 @@ diff -upr src/remove.c src/remove.c
struct stat st;
cache_stat_init (&st);
cycle_check_init (&ds->cycle_check_state);
-@@ -1556,6 +1561,7 @@ rm_1 (Dirstack_state *ds, char const *fi
+@@ -1584,6 +1589,7 @@ rm_1 (Dirstack_state *ds, char const *fi
AD_push_initial (ds);
AD_INIT_OTHER_MEMBERS ();
@@ -58,7 +58,7 @@ diff -upr src/remove.c src/remove.c
enum RM_status status = remove_entry (AT_FDCWD, ds, filename,
DT_UNKNOWN, &st, x);
if (status == RM_NONEMPTY_DIR)
-@@ -1574,6 +1580,8 @@ rm_1 (Dirstack_state *ds, char const *fi
+@@ -1602,6 +1608,8 @@ rm_1 (Dirstack_state *ds, char const *fi
ds_clear (ds);
return status;
}
diff --git a/src/remove.c b/src/remove.c
index 4f91dd420..1023048ff 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -165,6 +165,11 @@ struct dirstack_state
};
typedef struct dirstack_state Dirstack_state;
+/* A static buffer and its allocated size, these variables are used by
+ xfull_filename and full_filename to form full, relative file names. */
+static char *g_buf;
+static size_t g_n_allocated;
+
/* Like fstatat, but cache the result. If ST->st_size is -1, the
status has not been gotten yet. If less than -1, fstatat failed
with errno == -1 - ST->st_size. Otherwise, the status has already
@@ -302,83 +307,102 @@ right_justify (char *dst, size_t dst_len, const char *src, size_t src_len,
return dst_len - src_len;
}
-/* Using the global directory name obstack, create the full name FILENAME.
+/* Using the global directory name obstack, create the full name of FILENAME.
Return it in sometimes-realloc'd space that should not be freed by the
- caller. Realloc as necessary. If realloc fails, use a static buffer
- and put as long a suffix in that buffer as possible. Be careful not
- to change errno. */
+ caller. Realloc as necessary. If realloc fails, return NULL. */
-#define full_filename(Filename) full_filename_ (ds, Filename)
static char *
-full_filename_ (Dirstack_state const *ds, const char *filename)
+full_filename0 (Dirstack_state const *ds, const char *filename)
{
- static char *buf = NULL;
- static size_t n_allocated = 0;
- int saved_errno = errno;
-
size_t dir_len = obstack_object_size (&ds->dir_stack);
char *dir_name = obstack_base (&ds->dir_stack);
- size_t n_bytes_needed;
- size_t filename_len;
+ size_t filename_len = strlen (filename);
+ size_t n_bytes_needed = dir_len + filename_len + 1;
- filename_len = strlen (filename);
- n_bytes_needed = dir_len + filename_len + 1;
-
- if (n_allocated < n_bytes_needed)
+ if (g_n_allocated < n_bytes_needed)
{
- /* This code requires that realloc accept NULL as the first arg.
- This function must not use xrealloc. Otherwise, an out-of-memory
- error involving a file name to be expanded here wouldn't ever
- be issued. Use realloc and fall back on using a static buffer
- if memory allocation fails. */
- char *new_buf = realloc (buf, n_bytes_needed);
- n_allocated = n_bytes_needed;
-
+ char *new_buf = realloc (g_buf, n_bytes_needed);
if (new_buf == NULL)
- {
-#define SBUF_SIZE 512
-#define ELLIPSES_PREFIX "[...]"
- static char static_buf[SBUF_SIZE];
- bool truncated;
- size_t len;
- char *p;
-
- free (buf);
- len = right_justify (static_buf, SBUF_SIZE, filename,
- filename_len + 1, &p, &truncated);
- right_justify (static_buf, len, dir_name, dir_len, &p, &truncated);
- if (truncated)
- {
- memcpy (static_buf, ELLIPSES_PREFIX,
- sizeof (ELLIPSES_PREFIX) - 1);
- }
- errno = saved_errno;
- return p;
- }
+ return NULL;
- buf = new_buf;
+ g_buf = new_buf;
+ g_n_allocated = n_bytes_needed;
}
- if (filename_len == 1 && *filename == '.' && dir_len)
+ if (STREQ (filename, ".") && dir_len)
{
/* FILENAME is just `.' and dir_len is nonzero.
Copy the directory part, omitting the trailing slash,
and append a trailing zero byte. */
- char *p = mempcpy (buf, dir_name, dir_len - 1);
+ char *p = mempcpy (g_buf, dir_name, dir_len - 1);
*p = 0;
}
else
{
/* Copy the directory part, including trailing slash, and then
append the filename part, including a trailing zero byte. */
- memcpy (mempcpy (buf, dir_name, dir_len), filename, filename_len + 1);
- assert (strlen (buf) + 1 == n_bytes_needed);
+ memcpy (mempcpy (g_buf, dir_name, dir_len), filename, filename_len + 1);
+ assert (strlen (g_buf) + 1 == n_bytes_needed);
}
- errno = saved_errno;
+ return g_buf;
+}
+
+/* Using the global directory name obstack, create the full name of FILENAME.
+ Return it in sometimes-realloc'd space that should not be freed by the
+ caller. Realloc as necessary. If realloc fails, die. */
+
+static char *
+xfull_filename (Dirstack_state const *ds, const char *filename)
+{
+ char *buf = full_filename0 (ds, filename);
+ if (buf == NULL)
+ xalloc_die ();
return buf;
}
+/* Using the global directory name obstack, create the full name FILENAME.
+ Return it in sometimes-realloc'd space that should not be freed by the
+ caller. Realloc as necessary. If realloc fails, use a static buffer
+ and put as long a suffix in that buffer as possible. Be careful not
+ to change errno. */
+
+#define full_filename(Filename) full_filename_ (ds, Filename)
+static char *
+full_filename_ (Dirstack_state const *ds, const char *filename)
+{
+ int saved_errno = errno;
+ char *full_name = full_filename0 (ds, filename);
+ if (full_name)
+ {
+ errno = saved_errno;
+ return full_name;
+ }
+
+ {
+#define SBUF_SIZE 512
+#define ELLIPSES_PREFIX "[...]"
+ static char static_buf[SBUF_SIZE];
+ bool truncated;
+ size_t len;
+ char *p;
+ char *dir_name = obstack_base (&ds->dir_stack);
+ size_t dir_len = obstack_object_size (&ds->dir_stack);
+
+ free (g_buf);
+ len = right_justify (static_buf, SBUF_SIZE, filename,
+ strlen (filename) + 1, &p, &truncated);
+ right_justify (static_buf, len, dir_name, dir_len, &p, &truncated);
+ if (truncated)
+ {
+ memcpy (static_buf, ELLIPSES_PREFIX,
+ sizeof (ELLIPSES_PREFIX) - 1);
+ }
+ errno = saved_errno;
+ return p;
+ }
+}
+
static inline size_t
AD_stack_height (Dirstack_state const *ds)
{
@@ -512,7 +536,7 @@ AD_pop_and_chdir (DIR *dirp, int *fdp, Dirstack_state *ds)
searchable, when using Solaris' openat. Without this openat
call, tests/rm2 would fail to remove directories a/2 and a/3. */
if (fd < 0)
- fd = openat (AT_FDCWD, full_filename ("."), O_RDONLY);
+ fd = openat (AT_FDCWD, xfull_filename (ds, "."), O_RDONLY);
if (fd < 0)
{
@@ -765,13 +789,13 @@ write_protected_non_symlink (int fd_cwd,
Disadvantage: changes working directory (not reentrant) and can't
work if save_cwd fails.
- 3) if (euidaccess (full_filename (file), W_OK) == 0)
- Disadvantage: doesn't work if full_filename is too long.
+ 3) if (euidaccess (xfull_filename (file), W_OK) == 0)
+ Disadvantage: doesn't work if xfull_filename is too long.
Inefficient for very deep trees (O(Depth^2)).
4) If the full pathname is sufficiently short (say, less than
PATH_MAX or 8192 bytes, whichever is shorter):
- use method (3) (i.e., euidaccess (full_filename (file), W_OK));
+ use method (3) (i.e., euidaccess (xfull_filename (file), W_OK));
Otherwise: vfork, fchdir in the child, run euidaccess in the
child, then the child exits with a status that tells the parent
whether euidaccess succeeded.
@@ -785,7 +809,7 @@ write_protected_non_symlink (int fd_cwd,
5) If the full file name is sufficiently short (say, less than
PATH_MAX or 8192 bytes, whichever is shorter):
- use method (3) (i.e., euidaccess (full_filename (file), W_OK));
+ use method (3) (i.e., euidaccess (xfull_filename (file), W_OK));
Otherwise: look just at the file bits. Perhaps issue a warning
the first time this occurs.
@@ -801,7 +825,7 @@ write_protected_non_symlink (int fd_cwd,
if (MIN (PATH_MAX, 8192) <= file_name_len)
return - euidaccess_stat (buf, W_OK);
- if (euidaccess (full_filename (file), W_OK) == 0)
+ if (euidaccess (xfull_filename (ds, file), W_OK) == 0)
return 0;
if (errno == EACCES)
return -1;