Skip to content

Commit

Permalink
Stack trace symbolization for Chrome sandbox processes.
Browse files Browse the repository at this point in the history
Patching the POSIX implementation of base::debug::StackTrace to properly
symbolize stack traces when used in Chrome sandbox processes. The main
reason symbolization doesn't work today is because sandboxed processes
cannot open proc/self/maps and the object files that contain the symbols.

The proposed solution is to open all necessary files before enabling the
sandbox and then keep their file descriptors around for the duration of
the sandboxed process.  This cannot be done in  shipping/retail builds
because it has security implications so for the time being this code will
run only in DEBUG builds of Chrome.

Originally, the code review was started under a different account.  For
more information take a look at: https://codereview.chromium.org/156043003/

BUG=341966

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258548

Review URL: https://codereview.chromium.org/184273012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258719 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
ivanpe@google.com committed Mar 22, 2014
1 parent f3357cb commit ad9a012
Show file tree
Hide file tree
Showing 6 changed files with 322 additions and 8 deletions.
6 changes: 5 additions & 1 deletion base/debug/proc_maps_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ bool ReadProcMaps(std::string* proc_maps) {

bool ParseProcMaps(const std::string& input,
std::vector<MappedMemoryRegion>* regions_out) {
CHECK(regions_out);
std::vector<MappedMemoryRegion> regions;

// This isn't async safe nor terribly efficient, but it doesn't need to be at
Expand All @@ -101,8 +102,10 @@ bool ParseProcMaps(const std::string& input,
for (size_t i = 0; i < lines.size(); ++i) {
// Due to splitting on '\n' the last line should be empty.
if (i == lines.size() - 1) {
if (!lines[i].empty())
if (!lines[i].empty()) {
DLOG(WARNING) << "Last line not empty";
return false;
}
break;
}

Expand All @@ -125,6 +128,7 @@ bool ParseProcMaps(const std::string& input,
if (sscanf(line, "%" SCNxPTR "-%" SCNxPTR " %4c %llx %hhx:%hhx %ld %n",
&region.start, &region.end, permissions, &region.offset,
&dev_major, &dev_minor, &inode, &path_index) < 7) {
DPLOG(WARNING) << "sscanf failed for line: " << line;
return false;
}

Expand Down
32 changes: 32 additions & 0 deletions base/debug/proc_maps_linux_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -277,5 +277,37 @@ TEST(ProcMapsTest, InvalidInput) {
}
}

TEST(ProcMapsTest, ParseProcMapsEmptyString) {
std::vector<MappedMemoryRegion> regions;
EXPECT_TRUE(ParseProcMaps("", &regions));
EXPECT_EQ(0ULL, regions.size());
}

// Testing a couple of remotely possible weird things in the input:
// - Line ending with \r\n or \n\r.
// - File name contains quotes.
// - File name has whitespaces.
TEST(ProcMapsTest, ParseProcMapsWeirdCorrectInput) {
std::vector<MappedMemoryRegion> regions;
const std::string kContents =
"00400000-0040b000 r-xp 00000000 fc:00 2106562 "
" /bin/cat\r\n"
"7f53b7dad000-7f53b7f62000 r-xp 00000000 fc:00 263011 "
" /lib/x86_64-linux-gnu/libc-2.15.so\n\r"
"7f53b816d000-7f53b818f000 r-xp 00000000 fc:00 264284 "
" /lib/x86_64-linux-gnu/ld-2.15.so\n"
"7fff9c7ff000-7fff9c800000 r-xp 00000000 00:00 0 "
" \"vd so\"\n"
"ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 "
" [vsys call]\n";
EXPECT_TRUE(ParseProcMaps(kContents, &regions));
EXPECT_EQ(5ULL, regions.size());
EXPECT_EQ("/bin/cat", regions[0].path);
EXPECT_EQ("/lib/x86_64-linux-gnu/libc-2.15.so", regions[1].path);
EXPECT_EQ("/lib/x86_64-linux-gnu/ld-2.15.so", regions[2].path);
EXPECT_EQ("\"vd so\"", regions[3].path);
EXPECT_EQ("[vsys call]", regions[4].path);
}

} // namespace debug
} // namespace base
9 changes: 9 additions & 0 deletions base/debug/stack_trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ namespace debug {
// unit_tests only! This is not thread-safe: only call from main thread.
BASE_EXPORT bool EnableInProcessStackDumping();

// A different version of EnableInProcessStackDumping that also works for
// sandboxed processes. For more details take a look at the description
// of EnableInProcessStackDumping.
// Calling this function on Linux opens /proc/self/maps and caches its
// contents. In DEBUG builds, this function also opens the object files that
// are loaded in memory and caches their file descriptors (this cannot be
// done in official builds because it has security implications).
BASE_EXPORT bool EnableInProcessStackDumpingForSandbox();

// A stacktrace can be helpful in debugging. For example, you can include a
// stacktrace member in a object (probably around #ifndef NDEBUG) so that you
// can later see where the given object was created from.
Expand Down
Loading

0 comments on commit ad9a012

Please sign in to comment.