From 689186b88ccf025664ca24ac8efa68699f12d85d Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sun, 7 Oct 2007 22:58:29 +0200 Subject: 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. --- src/remove.c | 136 +++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 80 insertions(+), 56 deletions(-) (limited to 'src/remove.c') 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; -- cgit v1.2.3-54-g00ecf