aboutsummaryrefslogtreecommitdiff
path: root/libstdc++-v3/src/filesystem/dir-common.h
diff options
context:
space:
mode:
authorJonathan Wakely <jwakely@redhat.com>2022-02-01 22:04:46 +0000
committerJonathan Wakely <jwakely@redhat.com>2022-02-04 19:51:26 +0000
commitebf6175464768983a2d8c82c2d47771ee89192b8 (patch)
treee4d327aea589d6426109ca76aadffe144b8433cd /libstdc++-v3/src/filesystem/dir-common.h
parentb28b92bc008776c8b517841f99ba6a31bf7751d2 (diff)
downloadgcc-ebf6175464768983a2d8c82c2d47771ee89192b8.zip
gcc-ebf6175464768983a2d8c82c2d47771ee89192b8.tar.gz
gcc-ebf6175464768983a2d8c82c2d47771ee89192b8.tar.bz2
libstdc++: Fix filesystem::remove_all races [PR104161]
This fixes the remaining filesystem::remove_all race condition by using POSIX openat to recurse into sub-directories and using POSIX unlinkat to remove files. This avoids the remaining race where the directory being removed is replaced with a symlink after the directory has been opened, so that the filesystem::remove("subdir/file") resolves to "target/file" instead, because "subdir" has been removed and replaced with a symlink. The previous patch only fixed the case where the directory was replaced with a symlink before we tried to open it, but it still used the full (potentially compromised) path as an argument to filesystem::remove. The first part of the fix is to use openat when recursing into a sub-directory with recursive_directory_iterator. This means that opening "dir/subdir" uses the file descriptor for "dir", and so is sure to open "dir/subdir" and not "symlink/subdir". (The previous patch to use O_NOFOLLOW already ensured we won't open "dir/symlink/" here.) The second part of the fix is to use unlinkat for the remove_all operation. Previously we used a directory_iterator to get the name of each file in a directory and then used filesystem::remove(iter->path()) on that name. This meant that any checks (e.g. O_NOFOLLOW) done by the iterator could be invalidated before the remove operation on that pathname. The directory iterator contains an open DIR stream, which we can use to obtain a file descriptor to pass to unlinkat. This ensures that the file being deleted really is contained within the directory we're iterating over, rather than using a pathname that could resolve to some other file. The filesystem::remove_all function previously used a (non-recursive) filesystem::directory_iterator for each directory, and called itself recursively for sub-directories. The new implementation uses a single filesystem::recursive_directory_iterator object, and calls a new __erase member function on that iterator. That new __erase member function does the actual work of removing a file (or a directory after its contents have been iterated over and removed) using unlinkat. That means we don't need to expose the DIR stream or its file descriptor to the remove_all function, it's still encapuslated by the iterator class. It would be possible to add a __rewind member to directory iterators too, to call rewinddir after each modification to the directory. That would make it more likely for filesystem::remove_all to successfully remove everything even if files are being written to the directory tree while removing it. It's unclear if that is actually prefereable, or if it's better to fail and report an error at the first opportunity. The necessary APIs (openat, unlinkat, fdopendir, dirfd) are defined in POSIX.1-2008, and in Glibc since 2.10. But if the target doesn't provide them, the original code (with race conditions) is still used. This also reduces the number of small memory allocations needed for std::filesystem::remove_all, because we do not store the full path to every directory entry that is iterated over. The new filename_only option means we only store the filename in the directory entry, as that is all we need in order to use openat or unlinkat. Finally, rather than duplicating everything for the Filesystem TS, the std::experimental::filesystem::remove_all implementation now just calls std::filesystem::remove_all to do the work. libstdc++-v3/ChangeLog: PR libstdc++/104161 * acinclude.m4 (GLIBCXX_CHECK_FILESYSTEM_DEPS): Check for dirfd and unlinkat. * config.h.in: Regenerate. * configure: Regenerate. * include/bits/fs_dir.h (recursive_directory_iterator): Declare remove_all overloads as friends. (recursive_directory_iterator::__erase): Declare new member function. * include/bits/fs_fwd.h (remove, remove_all): Declare. * src/c++17/fs_dir.cc (_Dir): Add filename_only parameter to constructor. Pass file descriptor argument to base constructor. (_Dir::dir_and_pathname, _Dir::open_subdir, _Dir::do_unlink) (_Dir::unlink, _Dir::rmdir): Define new member functions. (directory_iterator): Pass filename_only argument to _Dir constructor. (recursive_directory_iterator::_Dir_stack): Adjust constructor parameters to take a _Dir rvalue instead of creating one. (_Dir_stack::orig): Add data member for storing original path. (_Dir_stack::report_error): Define new member function. (__directory_iterator_nofollow): Move here from dir-common.h and fix value to be a power of two. (__directory_iterator_filename_only): Define new constant. (recursive_directory_iterator): Construct _Dir object and move into _M_dirs stack. Pass skip_permission_denied argument to first advance call. (recursive_directory_iterator::increment): Use _Dir::open_subdir. (recursive_directory_iterator::__erase): Define new member function. * src/c++17/fs_ops.cc (ErrorReporter, do_remove_all): Remove. (fs::remove_all): Use new recursive_directory_iterator::__erase member function. * src/filesystem/dir-common.h (_Dir_base): Add int parameter to constructor and use openat to implement nofollow semantics. (_Dir_base::fdcwd, _Dir_base::set_close_on_exec, _Dir_base::openat): Define new member functions. (__directory_iterator_nofollow): Move to fs_dir.cc. * src/filesystem/dir.cc (_Dir): Pass file descriptor argument to base constructor. (_Dir::dir_and_pathname, _Dir::open_subdir): Define new member functions. (recursive_directory_iterator::_Dir_stack): Adjust constructor parameters to take a _Dir rvalue instead of creating one. (recursive_directory_iterator): Check for new nofollow option. Construct _Dir object and move into _M_dirs stack. Pass skip_permission_denied argument to first advance call. (recursive_directory_iterator::increment): Use _Dir::open_subdir. * src/filesystem/ops.cc (fs::remove_all): Use C++17 remove_all.
Diffstat (limited to 'libstdc++-v3/src/filesystem/dir-common.h')
-rw-r--r--libstdc++-v3/src/filesystem/dir-common.h145
1 files changed, 97 insertions, 48 deletions
diff --git a/libstdc++-v3/src/filesystem/dir-common.h b/libstdc++-v3/src/filesystem/dir-common.h
index 4bfdae4..ee4f33b 100644
--- a/libstdc++-v3/src/filesystem/dir-common.h
+++ b/libstdc++-v3/src/filesystem/dir-common.h
@@ -34,11 +34,11 @@
# ifdef _GLIBCXX_HAVE_SYS_TYPES_H
# include <sys/types.h>
# endif
-# include <dirent.h>
-#endif
-#ifdef _GLIBCXX_HAVE_FCNTL_H
-# include <fcntl.h> // O_NOFOLLOW, O_DIRECTORY
-# include <unistd.h> // close
+# include <dirent.h> // opendir, readdir, fdopendir, dirfd
+# ifdef _GLIBCXX_HAVE_FCNTL_H
+# include <fcntl.h> // open, openat, fcntl, AT_FDCWD, O_NOFOLLOW etc.
+# include <unistd.h> // close, unlinkat
+# endif
#endif
namespace std _GLIBCXX_VISIBILITY(default)
@@ -75,42 +75,32 @@ inline int closedir(DIR*) { return -1; }
namespace posix = __gnu_posix;
+inline bool
+is_permission_denied_error(int e)
+{
+ if (e == EACCES)
+ return true;
+#ifdef __APPLE__
+ if (e == EPERM) // See PR 99533
+ return true;
+#endif
+ return false;
+}
+
struct _Dir_base
{
_Dir_base(posix::DIR* dirp = nullptr) : dirp(dirp) { }
// If no error occurs then dirp is non-null,
- // otherwise null (even if an EACCES error is ignored).
- _Dir_base(const posix::char_type* pathname, bool skip_permission_denied,
- [[maybe_unused]] bool nofollow, error_code& ec) noexcept
- : dirp(nullptr)
+ // otherwise null (even if a permission denied error is ignored).
+ _Dir_base(int fd, const posix::char_type* pathname,
+ bool skip_permission_denied, bool nofollow,
+ error_code& ec) noexcept
+ : dirp(_Dir_base::openat(fd, pathname, nofollow))
{
-#if defined O_RDONLY && O_NOFOLLOW && defined O_DIRECTORY && defined O_CLOEXEC \
- && defined _GLIBCXX_HAVE_FDOPENDIR && !_GLIBCXX_FILESYSTEM_IS_WINDOWS
- if (nofollow)
- {
- // Do not allow opening a symlink (used by filesystem::remove_all)
- const int flags = O_RDONLY | O_NOFOLLOW | O_DIRECTORY | O_CLOEXEC;
- int fd = ::open(pathname, flags);
- if (fd != -1)
- {
- if ((dirp = ::fdopendir(fd)))
- {
- ec.clear();
- return;
- }
- }
- if (errno == EACCES && skip_permission_denied)
- ec.clear();
- else
- ec.assign(errno, std::generic_category());
- return;
- }
-#endif
-
- if ((dirp = posix::opendir(pathname)))
+ if (dirp)
ec.clear();
- else if (errno == EACCES && skip_permission_denied)
+ else if (is_permission_denied_error(errno) && skip_permission_denied)
ec.clear();
else
ec.assign(errno, std::generic_category());
@@ -153,6 +143,16 @@ struct _Dir_base
}
}
+ static constexpr int
+ fdcwd() noexcept
+ {
+#ifdef AT_FDCWD
+ return AT_FDCWD;
+#else
+ return -1; // Use invalid fd if AT_FDCWD isn't supported.
+#endif
+ }
+
static bool is_dot_or_dotdot(const char* s) noexcept
{ return !strcmp(s, ".") || !strcmp(s, ".."); }
@@ -161,20 +161,71 @@ struct _Dir_base
{ return !wcscmp(s, L".") || !wcscmp(s, L".."); }
#endif
- posix::DIR* dirp;
-};
-
-inline bool
-is_permission_denied_error(int e)
-{
- if (e == EACCES)
- return true;
-#ifdef __APPLE__
- if (e == EPERM) // See PR 99533
+ // Set the close-on-exec flag if not already done via O_CLOEXEC.
+ static bool
+ set_close_on_exec([[maybe_unused]] int fd)
+ {
+#if ! defined O_CLOEXEC && defined FD_CLOEXEC
+ int flags = ::fcntl(fd, F_GETFD);
+ if (flags == -1 || ::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1)
+ return false;
+#endif
return true;
+ }
+
+ static ::DIR*
+ openat(int fd, const posix::char_type* pathname, bool nofollow)
+ {
+#if _GLIBCXX_HAVE_FDOPENDIR && defined O_RDONLY && defined O_DIRECTORY \
+ && ! _GLIBCXX_FILESYSTEM_IS_WINDOWS
+
+ // Any file descriptor we open here should be closed on exec.
+#ifdef O_CLOEXEC
+ constexpr int close_on_exec = O_CLOEXEC;
+#else
+ constexpr int close_on_exec = 0;
+#endif
+
+ int flags = O_RDONLY | O_DIRECTORY | close_on_exec;
+
+ // Directory iterators are vulnerable to race conditions unless O_NOFOLLOW
+ // is supported, because a directory could be replaced with a symlink after
+ // checking is_directory(symlink_status(f)). O_NOFOLLOW avoids the race.
+#ifdef O_NOFOLLOW
+ if (nofollow)
+ flags |= O_NOFOLLOW;
+#else
+ nofollow = false;
+#endif
+
+
+#ifdef AT_FDCWD
+ fd = ::openat(fd, pathname, flags);
+#else
+ // If we cannot use openat, there's no benefit to using posix::open unless
+ // we will use O_NOFOLLOW, so just use the simpler posix::opendir.
+ if (!nofollow)
+ return posix::opendir(pathname);
+
+ fd = ::open(pathname, flags);
#endif
- return false;
-}
+
+ if (fd == -1)
+ return nullptr;
+ if (set_close_on_exec(fd))
+ if (::DIR* dirp = ::fdopendir(fd))
+ return dirp;
+ int err = errno;
+ ::close(fd);
+ errno = err;
+ return nullptr;
+#else
+ return posix::opendir(pathname);
+#endif
+ }
+
+ posix::DIR* dirp;
+};
} // namespace filesystem
@@ -211,8 +262,6 @@ get_file_type(const std::filesystem::__gnu_posix::dirent& d [[gnu::unused]])
#endif
}
-constexpr directory_options __directory_iterator_nofollow{99};
-
_GLIBCXX_END_NAMESPACE_FILESYSTEM
_GLIBCXX_END_NAMESPACE_VERSION