gugelfrei
/
android_system_vold
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge "Refactor ForkExecvp to improve locking behaviour"

Browse Source 
am: 6aaedb0dca

Change-Id: I571a44a1b068be0acd9e39c974b67d865f851e66
gugelfrei
Paul Crowley 6 years ago committed by android-build-merger
parent 4f75fa7703 6aaedb0dca
commit 12d8d6343b
7 changed files with 78 additions and 93 deletions
Show Stats Download Patch File Download Diff File

153
Utils.cpp
Unescape View File

@ -25,6 +25,7 @@
#include <android-base/properties.h> #include <android-base/properties.h>
#include <android-base/stringprintf.h> #include <android-base/stringprintf.h>
#include <android-base/strings.h> #include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <cutils/fs.h> #include <cutils/fs.h>
#include <logwrap/logwrap.h> #include <logwrap/logwrap.h>
#include <private/android_filesystem_config.h> #include <private/android_filesystem_config.h>
@ -235,7 +236,7 @@ static status_t readMetadata(const std::string& path, std::string* fsType, std::
cmd.push_back(path); cmd.push_back(path);
std::vector<std::string> output; std::vector<std::string> output;
status_t res = ForkExecvp(cmd, output, untrusted ? sBlkidUntrustedContext : sBlkidContext); status_t res = ForkExecvp(cmd, &output, untrusted ? sBlkidUntrustedContext : sBlkidContext);
if (res != OK) { if (res != OK) {
LOG(WARNING) << "blkid failed to identify " << path; LOG(WARNING) << "blkid failed to identify " << path;
return res; return res;
@ -261,101 +262,93 @@ status_t ReadMetadataUntrusted(const std::string& path, std::string* fsType, std
return readMetadata(path, fsType, fsUuid, fsLabel, true); return readMetadata(path, fsType, fsUuid, fsLabel, true);
} }
status_t ForkExecvp(const std::vector<std::string>& args) { static std::vector<const char*> ConvertToArgv(const std::vector<std::string>& args) {
return ForkExecvp(args, nullptr); std::vector<const char*> argv;
} argv.reserve(args.size() + 1);
for (const auto& arg : args) {
status_t ForkExecvp(const std::vector<std::string>& args, security_context_t context) { if (argv.empty()) {
std::lock_guard<std::mutex> lock(kSecurityLock); LOG(DEBUG) << arg;
size_t argc = args.size();
char** argv = (char**)calloc(argc, sizeof(char*));
for (size_t i = 0; i < argc; i++) {
argv[i] = (char*)args[i].c_str();
if (i == 0) {
LOG(DEBUG) << args[i];
} else { } else {
LOG(DEBUG) << " " << args[i]; LOG(DEBUG) << " " << arg;
} }
argv.emplace_back(arg.data());
} }
argv.emplace_back(nullptr);
return argv;
}
if (context) { static status_t ReadLinesFromFdAndLog(std::vector<std::string>* output,
if (setexeccon(context)) { android::base::unique_fd ufd) {
LOG(ERROR) << "Failed to setexeccon"; std::unique_ptr<FILE, int (*)(FILE*)> fp(android::base::Fdopen(std::move(ufd), "r"), fclose);
abort(); if (!fp) {
} PLOG(ERROR) << "fdopen in ReadLinesFromFdAndLog";
return -errno;
} }
status_t res = android_fork_execvp(argc, argv, NULL, false, true); if (output) output->clear();
if (context) { char line[1024];
if (setexeccon(nullptr)) { while (fgets(line, sizeof(line), fp.get()) != nullptr) {
LOG(ERROR) << "Failed to setexeccon"; LOG(DEBUG) << line;
abort(); if (output) output->emplace_back(line);
}
} }
return OK;
free(argv);
return res;
}
status_t ForkExecvp(const std::vector<std::string>& args, std::vector<std::string>& output) {
return ForkExecvp(args, output, nullptr);
} }
status_t ForkExecvp(const std::vector<std::string>& args, std::vector<std::string>& output, status_t ForkExecvp(const std::vector<std::string>& args, std::vector<std::string>* output,
security_context_t context) { security_context_t context) {
std::lock_guard<std::mutex> lock(kSecurityLock); auto argv = ConvertToArgv(args);
std::string cmd;
for (size_t i = 0; i < args.size(); i++) { int fd[2];
cmd += args[i] + " "; if (pipe2(fd, O_CLOEXEC) != 0) {
if (i == 0) { PLOG(ERROR) << "pipe2 in ForkExecvp";
LOG(DEBUG) << args[i]; return -errno;
} else {
LOG(DEBUG) << " " << args[i];
}
} }
output.clear(); android::base::unique_fd pipe_read(fd[0]);
android::base::unique_fd pipe_write(fd[1]);
if (context) { pid_t pid = fork();
if (setexeccon(context)) { if (pid == 0) {
LOG(ERROR) << "Failed to setexeccon"; if (context) {
abort(); if (setexeccon(context)) {
LOG(ERROR) << "Failed to setexeccon";
abort();
}
} }
} pipe_read.reset();
FILE* fp = popen(cmd.c_str(), "r"); // NOLINT if (pipe_write.get() != STDOUT_FILENO) {
if (context) { dup2(pipe_write.get(), STDOUT_FILENO);
if (setexeccon(nullptr)) { pipe_write.reset();
LOG(ERROR) << "Failed to setexeccon";
abort();
} }
execvp(argv[0], const_cast<char**>(argv.data()));
PLOG(ERROR) << "Failed to exec";
_exit(EXIT_FAILURE);
}
if (pid == -1) {
PLOG(ERROR) << "fork in ForkExecvp";
return -errno;
} }
if (!fp) { pipe_write.reset();
PLOG(ERROR) << "Failed to popen " << cmd; auto st = ReadLinesFromFdAndLog(output, std::move(pipe_read));
if (st != 0) return st;
int status;
if (waitpid(pid, &status, 0) == -1) {
PLOG(ERROR) << "waitpid in ForkExecvp";
return -errno; return -errno;
} }
char line[1024]; if (!WIFEXITED(status)) {
while (fgets(line, sizeof(line), fp) != nullptr) { LOG(ERROR) << "Process did not exit normally, status: " << status;
LOG(DEBUG) << line; return -ECHILD;
output.push_back(std::string(line));
} }
if (pclose(fp) != 0) { if (WEXITSTATUS(status)) {
PLOG(ERROR) << "Failed to pclose " << cmd; LOG(ERROR) << "Process exited with code: " << WEXITSTATUS(status);
return -errno; return WEXITSTATUS(status);
} }
return OK; return OK;
} }
pid_t ForkExecvpAsync(const std::vector<std::string>& args) { pid_t ForkExecvpAsync(const std::vector<std::string>& args) {
size_t argc = args.size(); auto argv = ConvertToArgv(args);
char** argv = (char**)calloc(argc + 1, sizeof(char*));
for (size_t i = 0; i < argc; i++) {
argv[i] = (char*)args[i].c_str();
if (i == 0) {
LOG(DEBUG) << args[i];
} else {
LOG(DEBUG) << " " << args[i];
}
}
pid_t pid = fork(); pid_t pid = fork();
if (pid == 0) { if (pid == 0) {
@ -363,18 +356,14 @@ pid_t ForkExecvpAsync(const std::vector<std::string>& args) {
close(STDOUT_FILENO); close(STDOUT_FILENO);
close(STDERR_FILENO); close(STDERR_FILENO);
if (execvp(argv[0], argv)) { execvp(argv[0], const_cast<char**>(argv.data()));
PLOG(ERROR) << "Failed to exec"; PLOG(ERROR) << "exec in ForkExecvpAsync";
} _exit(EXIT_FAILURE);
_exit(1);
} }
if (pid == -1) { if (pid == -1) {
PLOG(ERROR) << "Failed to exec"; PLOG(ERROR) << "fork in ForkExecvpAsync";
return -1;
} }
free(argv);
return pid; return pid;
} }

8
Utils.h
Unescape View File

@ -68,12 +68,8 @@ status_t ReadMetadataUntrusted(const std::string& path, std::string* fsType, std
std::string* fsLabel); std::string* fsLabel);
/* Returns either WEXITSTATUS() status, or a negative errno */ /* Returns either WEXITSTATUS() status, or a negative errno */
status_t ForkExecvp(const std::vector<std::string>& args); status_t ForkExecvp(const std::vector<std::string>& args, std::vector<std::string>* output = nullptr,
status_t ForkExecvp(const std::vector<std::string>& args, security_context_t context); security_context_t context = nullptr);
status_t ForkExecvp(const std::vector<std::string>& args, std::vector<std::string>& output);
status_t ForkExecvp(const std::vector<std::string>& args, std::vector<std::string>& output,
security_context_t context);
pid_t ForkExecvpAsync(const std::vector<std::string>& args); pid_t ForkExecvpAsync(const std::vector<std::string>& args);

2
fs/Exfat.cpp
Unescape View File

@ -43,7 +43,7 @@ status_t Check(const std::string& source) {
cmd.push_back(kFsckPath); cmd.push_back(kFsckPath);
cmd.push_back(source); cmd.push_back(source);
int rc = ForkExecvp(cmd, sFsckUntrustedContext); int rc = ForkExecvp(cmd, nullptr, sFsckUntrustedContext);
if (rc == 0) { if (rc == 0) {
LOG(INFO) << "Check OK"; LOG(INFO) << "Check OK";
return 0; return 0;

2
fs/Ext4.cpp
Unescape View File

@ -117,7 +117,7 @@ status_t Check(const std::string& source, const std::string& target) {
cmd.push_back(c_source); cmd.push_back(c_source);
// ext4 devices are currently always trusted // ext4 devices are currently always trusted
return ForkExecvp(cmd, sFsckContext); return ForkExecvp(cmd, nullptr, sFsckContext);
} }
return 0; return 0;

2
fs/F2fs.cpp
Unescape View File

@ -48,7 +48,7 @@ status_t Check(const std::string& source) {
cmd.push_back(source); cmd.push_back(source);
// f2fs devices are currently always trusted // f2fs devices are currently always trusted
return ForkExecvp(cmd, sFsckContext); return ForkExecvp(cmd, nullptr, sFsckContext);
} }
status_t Mount(const std::string& source, const std::string& target) { status_t Mount(const std::string& source, const std::string& target) {

2
fs/Vfat.cpp
Unescape View File

@ -67,7 +67,7 @@ status_t Check(const std::string& source) {
cmd.push_back(source); cmd.push_back(source);
// Fat devices are currently always untrusted // Fat devices are currently always untrusted
rc = ForkExecvp(cmd, sFsckUntrustedContext); rc = ForkExecvp(cmd, nullptr, sFsckUntrustedContext);
if (rc < 0) { if (rc < 0) {
LOG(ERROR) << "Filesystem check failed due to logwrap error"; LOG(ERROR) << "Filesystem check failed due to logwrap error";

2
model/Disk.cpp
Unescape View File

@ -341,7 +341,7 @@ status_t Disk::readPartitions() {
cmd.push_back(mDevPath); cmd.push_back(mDevPath);
std::vector<std::string> output; std::vector<std::string> output;
status_t res = ForkExecvp(cmd, output); status_t res = ForkExecvp(cmd, &output);
if (res != OK) { if (res != OK) {
LOG(WARNING) << "sgdisk failed to scan " << mDevPath; LOG(WARNING) << "sgdisk failed to scan " << mDevPath;

Write Preview
Loading…
Cancel
Save