diff options
-rw-r--r-- | debuggerd/crash_dump.cpp | 6 | ||||
-rw-r--r-- | debuggerd/tombstoned/intercept_manager.cpp | 4 | ||||
-rw-r--r-- | debuggerd/tombstoned/tombstoned.cpp | 4 | ||||
-rw-r--r-- | init/Android.bp | 1 | ||||
-rw-r--r-- | init/reboot.cpp | 4 | ||||
-rw-r--r-- | init/reboot.h | 6 | ||||
-rw-r--r-- | init/reboot_test.cpp | 186 | ||||
-rw-r--r-- | init/subcontext.cpp | 3 | ||||
-rw-r--r-- | libutils/String16_test.cpp | 14 | ||||
-rw-r--r-- | libutils/String8.cpp | 4 | ||||
-rw-r--r-- | llkd/libllkd.cpp | 3 |
11 files changed, 223 insertions, 12 deletions
diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp index 68a43cffb..230002a8e 100644 --- a/debuggerd/crash_dump.cpp +++ b/debuggerd/crash_dump.cpp @@ -153,14 +153,14 @@ static bool activity_manager_notify(pid_t pid, int signal, const std::string& am } struct timeval tv = { - .tv_sec = 1 * android::base::TimeoutMultiplier(), + .tv_sec = 1 * android::base::HwTimeoutMultiplier(), .tv_usec = 0, }; if (setsockopt(amfd.get(), SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)) == -1) { PLOG(ERROR) << "failed to set send timeout on activity manager socket"; return false; } - tv.tv_sec = 3 * android::base::TimeoutMultiplier(); // 3 seconds on handshake read + tv.tv_sec = 3 * android::base::HwTimeoutMultiplier(); // 3 seconds on handshake read if (setsockopt(amfd.get(), SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)) == -1) { PLOG(ERROR) << "failed to set receive timeout on activity manager socket"; return false; @@ -447,7 +447,7 @@ int main(int argc, char** argv) { // // Note: processes with many threads and minidebug-info can take a bit to // unwind, do not make this too small. b/62828735 - alarm(30 * android::base::TimeoutMultiplier()); + alarm(30 * android::base::HwTimeoutMultiplier()); // Get the process name (aka cmdline). std::string process_name = get_process_name(g_target_thread); diff --git a/debuggerd/tombstoned/intercept_manager.cpp b/debuggerd/tombstoned/intercept_manager.cpp index 4d4646a33..613e6f57d 100644 --- a/debuggerd/tombstoned/intercept_manager.cpp +++ b/debuggerd/tombstoned/intercept_manager.cpp @@ -163,7 +163,7 @@ static void intercept_request_cb(evutil_socket_t sockfd, short ev, void* arg) { event_assign(intercept->intercept_event, intercept_manager->base, sockfd, EV_READ | EV_TIMEOUT, intercept_close_cb, arg); - struct timeval timeout = {.tv_sec = 10 * android::base::TimeoutMultiplier(), .tv_usec = 0}; + struct timeval timeout = {.tv_sec = 10 * android::base::HwTimeoutMultiplier(), .tv_usec = 0}; event_add(intercept->intercept_event, &timeout); } @@ -179,7 +179,7 @@ static void intercept_accept_cb(evconnlistener* listener, evutil_socket_t sockfd intercept->intercept_manager = static_cast<InterceptManager*>(arg); intercept->sockfd.reset(sockfd); - struct timeval timeout = {1 * android::base::TimeoutMultiplier(), 0}; + struct timeval timeout = {1 * android::base::HwTimeoutMultiplier(), 0}; event_base* base = evconnlistener_get_base(listener); event* intercept_event = event_new(base, sockfd, EV_TIMEOUT | EV_READ, intercept_request_cb, intercept); diff --git a/debuggerd/tombstoned/tombstoned.cpp b/debuggerd/tombstoned/tombstoned.cpp index bc2d33d8f..0b87b7a08 100644 --- a/debuggerd/tombstoned/tombstoned.cpp +++ b/debuggerd/tombstoned/tombstoned.cpp @@ -320,7 +320,7 @@ static void perform_request(std::unique_ptr<Crash> crash) { } // TODO: Make this configurable by the interceptor? - struct timeval timeout = {10 * android::base::TimeoutMultiplier(), 0}; + struct timeval timeout = {10 * android::base::HwTimeoutMultiplier(), 0}; event_base* base = event_get_base(crash->crash_event); @@ -340,7 +340,7 @@ static void crash_accept_cb(evconnlistener* listener, evutil_socket_t sockfd, so // TODO: Make sure that only java crashes come in on the java socket // and only native crashes on the native socket. - struct timeval timeout = {1 * android::base::TimeoutMultiplier(), 0}; + struct timeval timeout = {1 * android::base::HwTimeoutMultiplier(), 0}; event* crash_event = event_new(base, sockfd, EV_TIMEOUT | EV_READ, crash_request_cb, crash); crash->crash_socket_fd.reset(sockfd); crash->crash_event = crash_event; diff --git a/init/Android.bp b/init/Android.bp index 1381c1d25..98e62fe0e 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -394,6 +394,7 @@ cc_test { "persistent_properties_test.cpp", "property_service_test.cpp", "property_type_test.cpp", + "reboot_test.cpp", "rlimit_parser_test.cpp", "service_test.cpp", "subcontext_test.cpp", diff --git a/init/reboot.cpp b/init/reboot.cpp index d9acee57f..2402cef51 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -533,8 +533,8 @@ static void StopServices(const std::set<std::string>& services, std::chrono::mil // Like StopServices, but also logs all the services that failed to stop after the provided timeout. // Returns number of violators. -static int StopServicesAndLogViolations(const std::set<std::string>& services, - std::chrono::milliseconds timeout, bool terminate) { +int StopServicesAndLogViolations(const std::set<std::string>& services, + std::chrono::milliseconds timeout, bool terminate) { StopServices(services, timeout, terminate); int still_running = 0; for (const auto& s : ServiceList::GetInstance()) { diff --git a/init/reboot.h b/init/reboot.h index 81c3edc12..551a114f6 100644 --- a/init/reboot.h +++ b/init/reboot.h @@ -17,11 +17,17 @@ #ifndef _INIT_REBOOT_H #define _INIT_REBOOT_H +#include <chrono> +#include <set> #include <string> namespace android { namespace init { +// Like StopServices, but also logs all the services that failed to stop after the provided timeout. +// Returns number of violators. +int StopServicesAndLogViolations(const std::set<std::string>& services, + std::chrono::milliseconds timeout, bool terminate); // Parses and handles a setprop sys.powerctl message. void HandlePowerctlMessage(const std::string& command); diff --git a/init/reboot_test.cpp b/init/reboot_test.cpp new file mode 100644 index 000000000..06702e3a6 --- /dev/null +++ b/init/reboot_test.cpp @@ -0,0 +1,186 @@ +/* + * Copyright (C) 2020 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 "reboot.h" + +#include <errno.h> +#include <unistd.h> + +#include <memory> +#include <string_view> + +#include <android-base/file.h> +#include <android-base/properties.h> +#include <android-base/strings.h> +#include <gtest/gtest.h> +#include <selinux/selinux.h> + +#include "builtin_arguments.h" +#include "builtins.h" +#include "parser.h" +#include "service_list.h" +#include "service_parser.h" +#include "subcontext.h" +#include "util.h" + +using namespace std::literals; + +using android::base::GetProperty; +using android::base::Join; +using android::base::SetProperty; +using android::base::Split; +using android::base::StringReplace; +using android::base::WaitForProperty; +using android::base::WriteStringToFd; + +namespace android { +namespace init { + +class RebootTest : public ::testing::Test { + public: + RebootTest() { + std::vector<std::string> names = GetServiceNames(); + if (!names.empty()) { + ADD_FAILURE() << "Expected empty ServiceList but found: [" << Join(names, ',') << "]"; + } + } + + ~RebootTest() { + std::vector<std::string> names = GetServiceNames(); + for (const auto& name : names) { + auto s = ServiceList::GetInstance().FindService(name); + auto pid = s->pid(); + ServiceList::GetInstance().RemoveService(*s); + if (pid > 0) { + kill(pid, SIGTERM); + kill(pid, SIGKILL); + } + } + } + + private: + std::vector<std::string> GetServiceNames() const { + std::vector<std::string> names; + for (const auto& s : ServiceList::GetInstance()) { + names.push_back(s->name()); + } + return names; + } +}; + +std::string GetSecurityContext() { + char* ctx; + if (getcon(&ctx) == -1) { + ADD_FAILURE() << "Failed to call getcon : " << strerror(errno); + } + std::string result = std::string(ctx); + freecon(ctx); + return result; +} + +void AddTestService(const std::string& name) { + static constexpr std::string_view kScriptTemplate = R"init( +service $name /system/bin/yes + user shell + group shell + seclabel $selabel +)init"; + + std::string script = StringReplace(StringReplace(kScriptTemplate, "$name", name, false), + "$selabel", GetSecurityContext(), false); + ServiceList& service_list = ServiceList::GetInstance(); + Parser parser; + parser.AddSectionParser("service", + std::make_unique<ServiceParser>(&service_list, nullptr, std::nullopt)); + + TemporaryFile tf; + ASSERT_TRUE(tf.fd != -1); + ASSERT_TRUE(WriteStringToFd(script, tf.fd)); + ASSERT_TRUE(parser.ParseConfig(tf.path)); +} + +TEST_F(RebootTest, StopServicesSIGTERM) { + AddTestService("A"); + AddTestService("B"); + + auto service_a = ServiceList::GetInstance().FindService("A"); + ASSERT_NE(nullptr, service_a); + auto service_b = ServiceList::GetInstance().FindService("B"); + ASSERT_NE(nullptr, service_b); + + ASSERT_RESULT_OK(service_a->Start()); + ASSERT_TRUE(service_a->IsRunning()); + ASSERT_RESULT_OK(service_b->Start()); + ASSERT_TRUE(service_b->IsRunning()); + + std::unique_ptr<Service> oneshot_service; + { + auto result = Service::MakeTemporaryOneshotService( + {"exec", GetSecurityContext(), "--", "/system/bin/yes"}); + ASSERT_RESULT_OK(result); + oneshot_service = std::move(*result); + } + std::string oneshot_service_name = oneshot_service->name(); + oneshot_service->Start(); + ASSERT_TRUE(oneshot_service->IsRunning()); + ServiceList::GetInstance().AddService(std::move(oneshot_service)); + + EXPECT_EQ(0, StopServicesAndLogViolations({"A", "B", oneshot_service_name}, 10s, + /* terminate= */ true)); + EXPECT_FALSE(service_a->IsRunning()); + EXPECT_FALSE(service_b->IsRunning()); + // Oneshot services are deleted from the ServiceList after they are destroyed. + auto oneshot_service_after_stop = ServiceList::GetInstance().FindService(oneshot_service_name); + EXPECT_EQ(nullptr, oneshot_service_after_stop); +} + +TEST_F(RebootTest, StopServicesSIGKILL) { + AddTestService("A"); + AddTestService("B"); + + auto service_a = ServiceList::GetInstance().FindService("A"); + ASSERT_NE(nullptr, service_a); + auto service_b = ServiceList::GetInstance().FindService("B"); + ASSERT_NE(nullptr, service_b); + + ASSERT_RESULT_OK(service_a->Start()); + ASSERT_TRUE(service_a->IsRunning()); + ASSERT_RESULT_OK(service_b->Start()); + ASSERT_TRUE(service_b->IsRunning()); + + std::unique_ptr<Service> oneshot_service; + { + auto result = Service::MakeTemporaryOneshotService( + {"exec", GetSecurityContext(), "--", "/system/bin/yes"}); + ASSERT_RESULT_OK(result); + oneshot_service = std::move(*result); + } + std::string oneshot_service_name = oneshot_service->name(); + oneshot_service->Start(); + ASSERT_TRUE(oneshot_service->IsRunning()); + ServiceList::GetInstance().AddService(std::move(oneshot_service)); + + EXPECT_EQ(0, StopServicesAndLogViolations({"A", "B", oneshot_service_name}, 10s, + /* terminate= */ false)); + EXPECT_FALSE(service_a->IsRunning()); + EXPECT_FALSE(service_b->IsRunning()); + // Oneshot services are deleted from the ServiceList after they are destroyed. + auto oneshot_service_after_stop = ServiceList::GetInstance().FindService(oneshot_service_name); + EXPECT_EQ(nullptr, oneshot_service_after_stop); +} + +} // namespace init +} // namespace android diff --git a/init/subcontext.cpp b/init/subcontext.cpp index f1fbffe60..fa48beab3 100644 --- a/init/subcontext.cpp +++ b/init/subcontext.cpp @@ -351,6 +351,9 @@ Subcontext* GetSubcontext() { } bool SubcontextChildReap(pid_t pid) { + if (!subcontext) { + return false; + } if (subcontext->pid() == pid) { if (!subcontext_terminated_by_shutdown) { subcontext->Restart(); diff --git a/libutils/String16_test.cpp b/libutils/String16_test.cpp index 2505f445d..9e02b7886 100644 --- a/libutils/String16_test.cpp +++ b/libutils/String16_test.cpp @@ -90,6 +90,13 @@ TEST(String16Test, Insert) { EXPECT_STR16EQ(u"VerifyInsert me", tmp); } +TEST(String16Test, RemoveDefault) { + String16 tmp("Verify me"); + tmp.remove(4); + EXPECT_EQ(4U, tmp.size()); + EXPECT_STR16EQ(u"Veri", tmp); +} + TEST(String16Test, Remove) { String16 tmp("Verify me"); tmp.remove(2, 6); @@ -97,6 +104,13 @@ TEST(String16Test, Remove) { EXPECT_STR16EQ(u" m", tmp); } +TEST(String16Test, RemoveOutOfBounds) { + String16 tmp("Verify me"); + tmp.remove(100, 6); + EXPECT_EQ(3U, tmp.size()); + EXPECT_STR16EQ(u" me", tmp); +} + TEST(String16Test, MakeLower) { String16 tmp("Verify Me!"); tmp.makeLower(); diff --git a/libutils/String8.cpp b/libutils/String8.cpp index 3dc2026d9..438ef83a9 100644 --- a/libutils/String8.cpp +++ b/libutils/String8.cpp @@ -313,8 +313,8 @@ status_t String8::appendFormatV(const char* fmt, va_list args) if (n > 0) { size_t oldLength = length(); - if ((size_t)n > SIZE_MAX - 1 || - oldLength > SIZE_MAX - (size_t)n - 1) { + if (n > std::numeric_limits<size_t>::max() - 1 || + oldLength > std::numeric_limits<size_t>::max() - n - 1) { return NO_MEMORY; } char* buf = lockBuffer(oldLength + n); diff --git a/llkd/libllkd.cpp b/llkd/libllkd.cpp index 9f3e21829..27a8baf3b 100644 --- a/llkd/libllkd.cpp +++ b/llkd/libllkd.cpp @@ -962,7 +962,8 @@ milliseconds llkCheck(bool checkRunning) { // // This alarm is effectively the live lock detection of llkd, as // we understandably can not monitor ourselves otherwise. - ::alarm(duration_cast<seconds>(llkTimeoutMs * 2 * android::base::TimeoutMultiplier()).count()); + ::alarm(duration_cast<seconds>(llkTimeoutMs * 2 * android::base::HwTimeoutMultiplier()) + .count()); // kernel jiffy precision fastest acquisition static timespec last; |