Implement a new section to attach LAST_KMSG to incident report
This section simply gzip a large file and stores result in GZippedFileProto
This greatly improves the size, before gzip, the last kmsg size ~500KB,
after gzip the proto size is ~60KB.
Bug: 73354384
Test: atest incidentd_test and manual on device test
Change-Id: I9bfc2cf07384487671edbffb5f0bd8495608fea6
diff --git a/cmds/incidentd/src/FdBuffer.cpp b/cmds/incidentd/src/FdBuffer.cpp
index db60794..64da677 100644
--- a/cmds/incidentd/src/FdBuffer.cpp
+++ b/cmds/incidentd/src/FdBuffer.cpp
@@ -76,6 +76,7 @@
return -errno;
}
} else if (amt == 0) {
+ VLOG("Reached EOF of fd=%d", fd);
break;
}
mBuffer.wp()->move(amt);
@@ -156,10 +157,10 @@
if (!(errno == EAGAIN || errno == EWOULDBLOCK)) {
VLOG("Fail to read fd %d: %s", fd, strerror(errno));
return -errno;
- } // otherwise just continue
- } else if (amt == 0) { // reach EOF so don't have to poll pfds[0].
- ::close(pfds[0].fd);
- pfds[0].fd = -1;
+ } // otherwise just continue
+ } else if (amt == 0) {
+ VLOG("Reached EOF of input file %d", fd);
+ pfds[0].fd = -1; // reach EOF so don't have to poll pfds[0].
} else {
rpos += amt;
cirSize += amt;
@@ -187,6 +188,7 @@
// if buffer is empty and fd is closed, close write fd.
if (cirSize == 0 && pfds[0].fd == -1 && pfds[1].fd != -1) {
+ VLOG("Close write pipe %d", toFd);
::close(pfds[1].fd);
pfds[1].fd = -1;
}
@@ -207,6 +209,7 @@
return -errno;
} // otherwise just continue
} else if (amt == 0) {
+ VLOG("Reached EOF of fromFd %d", fromFd);
break;
} else {
mBuffer.wp()->move(amt);
diff --git a/cmds/incidentd/src/FdBuffer.h b/cmds/incidentd/src/FdBuffer.h
index 5bfa093..66a3de1 100644
--- a/cmds/incidentd/src/FdBuffer.h
+++ b/cmds/incidentd/src/FdBuffer.h
@@ -26,7 +26,7 @@
using namespace std;
/**
- * Reads a file into a buffer, and then writes that data to an FdSet.
+ * Reads data from fd into a buffer, fd must be closed explicitly.
*/
class FdBuffer {
public:
@@ -83,6 +83,11 @@
*/
EncodedBuffer::iterator data() const;
+ /**
+ * Return the internal buffer, don't call unless you are familiar with EncodedBuffer.
+ */
+ EncodedBuffer* getInternalBuffer() { return &mBuffer; }
+
private:
EncodedBuffer mBuffer;
int64_t mStartTime;
diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index 64eae3a..334d77c 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -18,12 +18,8 @@
#include "Section.h"
-#include <errno.h>
-#include <sys/prctl.h>
-#include <unistd.h>
#include <wait.h>
-#include <memory>
#include <mutex>
#include <android-base/file.h>
@@ -37,6 +33,7 @@
#include "FdBuffer.h"
#include "Privacy.h"
#include "PrivacyBuffer.h"
+#include "frameworks/base/core/proto/android/os/data.proto.h"
#include "frameworks/base/core/proto/android/util/log.proto.h"
#include "incidentd_util.h"
@@ -52,31 +49,11 @@
const int WAIT_MAX = 5;
const struct timespec WAIT_INTERVAL_NS = {0, 200 * 1000 * 1000};
const char INCIDENT_HELPER[] = "/system/bin/incident_helper";
+const char GZIP[] = "/system/bin/gzip";
-static pid_t fork_execute_incident_helper(const int id, const char* name, Fpipe& p2cPipe,
- Fpipe& c2pPipe) {
+static pid_t fork_execute_incident_helper(const int id, Fpipe* p2cPipe, Fpipe* c2pPipe) {
const char* ihArgs[]{INCIDENT_HELPER, "-s", String8::format("%d", id).string(), NULL};
- // fork used in multithreaded environment, avoid adding unnecessary code in child process
- pid_t pid = fork();
- if (pid == 0) {
- if (TEMP_FAILURE_RETRY(dup2(p2cPipe.readFd(), STDIN_FILENO)) != 0 || !p2cPipe.close() ||
- TEMP_FAILURE_RETRY(dup2(c2pPipe.writeFd(), STDOUT_FILENO)) != 1 || !c2pPipe.close()) {
- ALOGW("%s can't setup stdin and stdout for incident helper", name);
- _exit(EXIT_FAILURE);
- }
-
- /* make sure the child dies when incidentd dies */
- prctl(PR_SET_PDEATHSIG, SIGKILL);
-
- execv(INCIDENT_HELPER, const_cast<char**>(ihArgs));
-
- ALOGW("%s failed in incident helper process: %s", name, strerror(errno));
- _exit(EXIT_FAILURE); // always exits with failure if any
- }
- // close the fds used in incident helper
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
- return pid;
+ return fork_execute_cmd(INCIDENT_HELPER, const_cast<char**>(ihArgs), p2cPipe, c2pPipe);
}
// ================================================================================
@@ -254,10 +231,12 @@
return NO_ERROR;
}
// ================================================================================
+static inline bool isSysfs(const char* filename) { return strncmp(filename, "/sys/", 5) == 0; }
+
FileSection::FileSection(int id, const char* filename, const int64_t timeoutMs)
: Section(id, timeoutMs), mFilename(filename) {
name = filename;
- mIsSysfs = strncmp(filename, "/sys/", 5) == 0;
+ mIsSysfs = isSysfs(filename);
}
FileSection::~FileSection() {}
@@ -280,7 +259,7 @@
return -errno;
}
- pid_t pid = fork_execute_incident_helper(this->id, this->name.string(), p2cPipe, c2pPipe);
+ pid_t pid = fork_execute_incident_helper(this->id, &p2cPipe, &c2pPipe);
if (pid == -1) {
ALOGW("FileSection '%s' failed to fork", this->name.string());
return -errno;
@@ -289,6 +268,8 @@
// parent process
status_t readStatus = buffer.readProcessedDataInStream(fd, p2cPipe.writeFd(), c2pPipe.readFd(),
this->timeoutMs, mIsSysfs);
+ close(fd); // close the fd anyway.
+
if (readStatus != NO_ERROR || buffer.timedOut()) {
ALOGW("FileSection '%s' failed to read data from incident helper: %s, timedout: %s",
this->name.string(), strerror(-readStatus), buffer.timedOut() ? "true" : "false");
@@ -313,7 +294,99 @@
return NO_ERROR;
}
+// ================================================================================
+GZipSection::GZipSection(int id, const char* filename, ...) : Section(id) {
+ name = "gzip ";
+ name += filename;
+ va_list args;
+ va_start(args, filename);
+ mFilenames = varargs(filename, args);
+ va_end(args);
+}
+GZipSection::~GZipSection() {}
+
+status_t GZipSection::Execute(ReportRequestSet* requests) const {
+ // Reads the files in order, use the first available one.
+ int index = 0;
+ int fd = -1;
+ while (mFilenames[index] != NULL) {
+ fd = open(mFilenames[index], O_RDONLY | O_CLOEXEC);
+ if (fd != -1) {
+ break;
+ }
+ ALOGW("GZipSection failed to open file %s", mFilenames[index]);
+ index++; // look at the next file.
+ }
+ VLOG("GZipSection is using file %s, fd=%d", mFilenames[index], fd);
+ if (fd == -1) return -1;
+
+ FdBuffer buffer;
+ Fpipe p2cPipe;
+ Fpipe c2pPipe;
+ // initiate pipes to pass data to/from gzip
+ if (!p2cPipe.init() || !c2pPipe.init()) {
+ ALOGW("GZipSection '%s' failed to setup pipes", this->name.string());
+ return -errno;
+ }
+
+ const char* gzipArgs[]{GZIP, NULL};
+ pid_t pid = fork_execute_cmd(GZIP, const_cast<char**>(gzipArgs), &p2cPipe, &c2pPipe);
+ if (pid == -1) {
+ ALOGW("GZipSection '%s' failed to fork", this->name.string());
+ return -errno;
+ }
+ // parent process
+
+ // construct Fdbuffer to output GZippedfileProto, the reason to do this instead of using
+ // ProtoOutputStream is to avoid allocation of another buffer inside ProtoOutputStream.
+ EncodedBuffer* internalBuffer = buffer.getInternalBuffer();
+ internalBuffer->writeHeader((uint32_t)GZippedFileProto::FILENAME, WIRE_TYPE_LENGTH_DELIMITED);
+ String8 usedFile(mFilenames[index]);
+ internalBuffer->writeRawVarint32(usedFile.size());
+ for (size_t i = 0; i < usedFile.size(); i++) {
+ internalBuffer->writeRawByte(mFilenames[index][i]);
+ }
+ internalBuffer->writeHeader((uint32_t)GZippedFileProto::GZIPPED_DATA,
+ WIRE_TYPE_LENGTH_DELIMITED);
+ size_t editPos = internalBuffer->wp()->pos();
+ internalBuffer->wp()->move(8); // reserve 8 bytes for the varint of the data size.
+ size_t dataBeginAt = internalBuffer->wp()->pos();
+ VLOG("GZipSection '%s' editPos=%zd, dataBeginAt=%zd", this->name.string(), editPos,
+ dataBeginAt);
+
+ status_t readStatus = buffer.readProcessedDataInStream(
+ fd, p2cPipe.writeFd(), c2pPipe.readFd(), this->timeoutMs, isSysfs(mFilenames[index]));
+ close(fd); // close the fd anyway.
+
+ if (readStatus != NO_ERROR || buffer.timedOut()) {
+ ALOGW("GZipSection '%s' failed to read data from gzip: %s, timedout: %s",
+ this->name.string(), strerror(-readStatus), buffer.timedOut() ? "true" : "false");
+ kill_child(pid);
+ return readStatus;
+ }
+
+ status_t gzipStatus = wait_child(pid);
+ if (gzipStatus != NO_ERROR) {
+ ALOGW("GZipSection '%s' abnormal child process: %s", this->name.string(),
+ strerror(-gzipStatus));
+ return gzipStatus;
+ }
+ // Revisit the actual size from gzip result and edit the internal buffer accordingly.
+ size_t dataSize = buffer.size() - dataBeginAt;
+ internalBuffer->wp()->rewind()->move(editPos);
+ internalBuffer->writeRawVarint32(dataSize);
+ internalBuffer->copy(dataBeginAt, dataSize);
+ VLOG("GZipSection '%s' wrote %zd bytes in %d ms, dataSize=%zd", this->name.string(),
+ buffer.size(), (int)buffer.durationMs(), dataSize);
+ status_t err = write_report_requests(this->id, buffer, requests);
+ if (err != NO_ERROR) {
+ ALOGW("GZipSection '%s' failed writing: %s", this->name.string(), strerror(-err));
+ return err;
+ }
+
+ return NO_ERROR;
+}
// ================================================================================
struct WorkerThreadData : public virtual RefBase {
const WorkerThreadSection* section;
@@ -457,42 +530,20 @@
}
// ================================================================================
-void CommandSection::init(const char* command, va_list args) {
- va_list copied_args;
- int numOfArgs = 0;
-
- va_copy(copied_args, args);
- while (va_arg(copied_args, const char*) != NULL) {
- numOfArgs++;
- }
- va_end(copied_args);
-
- // allocate extra 1 for command and 1 for NULL terminator
- mCommand = (const char**)malloc(sizeof(const char*) * (numOfArgs + 2));
-
- mCommand[0] = command;
- name = command;
- for (int i = 0; i < numOfArgs; i++) {
- const char* arg = va_arg(args, const char*);
- mCommand[i + 1] = arg;
- name += " ";
- name += arg;
- }
- mCommand[numOfArgs + 1] = NULL;
-}
-
CommandSection::CommandSection(int id, const int64_t timeoutMs, const char* command, ...)
: Section(id, timeoutMs) {
+ name = command;
va_list args;
va_start(args, command);
- init(command, args);
+ mCommand = varargs(command, args);
va_end(args);
}
CommandSection::CommandSection(int id, const char* command, ...) : Section(id) {
+ name = command;
va_list args;
va_start(args, command);
- init(command, args);
+ mCommand = varargs(command, args);
va_end(args);
}
@@ -527,7 +578,7 @@
strerror(errno));
_exit(err); // exit with command error code
}
- pid_t ihPid = fork_execute_incident_helper(this->id, this->name.string(), cmdPipe, ihPipe);
+ pid_t ihPid = fork_execute_incident_helper(this->id, &cmdPipe, &ihPipe);
if (ihPid == -1) {
ALOGW("CommandSection '%s' failed to fork", this->name.string());
return -errno;
@@ -544,8 +595,7 @@
}
// TODO: wait for command here has one trade-off: the failed status of command won't be detected
- // until
- // buffer timeout, but it has advatage on starting the data stream earlier.
+ // until buffer timeout, but it has advatage on starting the data stream earlier.
status_t cmdStatus = wait_child(cmdPid);
status_t ihStatus = wait_child(ihPid);
if (cmdStatus != NO_ERROR || ihStatus != NO_ERROR) {
diff --git a/cmds/incidentd/src/Section.h b/cmds/incidentd/src/Section.h
index d644681..8294be1 100644
--- a/cmds/incidentd/src/Section.h
+++ b/cmds/incidentd/src/Section.h
@@ -84,6 +84,21 @@
};
/**
+ * Section that reads in a file and gzips the content.
+ */
+class GZipSection : public Section {
+public:
+ GZipSection(int id, const char* filename, ...);
+ virtual ~GZipSection();
+
+ virtual status_t Execute(ReportRequestSet* requests) const;
+
+private:
+ // It looks up the content from multiple files and stops when the first one is available.
+ const char** mFilenames;
+};
+
+/**
* Base class for sections that call a command that might need a timeout.
*/
class WorkerThreadSection : public Section {
@@ -111,8 +126,6 @@
private:
const char** mCommand;
-
- void init(const char* command, va_list args);
};
/**
diff --git a/cmds/incidentd/src/incidentd_util.cpp b/cmds/incidentd/src/incidentd_util.cpp
index 2415860..fc7cec9 100644
--- a/cmds/incidentd/src/incidentd_util.cpp
+++ b/cmds/incidentd/src/incidentd_util.cpp
@@ -13,8 +13,13 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+#define DEBUG false
+#include "Log.h"
+
#include "incidentd_util.h"
+#include <sys/prctl.h>
+
#include "section_list.h"
const Privacy* get_privacy_of_section(int id) {
@@ -50,4 +55,49 @@
int Fpipe::readFd() const { return mRead.get(); }
-int Fpipe::writeFd() const { return mWrite.get(); }
\ No newline at end of file
+int Fpipe::writeFd() const { return mWrite.get(); }
+
+pid_t fork_execute_cmd(const char* cmd, char* const argv[], Fpipe* input, Fpipe* output) {
+ // fork used in multithreaded environment, avoid adding unnecessary code in child process
+ pid_t pid = fork();
+ if (pid == 0) {
+ if (TEMP_FAILURE_RETRY(dup2(input->readFd(), STDIN_FILENO)) < 0 || !input->close() ||
+ TEMP_FAILURE_RETRY(dup2(output->writeFd(), STDOUT_FILENO)) < 0 || !output->close()) {
+ ALOGW("Can't setup stdin and stdout for command %s", cmd);
+ _exit(EXIT_FAILURE);
+ }
+
+ /* make sure the child dies when incidentd dies */
+ prctl(PR_SET_PDEATHSIG, SIGKILL);
+
+ execv(cmd, argv);
+
+ ALOGW("%s failed in the child process: %s", cmd, strerror(errno));
+ _exit(EXIT_FAILURE); // always exits with failure if any
+ }
+ // close the fds used in child process.
+ close(input->readFd());
+ close(output->writeFd());
+ return pid;
+}
+// ================================================================================
+const char** varargs(const char* first, va_list rest) {
+ va_list copied_rest;
+ int numOfArgs = 1; // first is already count.
+
+ va_copy(copied_rest, rest);
+ while (va_arg(copied_rest, const char*) != NULL) {
+ numOfArgs++;
+ }
+ va_end(copied_rest);
+
+ // allocate extra 1 for NULL terminator
+ const char** ret = (const char**)malloc(sizeof(const char*) * (numOfArgs + 1));
+ ret[0] = first;
+ for (int i = 0; i < numOfArgs; i++) {
+ const char* arg = va_arg(rest, const char*);
+ ret[i + 1] = arg;
+ }
+ ret[numOfArgs + 1] = NULL;
+ return ret;
+}
diff --git a/cmds/incidentd/src/incidentd_util.h b/cmds/incidentd/src/incidentd_util.h
index 09aa040..db7ec82 100644
--- a/cmds/incidentd/src/incidentd_util.h
+++ b/cmds/incidentd/src/incidentd_util.h
@@ -20,12 +20,20 @@
#include <android-base/unique_fd.h>
+#include <stdarg.h>
+
#include "Privacy.h"
using namespace android::base;
+/**
+ * Looks up Privacy of a section in the auto-gen PRIVACY_POLICY_LIST;
+ */
const Privacy* get_privacy_of_section(int id);
+/**
+ * This class wraps android::base::Pipe.
+ */
class Fpipe {
public:
Fpipe();
@@ -41,4 +49,15 @@
unique_fd mWrite;
};
+/**
+ * Forks and exec a command with two pipes, one connects stdin for input,
+ * one connects stdout for output. It returns the pid of the child.
+ */
+pid_t fork_execute_cmd(const char* cmd, char* const argv[], Fpipe* input, Fpipe* output);
+
+/**
+ * Grabs varargs from stack and stores them in heap with NULL-terminated array.
+ */
+const char** varargs(const char* first, va_list rest);
+
#endif // INCIDENTD_UTIL_H
\ No newline at end of file