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

Browse Source 
Change-Id: I2f9d65570fb1a3540e9db8cd3d2fd69b547f87c5
This commit is contained in:
TreeHugger Robot
2020-03-18 20:35:33 +00:00
committed by Automerger Merge Worker
parent 6f7f7c71c2 0c4353a5d6
commit 0595081dc2
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;
FdBuffer buffer;
// Create shared data and pipe
WorkerThreadData data(this);
if (!data.pipe.init()) {
// Create shared data and pipe. Don't put data on the stack since this thread may exit early.
sp<WorkerThreadData> data = new WorkerThreadData(this);
if (!data->pipe.init()) {
return -errno;
}
std::thread([&]() {
data->incStrong(this);
std::thread([data, this]() {
// Don't crash the service if writing to a closed pipe (may happen if dumping times out)
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);
data.workerDone = true;
data.workerError = err;
std::scoped_lock<std::mutex> lock(data->lock);
data->workerDone = true;
data->workerError = err;
// 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.
data.pipe.writeFd().reset();
data->pipe.writeFd().reset();
}
data->decStrong(this);
}).detach();
// 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) {
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
// our possible error -- but it's more interesting anyway). If not, then we timed out.
{
std::unique_lock<std::mutex> lock(data.lock);
data.pipe.close();
if (data.workerError != NO_ERROR) {
err = data.workerError;
std::scoped_lock<std::mutex> lock(data->lock);
data->pipe.close();
if (data->workerError != NO_ERROR) {
err = data->workerError;
ALOGE("[%s] worker failed with error '%s'", this->name.string(), strerror(-err));
}
workerDone = data.workerDone;
workerDone = data->workerDone;
}
writer->setSectionStats(buffer);