diff options
26 files changed, 331 insertions, 134 deletions
diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp index 6596fa246c..eb0d50da28 100644 --- a/cmds/dumpstate/DumpstateService.cpp +++ b/cmds/dumpstate/DumpstateService.cpp @@ -43,9 +43,8 @@ static binder::Status error(uint32_t code, const std::string& msg) { static void* callAndNotify(void* data) { Dumpstate& ds = *static_cast<Dumpstate*>(data); - // TODO(111441001): Return status on listener. ds.Run(); - MYLOGE("Finished Run()\n"); + MYLOGD("Finished Run()\n"); return nullptr; } @@ -98,7 +97,10 @@ binder::Status DumpstateService::setListener(const std::string& name, return binder::Status::ok(); } -binder::Status DumpstateService::startBugreport(const android::base::unique_fd& bugreport_fd, +// TODO(b/111441001): Hook up to consent service & copy final br only if user approves. +binder::Status DumpstateService::startBugreport(int32_t /* calling_uid */, + const std::string& /* calling_package */, + const android::base::unique_fd& bugreport_fd, const android::base::unique_fd& screenshot_fd, int bugreport_mode, const sp<IDumpstateListener>& listener) { @@ -138,7 +140,18 @@ binder::Status DumpstateService::startBugreport(const android::base::unique_fd& return binder::Status::ok(); } +binder::Status DumpstateService::cancelBugreport() { + // This is a no-op since the cancellation is done from java side via setting sys properties. + // See BugreportManagerServiceImpl. + // TODO(b/111441001): maybe make native and java sides use different binder interface + // to avoid these annoyances. + return binder::Status::ok(); +} + status_t DumpstateService::dump(int fd, const Vector<String16>&) { + std::string destination = ds_.options_->bugreport_fd.get() != -1 + ? StringPrintf("[fd:%d]", ds_.options_->bugreport_fd.get()) + : ds_.bugreport_internal_dir_.c_str(); dprintf(fd, "id: %d\n", ds_.id_); dprintf(fd, "pid: %d\n", ds_.pid_); dprintf(fd, "update_progress: %s\n", ds_.options_->do_progress_updates ? "true" : "false"); @@ -149,8 +162,7 @@ status_t DumpstateService::dump(int fd, const Vector<String16>&) { dprintf(fd, "args: %s\n", ds_.options_->args.c_str()); dprintf(fd, "extra_options: %s\n", ds_.options_->extra_options.c_str()); dprintf(fd, "version: %s\n", ds_.version_.c_str()); - dprintf(fd, "bugreport_dir: %s\n", ds_.bugreport_dir_.c_str()); - dprintf(fd, "bugreport_internal_dir_: %s\n", ds_.bugreport_internal_dir_.c_str()); + dprintf(fd, "bugreport_dir: %s\n", destination.c_str()); dprintf(fd, "screenshot_path: %s\n", ds_.screenshot_path_.c_str()); dprintf(fd, "log_path: %s\n", ds_.log_path_.c_str()); dprintf(fd, "tmp_path: %s\n", ds_.tmp_path_.c_str()); diff --git a/cmds/dumpstate/DumpstateService.h b/cmds/dumpstate/DumpstateService.h index 1705317bdb..faeea5350a 100644 --- a/cmds/dumpstate/DumpstateService.h +++ b/cmds/dumpstate/DumpstateService.h @@ -42,10 +42,14 @@ class DumpstateService : public BinderService<DumpstateService>, public BnDumpst bool getSectionDetails, sp<IDumpstateToken>* returned_token) override; - binder::Status startBugreport(const android::base::unique_fd& bugreport_fd, + binder::Status startBugreport(int32_t calling_uid, const std::string& calling_package, + const android::base::unique_fd& bugreport_fd, const android::base::unique_fd& screenshot_fd, int bugreport_mode, const sp<IDumpstateListener>& listener) override; + // No-op + binder::Status cancelBugreport(); + private: Dumpstate& ds_; std::mutex lock_; diff --git a/cmds/dumpstate/README.md b/cmds/dumpstate/README.md index 6ac17d8772..273a5a645b 100644 --- a/cmds/dumpstate/README.md +++ b/cmds/dumpstate/README.md @@ -49,7 +49,7 @@ adb shell mkdir /data/nativetest64 Then run: ``` -mmm -j frameworks/native/cmds/dumpstate/ && adb push ${OUT}/data/nativetest64/dumpstate_test* /data/nativetest64 && adb shell /data/nativetest64/dumpstate_test/dumpstate_test +mmm -j frameworks/native/cmds/dumpstate/ && adb push ${OUT}/data/nativetest64/dumpstate_* /data/nativetest64 && adb shell /data/nativetest64/dumpstate_test/dumpstate_test ``` And to run just one test (for example, `DumpstateTest.RunCommandNoArgs`): diff --git a/cmds/dumpstate/binder/android/os/IDumpstate.aidl b/cmds/dumpstate/binder/android/os/IDumpstate.aidl index d24c953fa2..f58535eed0 100644 --- a/cmds/dumpstate/binder/android/os/IDumpstate.aidl +++ b/cmds/dumpstate/binder/android/os/IDumpstate.aidl @@ -59,14 +59,29 @@ interface IDumpstate { // Default mode. const int BUGREPORT_MODE_DEFAULT = 6; + // TODO(b/111441001): Should the args be for the consuming application rather than triggering? /* * Starts a bugreport in the background. * + *<p>Shows the user a dialog to get consent for sharing the bugreport with the calling + * application. If they deny {@link IDumpstateListener#onError} will be called. If they + * consent and bugreport generation is successful artifacts will be copied to the given fds and + * {@link IDumpstateListener#onFinished} will be called. If there + * are errors in bugreport generation {@link IDumpstateListener#onError} will be called. + * + * @param callingUid UID of the original application that requested the report. + * @param callingPackage package of the original application that requested the report. * @param bugreportFd the file to which the zipped bugreport should be written * @param screenshotFd the file to which screenshot should be written; optional * @param bugreportMode the mode that specifies other run time options; must be one of above * @param listener callback for updates; optional */ - void startBugreport(FileDescriptor bugreportFd, FileDescriptor screenshotFd, int bugreportMode, - IDumpstateListener listener); + void startBugreport(int callingUid, @utf8InCpp String callingPackage, + FileDescriptor bugreportFd, FileDescriptor screenshotFd, + int bugreportMode, IDumpstateListener listener); + + /* + * Cancels the bugreport currently in progress. + */ + void cancelBugreport(); } diff --git a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl index 2966c86ee9..8396acd0b4 100644 --- a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl +++ b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl @@ -35,20 +35,18 @@ interface IDumpstateListener { /* Bugreport encountered a runtime error */ const int BUGREPORT_ERROR_RUNTIME_ERROR = 2; + /* User denied consent to share the bugreport with the specified app */ + const int BUGREPORT_ERROR_USER_DENIED_CONSENT = 3; + /** * Called on an error condition with one of the error codes listed above. */ oneway void onError(int errorCode); /** - * Called when taking bugreport finishes successfully - * - * @param durationMs time capturing bugreport took in milliseconds - * @param title title for the bugreport; helpful in reminding the user why they took it - * @param description detailed description for the bugreport + * Called when taking bugreport finishes successfully. */ - oneway void onFinished(long durationMs, @utf8InCpp String title, - @utf8InCpp String description); + oneway void onFinished(); // TODO(b/111441001): Remove old methods when not used anymore. void onProgressUpdated(int progress); diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 2824c82db1..c0050845a0 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -135,10 +135,6 @@ static int Open(std::string path, int flags, mode_t mode = 0) { return fd; } -static int OpenForWrite(std::string path) { - return Open(path, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, - S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); -} static int OpenForRead(std::string path) { return Open(path, O_RDONLY | O_CLOEXEC | O_NOFOLLOW); @@ -169,17 +165,6 @@ static bool CopyFileToFd(const std::string& input_file, int out_fd) { return false; } -static bool CopyFileToFile(const std::string& input_file, const std::string& output_file) { - if (input_file == output_file) { - MYLOGD("Skipping copying bugreport file since the destination is the same (%s)\n", - output_file.c_str()); - return false; - } - - MYLOGD("Going to copy bugreport file (%s) to %s\n", input_file.c_str(), output_file.c_str()); - android::base::unique_fd out_fd(OpenForWrite(output_file)); - return CopyFileToFd(input_file, out_fd.get()); -} } // namespace } // namespace os @@ -1669,6 +1654,7 @@ static void ShowUsage() { "progress (requires -o and -B)\n" " -R: take bugreport in remote mode (requires -o, -z, -d and -B, " "shouldn't be used with -P)\n" + " -w: start binder service and make it wait for a call to startBugreport\n" " -v: prints the dumpstate header and exit\n"); } @@ -1800,16 +1786,9 @@ static void MaybeResolveSymlink(std::string* path) { static void PrepareToWriteToFile() { MaybeResolveSymlink(&ds.bugreport_internal_dir_); - std::string base_name_part1 = "bugreport"; - if (!ds.options_->use_outfile.empty()) { - ds.bugreport_dir_ = dirname(ds.options_->use_outfile.c_str()); - base_name_part1 = basename(ds.options_->use_outfile.c_str()); - } - std::string build_id = android::base::GetProperty("ro.build.id", "UNKNOWN_BUILD"); std::string device_name = android::base::GetProperty("ro.product.name", "UNKNOWN_DEVICE"); - ds.base_name_ = - StringPrintf("%s-%s-%s", base_name_part1.c_str(), device_name.c_str(), build_id.c_str()); + ds.base_name_ = StringPrintf("bugreport-%s-%s", device_name.c_str(), build_id.c_str()); if (ds.options_->do_add_date) { char date[80]; strftime(date, sizeof(date), "%Y-%m-%d-%H-%M-%S", localtime(&ds.now_)); @@ -1832,17 +1811,16 @@ static void PrepareToWriteToFile() { std::string destination = ds.options_->bugreport_fd.get() != -1 ? StringPrintf("[fd:%d]", ds.options_->bugreport_fd.get()) - : ds.bugreport_dir_.c_str(); + : ds.bugreport_internal_dir_.c_str(); MYLOGD( "Bugreport dir: %s\n" - "Internal Bugreport dir: %s\n" "Base name: %s\n" "Suffix: %s\n" "Log path: %s\n" "Temporary path: %s\n" "Screenshot path: %s\n", - destination.c_str(), ds.bugreport_internal_dir_.c_str(), ds.base_name_.c_str(), - ds.name_.c_str(), ds.log_path_.c_str(), ds.tmp_path_.c_str(), ds.screenshot_path_.c_str()); + destination.c_str(), ds.base_name_.c_str(), ds.name_.c_str(), ds.log_path_.c_str(), + ds.tmp_path_.c_str(), ds.screenshot_path_.c_str()); if (ds.options_->do_zip_file) { ds.path_ = ds.GetPath(".zip"); @@ -1909,18 +1887,13 @@ static void FinalizeFile() { } } // The zip file lives in an internal directory. Copy it over to output. - bool copy_succeeded = false; if (ds.options_->bugreport_fd.get() != -1) { - copy_succeeded = android::os::CopyFileToFd(ds.path_, ds.options_->bugreport_fd.get()); - } else { - ds.final_path_ = ds.GetPath(ds.bugreport_dir_, ".zip"); - copy_succeeded = android::os::CopyFileToFile(ds.path_, ds.final_path_); - } - if (copy_succeeded) { - if (remove(ds.path_.c_str())) { + bool copy_succeeded = + android::os::CopyFileToFd(ds.path_, ds.options_->bugreport_fd.get()); + if (!copy_succeeded && remove(ds.path_.c_str())) { MYLOGE("remove(%s): %s", ds.path_.c_str(), strerror(errno)); } - } + } // else - the file just remains in the internal directory. } } if (do_text_file) { @@ -1946,8 +1919,8 @@ static void FinalizeFile() { /* Broadcasts that we are done with the bugreport */ static void SendBugreportFinishedBroadcast() { // TODO(b/111441001): use callback instead of broadcast. - if (!ds.final_path_.empty()) { - MYLOGI("Final bugreport path: %s\n", ds.final_path_.c_str()); + if (!ds.path_.empty()) { + MYLOGI("Final bugreport path: %s\n", ds.path_.c_str()); // clang-format off std::vector<std::string> am_args = { @@ -1955,7 +1928,7 @@ static void SendBugreportFinishedBroadcast() { "--ei", "android.intent.extra.ID", std::to_string(ds.id_), "--ei", "android.intent.extra.PID", std::to_string(ds.pid_), "--ei", "android.intent.extra.MAX", std::to_string(ds.progress_->GetMax()), - "--es", "android.intent.extra.BUGREPORT", ds.final_path_, + "--es", "android.intent.extra.BUGREPORT", ds.path_, "--es", "android.intent.extra.DUMPSTATE_LOG", ds.log_path_ }; // clang-format on @@ -1977,7 +1950,7 @@ static void SendBugreportFinishedBroadcast() { if (ds.options_->is_remote_mode) { am_args.push_back("--es"); am_args.push_back("android.intent.extra.REMOTE_BUGREPORT_HASH"); - am_args.push_back(SHA256_file_hash(ds.final_path_)); + am_args.push_back(SHA256_file_hash(ds.path_)); SendBroadcast("com.android.internal.intent.action.REMOTE_BUGREPORT_FINISHED", am_args); } else { SendBroadcast("com.android.internal.intent.action.BUGREPORT_FINISHED", am_args); @@ -2115,7 +2088,6 @@ static void LogDumpOptions(const Dumpstate::DumpOptions& options) { MYLOGI("wifi_only: %d\n", options.wifi_only); MYLOGI("do_progress_updates: %d\n", options.do_progress_updates); MYLOGI("fd: %d\n", options.bugreport_fd.get()); - MYLOGI("use_outfile: %s\n", options.use_outfile.c_str()); MYLOGI("extra_options: %s\n", options.extra_options.c_str()); MYLOGI("args: %s\n", options.args.c_str()); MYLOGI("notification_title: %s\n", options.notification_title.c_str()); @@ -2141,12 +2113,14 @@ void Dumpstate::DumpOptions::Initialize(BugreportMode bugreport_mode, Dumpstate::RunStatus Dumpstate::DumpOptions::Initialize(int argc, char* argv[]) { RunStatus status = RunStatus::OK; int c; - while ((c = getopt(argc, argv, "dho:svqzpPBRSV:")) != -1) { + while ((c = getopt(argc, argv, "dho:svqzpPBRSV:w")) != -1) { switch (c) { // clang-format off case 'd': do_add_date = true; break; case 'z': do_zip_file = true; break; - case 'o': use_outfile = optarg; break; + // o=use_outfile not supported anymore. + // TODO(b/111441001): Remove when all callers have migrated. + case 'o': break; case 's': use_socket = true; break; case 'S': use_control_socket = true; break; case 'v': show_header_only = true; break; @@ -2156,6 +2130,9 @@ Dumpstate::RunStatus Dumpstate::DumpOptions::Initialize(int argc, char* argv[]) case 'R': is_remote_mode = true; break; case 'B': do_broadcast = true; break; case 'V': break; // compatibility no-op + case 'w': + // This was already processed + break; case 'h': status = RunStatus::HELP; break; @@ -2187,9 +2164,7 @@ bool Dumpstate::DumpOptions::ValidateOptions() const { return false; } - bool has_out_file_options = !use_outfile.empty() || bugreport_fd.get() != -1; - if ((do_zip_file || do_add_date || do_progress_updates || do_broadcast) && - !has_out_file_options) { + if ((do_zip_file || do_add_date || do_progress_updates || do_broadcast) && !OutputToFile()) { return false; } @@ -2216,9 +2191,7 @@ Dumpstate::RunStatus Dumpstate::Run() { if (listener_ != nullptr) { switch (status) { case Dumpstate::RunStatus::OK: - // TODO(b/111441001): duration argument does not make sense. Remove. - listener_->onFinished(0 /* duration */, options_->notification_title, - options_->notification_description); + listener_->onFinished(); break; case Dumpstate::RunStatus::HELP: break; @@ -2251,8 +2224,8 @@ Dumpstate::RunStatus Dumpstate::Run() { * If zipping, a bunch of other files and dumps also get added to the zip archive. The log file also * gets added to the archive. * - * Bugreports are first generated in a local directory and later copied to the caller's fd or - * directory. + * Bugreports are first generated in a local directory and later copied to the caller's fd if + * supplied. */ Dumpstate::RunStatus Dumpstate::RunInternal() { LogDumpOptions(*options_); @@ -2293,7 +2266,7 @@ Dumpstate::RunStatus Dumpstate::RunInternal() { } // Redirect output if needed - bool is_redirecting = !options_->use_socket && !options_->use_outfile.empty(); + bool is_redirecting = options_->OutputToFile(); // TODO: temporarily set progress until it's part of the Dumpstate constructor std::string stats_path = @@ -2444,7 +2417,7 @@ Dumpstate::RunStatus Dumpstate::RunInternal() { } /* rename or zip the (now complete) .tmp file to its final location */ - if (!options_->use_outfile.empty()) { + if (options_->OutputToFile()) { FinalizeFile(); } diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index c620c079a7..3dfe4e955f 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -331,6 +331,7 @@ class Dumpstate { bool do_add_date = false; bool do_zip_file = false; bool do_vibrate = true; + // Writes bugreport content to a socket; only flatfile format is supported. bool use_socket = false; bool use_control_socket = false; bool do_fb = false; @@ -342,13 +343,11 @@ class Dumpstate { bool wifi_only = false; // Whether progress updates should be published. bool do_progress_updates = false; - // File descriptor to output zip file. Takes precedence over use_outfile. + // File descriptor to output zip file. android::base::unique_fd bugreport_fd; // File descriptor to screenshot file. // TODO(b/111441001): Use this fd. android::base::unique_fd screenshot_fd; - // Partial path to output file. - std::string use_outfile; // TODO: rename to MODE. // Extra options passed as system property. std::string extra_options; @@ -367,6 +366,13 @@ class Dumpstate { /* Returns true if the options set so far are consistent. */ bool ValidateOptions() const; + + /* Returns if options specified require writing bugreport to a file */ + bool OutputToFile() const { + // If we are not writing to socket, we will write to a file. If bugreport_fd is + // specified, it is preferred. If not bugreport is written to /bugreports. + return !use_socket; + } }; // TODO: initialize fields on constructor @@ -424,13 +430,6 @@ class Dumpstate { // Full path of the temporary file containing the screenshot (when requested). std::string screenshot_path_; - // TODO(b/111441001): remove when obsolete. - // Full path of the final zip file inside the caller-specified directory, if available. - std::string final_path_; - - // The caller-specified directory, if available. - std::string bugreport_dir_; - // Pointer to the zipped file. std::unique_ptr<FILE, int (*)(FILE*)> zip_file{nullptr, fclose}; diff --git a/cmds/dumpstate/dumpstate.rc b/cmds/dumpstate/dumpstate.rc index 2e72574738..14937b80b9 100644 --- a/cmds/dumpstate/dumpstate.rc +++ b/cmds/dumpstate/dumpstate.rc @@ -17,3 +17,9 @@ service dumpstatez /system/bin/dumpstate -S -d -z \ class main disabled oneshot + +# bugreportd starts dumpstate binder service and makes it wait for a listener to connect. +service bugreportd /system/bin/dumpstate -w + class main + disabled + oneshot diff --git a/cmds/dumpstate/main.cpp b/cmds/dumpstate/main.cpp index 78aad1137b..68d373377c 100644 --- a/cmds/dumpstate/main.cpp +++ b/cmds/dumpstate/main.cpp @@ -14,8 +14,55 @@ * limitations under the License. */ +#define LOG_TAG "dumpstate" + +#include <binder/IPCThreadState.h> + +#include "DumpstateInternal.h" +#include "DumpstateService.h" #include "dumpstate.h" +namespace { + +// Returns true if we should start the service and wait for a listener +// to bind with bugreport options. +bool ShouldStartServiceAndWait(int argc, char* argv[]) { + bool do_wait = false; + int c; + // Keep flags in sync with Dumpstate::DumpOptions::Initialize. + while ((c = getopt(argc, argv, "wdho:svqzpPBRSV:")) != -1 && !do_wait) { + switch (c) { + case 'w': + do_wait = true; + break; + default: + // Ignore all other options + break; + } + } + + // Reset next index used by getopt so getopt can be called called again in Dumpstate::Run to + // parse bugreport options. + optind = 1; + return do_wait; +} + +} // namespace + int main(int argc, char* argv[]) { - return run_main(argc, argv); + if (ShouldStartServiceAndWait(argc, argv)) { + int ret; + if ((ret = android::os::DumpstateService::Start()) != android::OK) { + MYLOGE("Unable to start 'dumpstate' service: %d", ret); + exit(1); + } + MYLOGI("'dumpstate' service started and will wait for a call to startBugreport()"); + + // Waits forever for an incoming connection. + // TODO(b/111441001): should this time out? + android::IPCThreadState::self()->joinThreadPool(); + return 0; + } else { + return run_main(argc, argv); + } } diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp index c57775fac8..570c6c9b61 100644 --- a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp @@ -61,9 +61,8 @@ class DumpstateListener : public IDumpstateListener { dprintf(outFd_, "\rError %d", error_code); return binder::Status::ok(); } - binder::Status onFinished(int64_t duration_ms, const ::std::string&, - const ::std::string&) override { - dprintf(outFd_, "\rFinished in %lld", (long long) duration_ms); + binder::Status onFinished() override { + dprintf(outFd_, "\rFinished"); return binder::Status::ok(); } binder::Status onProgressUpdated(int32_t progress) override { diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp index cd9b97ff9d..eb73d41dc4 100644 --- a/cmds/dumpstate/tests/dumpstate_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_test.cpp @@ -61,8 +61,7 @@ class DumpstateListenerMock : public IDumpstateListener { public: MOCK_METHOD1(onProgress, binder::Status(int32_t progress)); MOCK_METHOD1(onError, binder::Status(int32_t error_code)); - MOCK_METHOD3(onFinished, binder::Status(int64_t duration_ms, const ::std::string& title, - const ::std::string& description)); + MOCK_METHOD0(onFinished, binder::Status()); MOCK_METHOD1(onProgressUpdated, binder::Status(int32_t progress)); MOCK_METHOD1(onMaxProgressUpdated, binder::Status(int32_t max_progress)); MOCK_METHOD4(onSectionComplete, binder::Status(const ::std::string& name, int32_t status, @@ -173,7 +172,6 @@ TEST_F(DumpOptionsTest, InitializeNone) { EXPECT_FALSE(options_.do_add_date); EXPECT_FALSE(options_.do_zip_file); - EXPECT_EQ("", options_.use_outfile); EXPECT_FALSE(options_.use_socket); EXPECT_FALSE(options_.use_control_socket); EXPECT_FALSE(options_.show_header_only); @@ -191,7 +189,6 @@ TEST_F(DumpOptionsTest, InitializeAdbBugreport) { const_cast<char*>("-S"), const_cast<char*>("-d"), const_cast<char*>("-z"), - const_cast<char*>("-o abc"), }; // clang-format on @@ -201,7 +198,6 @@ TEST_F(DumpOptionsTest, InitializeAdbBugreport) { EXPECT_TRUE(options_.do_add_date); EXPECT_TRUE(options_.do_zip_file); EXPECT_TRUE(options_.use_control_socket); - EXPECT_EQ(" abc", std::string(options_.use_outfile)); // Other options retain default values EXPECT_TRUE(options_.do_vibrate); @@ -228,7 +224,6 @@ TEST_F(DumpOptionsTest, InitializeAdbShellBugreport) { // Other options retain default values EXPECT_TRUE(options_.do_vibrate); - EXPECT_EQ("", options_.use_outfile); EXPECT_FALSE(options_.do_add_date); EXPECT_FALSE(options_.do_zip_file); EXPECT_FALSE(options_.use_control_socket); @@ -247,7 +242,6 @@ TEST_F(DumpOptionsTest, InitializeFullBugReport) { const_cast<char*>("-p"), const_cast<char*>("-B"), const_cast<char*>("-z"), - const_cast<char*>("-o abc"), }; // clang-format on property_set("dumpstate.options", "bugreportfull"); @@ -259,7 +253,6 @@ TEST_F(DumpOptionsTest, InitializeFullBugReport) { EXPECT_TRUE(options_.do_fb); EXPECT_TRUE(options_.do_zip_file); EXPECT_TRUE(options_.do_broadcast); - EXPECT_EQ(" abc", std::string(options_.use_outfile)); // Other options retain default values EXPECT_TRUE(options_.do_vibrate); @@ -279,7 +272,6 @@ TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) { const_cast<char*>("-p"), const_cast<char*>("-B"), const_cast<char*>("-z"), - const_cast<char*>("-o abc"), }; // clang-format on @@ -294,7 +286,6 @@ TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) { EXPECT_TRUE(options_.do_progress_updates); EXPECT_TRUE(options_.do_start_service); EXPECT_FALSE(options_.do_fb); - EXPECT_EQ(" abc", std::string(options_.use_outfile)); // Other options retain default values EXPECT_TRUE(options_.do_vibrate); @@ -312,7 +303,6 @@ TEST_F(DumpOptionsTest, InitializeRemoteBugReport) { const_cast<char*>("-p"), const_cast<char*>("-B"), const_cast<char*>("-z"), - const_cast<char*>("-o abc"), }; // clang-format on @@ -327,7 +317,6 @@ TEST_F(DumpOptionsTest, InitializeRemoteBugReport) { EXPECT_TRUE(options_.is_remote_mode); EXPECT_FALSE(options_.do_vibrate); EXPECT_FALSE(options_.do_fb); - EXPECT_EQ(" abc", std::string(options_.use_outfile)); // Other options retain default values EXPECT_FALSE(options_.use_control_socket); @@ -344,7 +333,6 @@ TEST_F(DumpOptionsTest, InitializeWearBugReport) { const_cast<char*>("-p"), const_cast<char*>("-B"), const_cast<char*>("-z"), - const_cast<char*>("-o abc"), }; // clang-format on @@ -359,7 +347,6 @@ TEST_F(DumpOptionsTest, InitializeWearBugReport) { EXPECT_TRUE(options_.do_zip_file); EXPECT_TRUE(options_.do_progress_updates); EXPECT_TRUE(options_.do_start_service); - EXPECT_EQ(" abc", std::string(options_.use_outfile)); // Other options retain default values EXPECT_TRUE(options_.do_vibrate); @@ -377,7 +364,6 @@ TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) { const_cast<char*>("-p"), const_cast<char*>("-B"), const_cast<char*>("-z"), - const_cast<char*>("-o abc"), }; // clang-format on @@ -391,7 +377,6 @@ TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) { EXPECT_TRUE(options_.do_broadcast); EXPECT_TRUE(options_.do_zip_file); EXPECT_TRUE(options_.telephony_only); - EXPECT_EQ(" abc", std::string(options_.use_outfile)); // Other options retain default values EXPECT_TRUE(options_.do_vibrate); @@ -410,7 +395,6 @@ TEST_F(DumpOptionsTest, InitializeWifiBugReport) { const_cast<char*>("-p"), const_cast<char*>("-B"), const_cast<char*>("-z"), - const_cast<char*>("-o abc"), }; // clang-format on @@ -424,7 +408,6 @@ TEST_F(DumpOptionsTest, InitializeWifiBugReport) { EXPECT_TRUE(options_.do_broadcast); EXPECT_TRUE(options_.do_zip_file); EXPECT_TRUE(options_.wifi_only); - EXPECT_EQ(" abc", std::string(options_.use_outfile)); // Other options retain default values EXPECT_TRUE(options_.do_vibrate); @@ -444,7 +427,6 @@ TEST_F(DumpOptionsTest, InitializeDefaultBugReport) { const_cast<char*>("-p"), const_cast<char*>("-B"), const_cast<char*>("-z"), - const_cast<char*>("-o abc"), }; // clang-format on @@ -457,7 +439,6 @@ TEST_F(DumpOptionsTest, InitializeDefaultBugReport) { EXPECT_TRUE(options_.do_fb); EXPECT_TRUE(options_.do_zip_file); EXPECT_TRUE(options_.do_broadcast); - EXPECT_EQ(" abc", std::string(options_.use_outfile)); // Other options retain default values EXPECT_TRUE(options_.do_vibrate); @@ -475,7 +456,6 @@ TEST_F(DumpOptionsTest, InitializePartial1) { const_cast<char*>("dumpstate"), const_cast<char*>("-d"), const_cast<char*>("-z"), - const_cast<char*>("-o abc"), const_cast<char*>("-s"), const_cast<char*>("-S"), @@ -488,7 +468,6 @@ TEST_F(DumpOptionsTest, InitializePartial1) { EXPECT_TRUE(options_.do_add_date); EXPECT_TRUE(options_.do_zip_file); // TODO: Maybe we should trim the filename - EXPECT_EQ(" abc", std::string(options_.use_outfile)); EXPECT_TRUE(options_.use_socket); EXPECT_TRUE(options_.use_control_socket); @@ -527,7 +506,6 @@ TEST_F(DumpOptionsTest, InitializePartial2) { // Other options retain default values EXPECT_FALSE(options_.do_add_date); EXPECT_FALSE(options_.do_zip_file); - EXPECT_EQ("", options_.use_outfile); EXPECT_FALSE(options_.use_socket); EXPECT_FALSE(options_.use_control_socket); } @@ -562,15 +540,21 @@ TEST_F(DumpOptionsTest, InitializeUnknown) { TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile1) { options_.do_zip_file = true; + // Writing to socket = !writing to file. + options_.use_socket = true; EXPECT_FALSE(options_.ValidateOptions()); - options_.use_outfile = "a/b/c"; + + options_.use_socket = false; EXPECT_TRUE(options_.ValidateOptions()); } TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile2) { options_.do_broadcast = true; + // Writing to socket = !writing to file. + options_.use_socket = true; EXPECT_FALSE(options_.ValidateOptions()); - options_.use_outfile = "a/b/c"; + + options_.use_socket = false; EXPECT_TRUE(options_.ValidateOptions()); } @@ -579,13 +563,11 @@ TEST_F(DumpOptionsTest, ValidateOptionsNeedZipfile) { EXPECT_FALSE(options_.ValidateOptions()); options_.do_zip_file = true; - options_.use_outfile = "a/b/c"; // do_zip_file needs outfile EXPECT_TRUE(options_.ValidateOptions()); } TEST_F(DumpOptionsTest, ValidateOptionsUpdateProgressNeedsBroadcast) { options_.do_progress_updates = true; - options_.use_outfile = "a/b/c"; // do_progress_updates needs outfile EXPECT_FALSE(options_.ValidateOptions()); options_.do_broadcast = true; @@ -599,7 +581,6 @@ TEST_F(DumpOptionsTest, ValidateOptionsRemoteMode) { options_.do_broadcast = true; options_.do_zip_file = true; options_.do_add_date = true; - options_.use_outfile = "a/b/c"; // do_broadcast needs outfile EXPECT_TRUE(options_.ValidateOptions()); } diff --git a/cmds/installd/Android.bp b/cmds/installd/Android.bp index 2e9701f8b0..a6f77aa086 100644 --- a/cmds/installd/Android.bp +++ b/cmds/installd/Android.bp @@ -18,6 +18,7 @@ cc_defaults { "dexopt.cpp", "globals.cpp", "utils.cpp", + "view_compiler.cpp", ":installd_aidl", ], header_libs: [ @@ -30,6 +31,7 @@ cc_defaults { "libcutils", "liblog", "liblogwrap", + "libprocessgroup", "libselinux", "libutils", ], @@ -168,6 +170,7 @@ cc_library_static { "libbase", "libcutils", "liblog", + "libprocessgroup", "libutils", ], } @@ -188,6 +191,7 @@ cc_binary { "globals.cpp", "otapreopt.cpp", "utils.cpp", + "view_compiler.cpp", ], header_libs: ["dex2oat_headers"], @@ -204,6 +208,7 @@ cc_binary { "libcutils", "liblog", "liblogwrap", + "libprocessgroup", "libselinux", "libutils", ], diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index 2439dff54e..8a036a5c1c 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -60,6 +60,7 @@ #include "installd_deps.h" #include "otapreopt_utils.h" #include "utils.h" +#include "view_compiler.h" #include "CacheTracker.h" #include "MatchExtensionGen.h" @@ -1831,6 +1832,17 @@ binder::Status InstalldNativeService::dexopt(const std::string& apkPath, int32_t return res ? error(res, error_msg) : ok(); } +binder::Status InstalldNativeService::compileLayouts(const std::string& apkPath, + const std::string& packageName, + const std ::string& outDexFile, int uid, + bool* _aidl_return) { + const char* apk_path = apkPath.c_str(); + const char* package_name = packageName.c_str(); + const char* out_dex_file = outDexFile.c_str(); + *_aidl_return = android::installd::view_compiler(apk_path, package_name, out_dex_file, uid); + return *_aidl_return ? ok() : error("viewcompiler failed"); +} + binder::Status InstalldNativeService::markBootComplete(const std::string& instructionSet) { ENFORCE_UID(AID_SYSTEM); std::lock_guard<std::recursive_mutex> lock(mLock); diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h index 367f2c1547..d482472605 100644 --- a/cmds/installd/InstalldNativeService.h +++ b/cmds/installd/InstalldNativeService.h @@ -89,6 +89,9 @@ public: const std::unique_ptr<std::string>& dexMetadataPath, const std::unique_ptr<std::string>& compilationReason); + binder::Status compileLayouts(const std::string& apkPath, const std::string& packageName, + const std::string& outDexFile, int uid, bool* _aidl_return); + binder::Status rmdex(const std::string& codePath, const std::string& instructionSet); binder::Status mergeProfiles(int32_t uid, const std::string& packageName, diff --git a/cmds/installd/binder/android/os/IInstalld.aidl b/cmds/installd/binder/android/os/IInstalld.aidl index 91e20b75ae..caa0b67a4c 100644 --- a/cmds/installd/binder/android/os/IInstalld.aidl +++ b/cmds/installd/binder/android/os/IInstalld.aidl @@ -55,6 +55,8 @@ interface IInstalld { @nullable @utf8InCpp String profileName, @nullable @utf8InCpp String dexMetadataPath, @nullable @utf8InCpp String compilationReason); + boolean compileLayouts(@utf8InCpp String apkPath, @utf8InCpp String packageName, + @utf8InCpp String outDexFile, int uid); void rmdex(@utf8InCpp String codePath, @utf8InCpp String instructionSet); diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index 32c1313f65..e3a35c71d4 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -43,6 +43,7 @@ #include <log/log.h> // TODO: Move everything to base/logging. #include <openssl/sha.h> #include <private/android_filesystem_config.h> +#include <processgroup/sched_policy.h> #include <selinux/android.h> #include <system/thread_defs.h> @@ -627,27 +628,6 @@ static void open_profile_files(uid_t uid, const std::string& package_name, } } -static void drop_capabilities(uid_t uid) { - if (setgid(uid) != 0) { - PLOG(ERROR) << "setgid(" << uid << ") failed in installd during dexopt"; - exit(DexoptReturnCodes::kSetGid); - } - if (setuid(uid) != 0) { - PLOG(ERROR) << "setuid(" << uid << ") failed in installd during dexopt"; - exit(DexoptReturnCodes::kSetUid); - } - // drop capabilities - struct __user_cap_header_struct capheader; - struct __user_cap_data_struct capdata[2]; - memset(&capheader, 0, sizeof(capheader)); - memset(&capdata, 0, sizeof(capdata)); - capheader.version = _LINUX_CAPABILITY_VERSION_3; - if (capset(&capheader, &capdata[0]) < 0) { - PLOG(ERROR) << "capset failed"; - exit(DexoptReturnCodes::kCapSet); - } -} - static constexpr int PROFMAN_BIN_RETURN_CODE_COMPILE = 0; static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION = 1; static constexpr int PROFMAN_BIN_RETURN_CODE_BAD_PROFILES = 2; diff --git a/cmds/installd/tests/Android.bp b/cmds/installd/tests/Android.bp index 739f33f50f..9c9db0f21d 100644 --- a/cmds/installd/tests/Android.bp +++ b/cmds/installd/tests/Android.bp @@ -28,6 +28,7 @@ cc_test { "libbinder", "libcrypto", "libcutils", + "libprocessgroup", "libselinux", "libutils", ], @@ -50,6 +51,7 @@ cc_test { "libbinder", "libcrypto", "libcutils", + "libprocessgroup", "libselinux", "libutils", ], @@ -72,6 +74,7 @@ cc_test { "libbinder", "libcrypto", "libcutils", + "libprocessgroup", "libselinux", "libutils", ], diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp index 74ad1841a5..bbf14cb5f7 100644 --- a/cmds/installd/utils.cpp +++ b/cmds/installd/utils.cpp @@ -20,6 +20,7 @@ #include <fcntl.h> #include <fts.h> #include <stdlib.h> +#include <sys/capability.h> #include <sys/stat.h> #include <sys/wait.h> #include <sys/xattr.h> @@ -34,6 +35,7 @@ #include <log/log.h> #include <private/android_filesystem_config.h> +#include "dexopt_return_codes.h" #include "globals.h" // extern variables. #ifndef LOG_TAG @@ -1063,5 +1065,26 @@ bool collect_profiles(std::vector<std::string>* profiles_paths) { } } +void drop_capabilities(uid_t uid) { + if (setgid(uid) != 0) { + PLOG(ERROR) << "setgid(" << uid << ") failed in installd during dexopt"; + exit(DexoptReturnCodes::kSetGid); + } + if (setuid(uid) != 0) { + PLOG(ERROR) << "setuid(" << uid << ") failed in installd during dexopt"; + exit(DexoptReturnCodes::kSetUid); + } + // drop capabilities + struct __user_cap_header_struct capheader; + struct __user_cap_data_struct capdata[2]; + memset(&capheader, 0, sizeof(capheader)); + memset(&capdata, 0, sizeof(capdata)); + capheader.version = _LINUX_CAPABILITY_VERSION_3; + if (capset(&capheader, &capdata[0]) < 0) { + PLOG(ERROR) << "capset failed"; + exit(DexoptReturnCodes::kCapSet); + } +} + } // namespace installd } // namespace android diff --git a/cmds/installd/utils.h b/cmds/installd/utils.h index d05724a8f0..206ad4158a 100644 --- a/cmds/installd/utils.h +++ b/cmds/installd/utils.h @@ -140,6 +140,8 @@ int prepare_app_cache_dir(const std::string& parent, const char* name, mode_t ta // It returns true if there were no errors at all, and false otherwise. bool collect_profiles(std::vector<std::string>* profiles_paths); +void drop_capabilities(uid_t uid); + } // namespace installd } // namespace android diff --git a/cmds/installd/view_compiler.cpp b/cmds/installd/view_compiler.cpp new file mode 100644 index 0000000000..f1ac7178f6 --- /dev/null +++ b/cmds/installd/view_compiler.cpp @@ -0,0 +1,99 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "view_compiler.h" + +#include <string> + +#include <fcntl.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <sys/wait.h> +#include <unistd.h> + +#include "utils.h" + +#include "android-base/logging.h" +#include "android-base/stringprintf.h" +#include "android-base/unique_fd.h" + +namespace android { +namespace installd { + +using base::unique_fd; + +bool view_compiler(const char* apk_path, const char* package_name, const char* out_dex_file, + int uid) { + CHECK(apk_path != nullptr); + CHECK(package_name != nullptr); + CHECK(out_dex_file != nullptr); + + // viewcompiler won't have permission to open anything, so we have to open the files first + // and pass file descriptors. + + // Open input file + unique_fd infd{open(apk_path, 0)}; + if (infd.get() < 0) { + PLOG(ERROR) << "Could not open input file: " << apk_path; + return false; + } + + // Set up output file. viewcompiler can't open outputs by fd, but it can write to stdout, so + // we close stdout and open it towards the right output. + unique_fd outfd{open(out_dex_file, O_CREAT | O_TRUNC | O_WRONLY, 0644)}; + if (outfd.get() < 0) { + PLOG(ERROR) << "Could not open output file: " << out_dex_file; + return false; + } + if (fchmod(outfd, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH) != 0) { + PLOG(ERROR) << "Could not change output file permissions"; + return false; + } + if (close(STDOUT_FILENO) != 0) { + PLOG(ERROR) << "Could not close stdout"; + return false; + } + if (dup2(outfd, STDOUT_FILENO) < 0) { + PLOG(ERROR) << "Could not duplicate output file descriptor"; + return false; + } + + // Prepare command line arguments for viewcompiler + std::string args[] = {"/system/bin/viewcompiler", + "--apk", + "--infd", + android::base::StringPrintf("%d", infd.get()), + "--dex", + "--package", + package_name}; + char* const argv[] = {const_cast<char*>(args[0].c_str()), const_cast<char*>(args[1].c_str()), + const_cast<char*>(args[2].c_str()), const_cast<char*>(args[3].c_str()), + const_cast<char*>(args[4].c_str()), const_cast<char*>(args[5].c_str()), + const_cast<char*>(args[6].c_str()), nullptr}; + + pid_t pid = fork(); + if (pid == 0) { + // Now that we've opened the files we need, drop privileges. + drop_capabilities(uid); + execv("/system/bin/viewcompiler", argv); + _exit(1); + } + + return wait_child(pid) == 0; +} + +} // namespace installd +} // namespace android
\ No newline at end of file diff --git a/cmds/installd/view_compiler.h b/cmds/installd/view_compiler.h new file mode 100644 index 0000000000..f7c6e57722 --- /dev/null +++ b/cmds/installd/view_compiler.h @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef VIEW_COMPILER_H_ +#define VIEW_COMPILER_H_ + +namespace android { +namespace installd { + +bool view_compiler(const char* apk_path, const char* package_name, const char* out_dex_file, + int uid); + +} // namespace installd +} // namespace android + +#endif // VIEW_COMPILER_H_
\ No newline at end of file diff --git a/libs/vr/libvrflinger/Android.bp b/libs/vr/libvrflinger/Android.bp index 233e0fceaa..26e8201251 100644 --- a/libs/vr/libvrflinger/Android.bp +++ b/libs/vr/libvrflinger/Android.bp @@ -47,6 +47,7 @@ sharedLibraries = [ "liblog", "libhardware", "libnativewindow", + "libprocessgroup", "libutils", "libEGL", "libGLESv1_CM", diff --git a/libs/vr/libvrflinger/vr_flinger.cpp b/libs/vr/libvrflinger/vr_flinger.cpp index 26aed4f25f..a27d58d252 100644 --- a/libs/vr/libvrflinger/vr_flinger.cpp +++ b/libs/vr/libvrflinger/vr_flinger.cpp @@ -12,9 +12,9 @@ #include <binder/IServiceManager.h> #include <binder/ProcessState.h> #include <cutils/properties.h> -#include <cutils/sched_policy.h> #include <log/log.h> #include <private/dvr/display_client.h> +#include <processgroup/sched_policy.h> #include <sys/prctl.h> #include <sys/resource.h> diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp index ce4daa5f35..22e7761323 100644 --- a/services/surfaceflinger/Android.bp +++ b/services/surfaceflinger/Android.bp @@ -44,6 +44,7 @@ cc_defaults { "liblayers_proto", "liblog", "libpdx_default_transport", + "libprocessgroup", "libprotobuf-cpp-lite", "libsync", "libtimestats_proto", @@ -173,6 +174,7 @@ cc_binary { "libhidltransport", "liblayers_proto", "liblog", + "libprocessgroup", "libsurfaceflinger", "libtimestats_proto", "libutils", @@ -205,6 +207,7 @@ cc_library_shared { "libcutils", "libdl", "liblog", + "libprocessgroup", ], product_variables: { // uses jni which may not be available in PDK diff --git a/services/surfaceflinger/main_surfaceflinger.cpp b/services/surfaceflinger/main_surfaceflinger.cpp index d0900e9a5a..aefe7048e7 100644 --- a/services/surfaceflinger/main_surfaceflinger.cpp +++ b/services/surfaceflinger/main_surfaceflinger.cpp @@ -21,13 +21,13 @@ #include <android/frameworks/displayservice/1.0/IDisplayService.h> #include <android/hardware/configstore/1.0/ISurfaceFlingerConfigs.h> #include <android/hardware/graphics/allocator/2.0/IAllocator.h> -#include <cutils/sched_policy.h> #include <binder/IServiceManager.h> #include <binder/IPCThreadState.h> #include <binder/ProcessState.h> #include <binder/IServiceManager.h> #include <displayservice/DisplayService.h> #include <hidl/LegacySupport.h> +#include <processgroup/sched_policy.h> #include <configstore/Utils.h> #include "SurfaceFlinger.h" diff --git a/services/surfaceflinger/surfaceflinger.rc b/services/surfaceflinger/surfaceflinger.rc index aea602bba4..5bec502e1d 100644 --- a/services/surfaceflinger/surfaceflinger.rc +++ b/services/surfaceflinger/surfaceflinger.rc @@ -2,6 +2,7 @@ service surfaceflinger /system/bin/surfaceflinger class core animation user system group graphics drmrpc readproc + updatable onrestart restart zygote writepid /dev/stune/foreground/tasks socket pdx/system/vr/display/client stream 0666 system graphics u:object_r:pdx_display_client_endpoint_socket:s0 |