Merge "dumpstate: handle errors gracefully"

3 files changed, 50 insertions, 21 deletions

diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 06dc58f8bd..3c47767c18 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -1746,7 +1746,9 @@ bool Dumpstate::FinishZipFile() {
     }
     // TODO: Should truncate the existing file.
     // ... and re-open it for further logging.
-    redirect_to_existing_file(stderr, const_cast<char*>(ds.log_path_.c_str()));
+    if (!redirect_to_existing_file(stderr, const_cast<char*>(ds.log_path_.c_str()))) {
+        return false;
+    }
     fprintf(stderr, "\n");
 
     int32_t err = zip_writer_->Finish();
@@ -2366,12 +2368,17 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid,
     // If we are going to use a socket, do it as early as possible
     // to avoid timeouts from bugreport.
     if (options_->use_socket) {
-        redirect_to_socket(stdout, "dumpstate");
+        if (!redirect_to_socket(stdout, "dumpstate")) {
+            return ERROR;
+        }
     }
 
     if (options_->use_control_socket) {
         MYLOGD("Opening control socket\n");
         control_socket_fd_ = open_socket("dumpstate");
+        if (control_socket_fd_ == -1) {
+            return ERROR;
+        }
         options_->do_progress_updates = 1;
     }
 
@@ -2430,7 +2437,9 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid,
     if (is_redirecting) {
         // Redirect stderr to log_path_ for debugging.
         TEMP_FAILURE_RETRY(dup_stderr_fd = dup(fileno(stderr)));
-        redirect_to_file(stderr, const_cast<char*>(log_path_.c_str()));
+        if (!redirect_to_file(stderr, const_cast<char*>(log_path_.c_str()))) {
+            return ERROR;
+        }
         if (chown(log_path_.c_str(), AID_SHELL, AID_SHELL)) {
             MYLOGE("Unable to change ownership of dumpstate log file %s: %s\n", log_path_.c_str(),
                    strerror(errno));
@@ -2443,7 +2452,9 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid,
         /* TODO: rather than generating a text file now and zipping it later,
            it would be more efficient to redirect stdout to the zip entry
            directly, but the libziparchive doesn't support that option yet. */
-        redirect_to_file(stdout, const_cast<char*>(tmp_path_.c_str()));
+        if (!redirect_to_file(stdout, const_cast<char*>(tmp_path_.c_str()))) {
+            return ERROR;
+        }
         if (chown(tmp_path_.c_str(), AID_SHELL, AID_SHELL)) {
             MYLOGE("Unable to change ownership of temporary bugreport file %s: %s\n",
                    tmp_path_.c_str(), strerror(errno));
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 9803f00aa7..c326bb6bfa 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -519,14 +519,26 @@ int dump_files(const std::string& title, const char* dir, bool (*skip)(const cha
 /** opens a socket and returns its file descriptor */
 int open_socket(const char *service);
 
-/* redirect output to a service control socket */
-void redirect_to_socket(FILE *redirect, const char *service);
+/*
+ * Redirects 'redirect' to a service control socket.
+ *
+ * Returns true if redirect succeeds.
+ */
+bool redirect_to_socket(FILE* redirect, const char* service);
 
-/* redirect output to a new file */
-void redirect_to_file(FILE *redirect, char *path);
+/*
+ * Redirects 'redirect' to a file indicated by 'path', truncating it.
+ *
+ * Returns true if redirect succeeds.
+ */
+bool redirect_to_file(FILE* redirect, char* path);
 
-/* redirect output to an existing file */
-void redirect_to_existing_file(FILE *redirect, char *path);
+/*
+ * Redirects 'redirect' to an existing file indicated by 'path', appending it.
+ *
+ * Returns true if redirect succeeds.
+ */
+bool redirect_to_existing_file(FILE* redirect, char* path);
 
 /* create leading directories, if necessary */
 void create_parent_dirs(const char *path);
diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp
index 4bc0e1d838..2a5516d7f5 100644
--- a/cmds/dumpstate/utils.cpp
+++ b/cmds/dumpstate/utils.cpp
@@ -712,12 +712,12 @@ int open_socket(const char *service) {
     int s = android_get_control_socket(service);
     if (s < 0) {
         MYLOGE("android_get_control_socket(%s): %s\n", service, strerror(errno));
-        exit(1);
+        return -1;
     }
     fcntl(s, F_SETFD, FD_CLOEXEC);
     if (listen(s, 4) < 0) {
         MYLOGE("listen(control socket): %s\n", strerror(errno));
-        exit(1);
+        return -1;
     }
 
     struct sockaddr addr;
@@ -725,18 +725,23 @@ int open_socket(const char *service) {
     int fd = accept(s, &addr, &alen);
     if (fd < 0) {
         MYLOGE("accept(control socket): %s\n", strerror(errno));
-        exit(1);
+        return -1;
     }
 
     return fd;
 }
 
 /* redirect output to a service control socket */
-void redirect_to_socket(FILE *redirect, const char *service) {
+bool redirect_to_socket(FILE* redirect, const char* service) {
     int fd = open_socket(service);
+    if (fd == -1) {
+        return false;
+    }
     fflush(redirect);
-    dup2(fd, fileno(redirect));
+    // TODO: handle dup2 failure
+    TEMP_FAILURE_RETRY(dup2(fd, fileno(redirect)));
     close(fd);
+    return true;
 }
 
 // TODO: should call is_valid_output_file and/or be merged into it.
@@ -766,7 +771,7 @@ void create_parent_dirs(const char *path) {
     }
 }
 
-void _redirect_to_file(FILE *redirect, char *path, int truncate_flag) {
+bool _redirect_to_file(FILE* redirect, char* path, int truncate_flag) {
     create_parent_dirs(path);
 
     int fd = TEMP_FAILURE_RETRY(open(path,
@@ -774,19 +779,20 @@ void _redirect_to_file(FILE *redirect, char *path, int truncate_flag) {
                                      S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
     if (fd < 0) {
         MYLOGE("%s: %s\n", path, strerror(errno));
-        exit(1);
+        return false;
     }
 
     TEMP_FAILURE_RETRY(dup2(fd, fileno(redirect)));
     close(fd);
+    return true;
 }
 
-void redirect_to_file(FILE *redirect, char *path) {
-    _redirect_to_file(redirect, path, O_TRUNC);
+bool redirect_to_file(FILE* redirect, char* path) {
+    return _redirect_to_file(redirect, path, O_TRUNC);
 }
 
-void redirect_to_existing_file(FILE *redirect, char *path) {
-    _redirect_to_file(redirect, path, O_APPEND);
+bool redirect_to_existing_file(FILE* redirect, char* path) {
+    return _redirect_to_file(redirect, path, O_APPEND);
 }
 
 // Dump Dalvik and native stack traces, return the trace file location (nullptr if none).