Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Fix incidentd stack use-after-return" into rvc-dev am: 0c4353a5d6 am: 0595081dc2

Browse Source 
Change-Id: I9a50193d9cc02d9944d54fbb44cd13c5ed020ff3
This commit is contained in:
TreeHugger Robot
2020-03-18 20:39:02 +00:00
committed by Automerger Merge Worker
parent cf63f4b0d9 0595081dc2
commit 5db0fc10b2
1 changed files with 17 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
cmds/incidentd/src/Section.cpp
View File

@@ -267,28 +267,29 @@ status_t WorkerThreadSection::Execute(ReportWriter* writer) const {
bool workerDone = false; bool workerDone = false;
FdBuffer buffer; FdBuffer buffer;
// Create shared data and pipe // Create shared data and pipe. Don't put data on the stack since this thread may exit early.
WorkerThreadData data(this); sp<WorkerThreadData> data = new WorkerThreadData(this);
if (!data.pipe.init()) { if (!data->pipe.init()) {
return -errno; return -errno;
} }
data->incStrong(this);
std::thread([&]() { std::thread([data, this]() {
// Don't crash the service if writing to a closed pipe (may happen if dumping times out) // Don't crash the service if writing to a closed pipe (may happen if dumping times out)
signal(SIGPIPE, sigpipe_handler); signal(SIGPIPE, sigpipe_handler);
status_t err = data.section->BlockingCall(data.pipe.writeFd()); status_t err = data->section->BlockingCall(data->pipe.writeFd());
{ {
std::unique_lock<std::mutex> lock(data.lock); std::scoped_lock<std::mutex> lock(data->lock);
data.workerDone = true; data->workerDone = true;
data.workerError = err; data->workerError = err;
// unique_fd is not thread safe. If we don't lock it, reset() may pause half way while // unique_fd is not thread safe. If we don't lock it, reset() may pause half way while
// the other thread executes to the end, calling ~Fpipe, which is a race condition. // the other thread executes to the end, calling ~Fpipe, which is a race condition.
data.pipe.writeFd().reset(); data->pipe.writeFd().reset();
} }
data->decStrong(this);
}).detach(); }).detach();
// Loop reading until either the timeout or the worker side is done (i.e. eof). // Loop reading until either the timeout or the worker side is done (i.e. eof).
err = buffer.read(data.pipe.readFd().get(), this->timeoutMs); err = buffer.read(data->pipe.readFd().get(), this->timeoutMs);
if (err != NO_ERROR) { if (err != NO_ERROR) {
ALOGE("[%s] reader failed with error '%s'", this->name.string(), strerror(-err)); ALOGE("[%s] reader failed with error '%s'", this->name.string(), strerror(-err));
} }
@@ -296,13 +297,13 @@ status_t WorkerThreadSection::Execute(ReportWriter* writer) const {
// If the worker side is finished, then return its error (which may overwrite // If the worker side is finished, then return its error (which may overwrite
// our possible error -- but it's more interesting anyway). If not, then we timed out. // our possible error -- but it's more interesting anyway). If not, then we timed out.
{ {
std::unique_lock<std::mutex> lock(data.lock); std::scoped_lock<std::mutex> lock(data->lock);
data.pipe.close(); data->pipe.close();
if (data.workerError != NO_ERROR) { if (data->workerError != NO_ERROR) {
err = data.workerError; err = data->workerError;
ALOGE("[%s] worker failed with error '%s'", this->name.string(), strerror(-err)); ALOGE("[%s] worker failed with error '%s'", this->name.string(), strerror(-err));
} }
workerDone = data.workerDone; workerDone = data->workerDone;
} }
writer->setSectionStats(buffer); writer->setSectionStats(buffer);