Replace readlink calls with a safer version that guarantees NULL-termination.

This patch is part of a bigger patch that helps merging the breakpad code
with the modified version in Chromium OS.

Specifically, this patch makes the following changes:
1. Add a SafeReadLink function that wraps sys_readlink() to resolve a
   symbolic link but guarantees the result is NULL-terminated on success.
2. Refactor other source code to use SafeReadLink instead of readlink()
   or sys_readlink().

BUG=455
TEST=Tested the following:
1. Build on 32-bit and 64-bit Linux with gcc 4.4.3 and gcc 4.6.
2. Build on Mac OS X 10.6.8 with gcc 4.2 and clang 3.0 (with latest gmock).
3. All unit tests pass.
4. Run minidump-2-core to covnert a minidump file to a core file.
Review URL: http://breakpad.appspot.com/334001

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@896 4c0a9323-5329-0410-9bdc-e9ce6186880e

4 files changed, 12 insertions, 14 deletions

diff --git a/src/client/linux/crash_generation/crash_generation_server.cc b/src/client/linux/crash_generation/crash_generation_server.cc
index 2f7edb69..76c9d9cd 100644
--- a/src/client/linux/crash_generation/crash_generation_server.cc
+++ b/src/client/linux/crash_generation/crash_generation_server.cc
@@ -47,6 +47,7 @@
 #include "client/linux/minidump_writer/minidump_writer.h"
 #include "common/linux/eintr_wrapper.h"
 #include "common/linux/guid_creator.h"
+#include "common/linux/safe_readlink.h"
 
 static const char kCommandQuit = 'x';
 
@@ -79,12 +80,10 @@ GetInodeForProcPath(ino_t* inode_out, const char* path)
   assert(inode_out);
   assert(path);
 
-  char buf[256];
-  const ssize_t n = readlink(path, buf, sizeof(buf) - 1);
-  if (n == -1) {
+  char buf[PATH_MAX];
+  if (!SafeReadLink(path, buf)) {
     return false;
   }
-  buf[n] = 0;
 
   if (0 != memcmp(kSocketLinkPrefix, buf, sizeof(kSocketLinkPrefix) - 1)) {
     return false;
@@ -130,7 +129,7 @@ FindProcessHoldingSocket(pid_t* pid_out, ino_t socket_inode)
   for (std::vector<pid_t>::const_iterator
        i = pids.begin(); i != pids.end(); ++i) {
     const pid_t current_pid = *i;
-    char buf[256];
+    char buf[PATH_MAX];
     snprintf(buf, sizeof(buf), "/proc/%d/fd", current_pid);
     DIR* fd = opendir(buf);
     if (!fd)
diff --git a/src/client/linux/minidump_writer/linux_dumper.cc b/src/client/linux/minidump_writer/linux_dumper.cc
index f4ab5b1f..a119abaf 100644
--- a/src/client/linux/minidump_writer/linux_dumper.cc
+++ b/src/client/linux/minidump_writer/linux_dumper.cc
@@ -57,6 +57,7 @@
 #include "common/linux/file_id.h"
 #include "common/linux/linux_libc_support.h"
 #include "common/linux/memory_mapped_file.h"
+#include "common/linux/safe_readlink.h"
 #include "third_party/lss/linux_syscall_support.h"
 
 static const char kMappedFileUnsafePrefix[] = "/dev/";
@@ -536,10 +537,8 @@ bool LinuxDumper::HandleDeletedFileInMapping(char* path) const {
   char exe_link[NAME_MAX];
   char new_path[NAME_MAX];
   BuildProcPath(exe_link, pid_, "exe");
-  ssize_t new_path_len = sys_readlink(exe_link, new_path, NAME_MAX);
-  if (new_path_len <= 0 || new_path_len == NAME_MAX)
+  if (!SafeReadLink(exe_link, new_path))
     return false;
-  new_path[new_path_len] = '\0';
   if (my_strcmp(path, new_path) != 0)
     return false;
 
diff --git a/src/client/linux/minidump_writer/linux_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_dumper_unittest.cc
index 1eee9a18..6f4e0e44 100644
--- a/src/client/linux/minidump_writer/linux_dumper_unittest.cc
+++ b/src/client/linux/minidump_writer/linux_dumper_unittest.cc
@@ -43,6 +43,7 @@
 #include "client/linux/minidump_writer/linux_dumper.h"
 #include "common/linux/eintr_wrapper.h"
 #include "common/linux/file_id.h"
+#include "common/linux/safe_readlink.h"
 #include "common/memory.h"
 
 using std::string;
@@ -54,7 +55,7 @@ typedef testing::Test LinuxDumperTest;
 string GetHelperBinary() {
   // Locate helper binary next to the current binary.
   char self_path[PATH_MAX];
-  if (readlink("/proc/self/exe", self_path, sizeof(self_path) - 1) == -1) {
+  if (!SafeReadLink("/proc/self/exe", self_path)) {
     return "";
   }
   string helper_path(self_path);
@@ -379,9 +380,7 @@ TEST(LinuxDumperTest, FileIDsMatch) {
   // FileID::ElfFileIdentifier and LinuxDumper::ElfFileIdentifierForMapping
   // and ensure that we get the same result from both.
   char exe_name[PATH_MAX];
-  ssize_t len = readlink("/proc/self/exe", exe_name, PATH_MAX - 1);
-  ASSERT_NE(len, -1);
-  exe_name[len] = '\0';
+  ASSERT_TRUE(SafeReadLink("/proc/self/exe", exe_name));
 
   int fds[2];
   ASSERT_NE(-1, pipe(fds));
diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest.cc b/src/client/linux/minidump_writer/minidump_writer_unittest.cc
index 41465268..ee021ae3 100644
--- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc
+++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc
@@ -42,6 +42,7 @@
 #include "client/linux/minidump_writer/minidump_writer.h"
 #include "common/linux/eintr_wrapper.h"
 #include "common/linux/file_id.h"
+#include "common/linux/safe_readlink.h"
 #include "common/tests/auto_tempdir.h"
 #include "google_breakpad/processor/minidump.h"
 
@@ -287,8 +288,8 @@ TEST(MinidumpWriterTest, DeletedBinary) {
 
   // Locate helper binary next to the current binary.
   char self_path[PATH_MAX];
-  if (readlink("/proc/self/exe", self_path, sizeof(self_path) - 1) == -1) {
-    FAIL() << "readlink failed: " << strerror(errno);
+  if (!SafeReadLink("/proc/self/exe", self_path)) {
+    FAIL() << "readlink failed";
     exit(1);
   }
   string helper_path(self_path);