3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2025-03-09 14:06:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use readdir() instead of readdir_r() on all (POSIX) platforms

Browse Source 
readdir_r() is a thread-safe version of readdir(), although readdir() is
not particularly thread-unsafe with most usage. The dirent* returned by
readdir() can only be invalidated by a subsequent readdir() or
closedir() on the same DIR*. In typical usage, where a returned dirent*
is used exclusively within a loop around readdir() and is not expected
to outlive that loop, there are no lifetime or thread-safety issues with
the use of readdir().

readdir_r() may be harmful in certain situations because its buffer is
not explicitly sized, and attempts to provide a suitably sized buffer
dynamically (which, incidentally, our code did not do) are subject to a
race condition.

https://elliotth.blogspot.com/2012/10/how-not-to-use-readdirr3.html
https://womble.decadent.org.uk/readdir_r-advisory.html

glibc has already deprecated readdir_r(), and all Linux (including
Android) code was already using readdir(). This change eliminates
variant codepaths. It delegates buffer sizing (which we weren’t doing
correctly) to the C library, which also has more options at its disposal
to avoid races in sizing that buffer.

Change-Id: I4fca8948454116360180ad0017f226d06727ef81
Reviewed-on: https://chromium-review.googlesource.com/705756
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
This commit is contained in:
Mark Mentovai 2017-10-06 15:11:59 -04:00
parent 2633708f84
commit 1abaf22e28
2 changed files with 8 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
handler/mac/file_limit_annotation.cc
View File

@ -52,7 +52,7 @@ std::string FormatFromSysctl(int rv, const int* value, const size_t* size) {
// Counts the number of open file descriptors in the process and returns it as a
// string. This /dev/fd and the value returned will include the open file
// descriptor for that directory. If opendir() fails, the returned string will
// be "E" followed by the error number. If readdir_r() fails, it will be "R"
// be "E" followed by the error number. If readdir() fails, it will be "R"
// followed by the error number.
std::string CountOpenFileDescriptors() {
DIR* dir = opendir("/dev/fd");
@ -62,11 +62,10 @@ std::string CountOpenFileDescriptors() {
ScopedDIR dir_owner(dir);
dirent entry;
dirent* result;
dirent* entry;
int count = 0;
while ((errno = readdir_r(dir, &entry, &result)) == 0 && result != nullptr) {
const char* entry_name = &(*result->d_name);
while ((errno = 0, entry = readdir(dir)) != nullptr) {
const char* entry_name = entry->d_name;
if (strcmp(entry_name, ".") == 0 || strcmp(entry_name, "..") == 0) {
continue;
}

19
util/posix/close_multiple.cc
View File

@ -94,20 +94,9 @@ bool CloseMultipleNowOrOnExecUsingFDDir(int fd, int preserve_fd) {
return false;
}
dirent* result;
#if defined(OS_LINUX)
// readdir_r() is deprecated as of glibc 2.24. See
// https://sourceware.org/bugzilla/show_bug.cgi?id=19056 and
// https://git.kernel.org/cgit/docs/man-pages/man-pages.git/commit?id=0c52f6d623636a61eacd0f7b7a3bb942793a2a05.
static constexpr char kReaddirName[] = "readdir";
while ((errno = 0, result = readdir(dir)) != nullptr)
#else
static constexpr char kReaddirName[] = "readdir_r";
dirent entry;
while ((errno = readdir_r(dir, &entry, &result)) == 0 && result != nullptr)
#endif
{
const char* entry_name = &(*result->d_name);
dirent* entry;
while ((errno = 0, entry = readdir(dir)) != nullptr) {
const char* entry_name = entry->d_name;
if (strcmp(entry_name, ".") == 0 || strcmp(entry_name, "..") == 0) {
continue;
}
@ -131,7 +120,7 @@ bool CloseMultipleNowOrOnExecUsingFDDir(int fd, int preserve_fd) {
}
if (errno != 0) {
PLOG(WARNING) << kReaddirName;
PLOG(WARNING) << "readdir";
return false;
}