diff --git a/init/property_service.cpp b/init/property_service.cpp index 9249d58db..58a0a7faa 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -58,6 +58,7 @@ #include #include #include +#include #include #include #include @@ -117,12 +118,13 @@ constexpr auto DIGEST_SIZE_USED = 8; static bool persistent_properties_loaded = false; -static int property_set_fd = -1; static int from_init_socket = -1; static int init_socket = -1; static bool accept_messages = false; static std::mutex accept_messages_lock; +static std::mutex selinux_check_access_lock; static std::thread property_service_thread; +static std::thread property_service_for_system_thread; static std::unique_ptr persist_write_thread; @@ -167,6 +169,7 @@ bool CanReadProperty(const std::string& source_context, const std::string& name) ucred cr = {.pid = 0, .uid = 0, .gid = 0}; audit_data.cr = &cr; + auto lock = std::lock_guard{selinux_check_access_lock}; return selinux_check_access(source_context.c_str(), target_context, "file", "read", &audit_data) == 0; } @@ -182,10 +185,9 @@ static bool CheckMacPerms(const std::string& name, const char* target_context, audit_data.name = name.c_str(); audit_data.cr = &cr; - bool has_access = (selinux_check_access(source_context, target_context, "property_service", - "set", &audit_data) == 0); - - return has_access; + auto lock = std::lock_guard{selinux_check_access_lock}; + return selinux_check_access(source_context, target_context, "property_service", "set", + &audit_data) == 0; } void NotifyPropertyChange(const std::string& name, const std::string& value) { @@ -400,30 +402,38 @@ static std::optional PropertySet(const std::string& name, const std::s return {PROP_ERROR_INVALID_VALUE}; } - prop_info* pi = (prop_info*)__system_property_find(name.c_str()); - if (pi != nullptr) { - // ro.* properties are actually "write-once". - if (StartsWith(name, "ro.")) { - *error = "Read-only property was already set"; - return {PROP_ERROR_READ_ONLY_PROPERTY}; - } - - __system_property_update(pi, value.c_str(), valuelen); + if (name == "sys.powerctl") { + // No action here - NotifyPropertyChange will trigger the appropriate action, and since this + // can come to the second thread, we mustn't call out to the __system_property_* functions + // which support multiple readers but only one mutator. } else { - int rc = __system_property_add(name.c_str(), name.size(), value.c_str(), valuelen); - if (rc < 0) { - *error = "__system_property_add failed"; - return {PROP_ERROR_SET_FAILED}; - } - } + prop_info* pi = (prop_info*)__system_property_find(name.c_str()); + if (pi != nullptr) { + // ro.* properties are actually "write-once". + if (StartsWith(name, "ro.")) { + *error = "Read-only property was already set"; + return {PROP_ERROR_READ_ONLY_PROPERTY}; + } - bool need_persist = StartsWith(name, "persist.") || StartsWith(name, "next_boot."); - if (socket && persistent_properties_loaded && need_persist) { - if (persist_write_thread) { - persist_write_thread->Write(name, value, std::move(*socket)); - return {}; + __system_property_update(pi, value.c_str(), valuelen); + } else { + int rc = __system_property_add(name.c_str(), name.size(), value.c_str(), valuelen); + if (rc < 0) { + *error = "__system_property_add failed"; + return {PROP_ERROR_SET_FAILED}; + } + } + + // Don't write properties to disk until after we have read all default + // properties to prevent them from being overwritten by default values. + bool need_persist = StartsWith(name, "persist.") || StartsWith(name, "next_boot."); + if (socket && persistent_properties_loaded && need_persist) { + if (persist_write_thread) { + persist_write_thread->Write(name, value, std::move(*socket)); + return {}; + } + WritePersistentProperty(name, value); } - WritePersistentProperty(name, value); } NotifyPropertyChange(name, value); @@ -584,10 +594,10 @@ uint32_t HandlePropertySetNoSocket(const std::string& name, const std::string& v return *ret; } -static void handle_property_set_fd() { +static void handle_property_set_fd(int fd) { static constexpr uint32_t kDefaultSocketTimeout = 2000; /* ms */ - int s = accept4(property_set_fd, nullptr, nullptr, SOCK_CLOEXEC); + int s = accept4(fd, nullptr, nullptr, SOCK_CLOEXEC); if (s == -1) { return; } @@ -1301,7 +1311,7 @@ void CreateSerializedPropertyInfo() { mkdir(APPCOMPAT_OVERRIDE_PROP_FOLDERNAME, S_IRWXU | S_IXGRP | S_IXOTH); if (!WriteStringToFile(serialized_contexts, APPCOMPAT_OVERRIDE_PROP_TREE_FILE, 0444, 0, 0, false)) { - PLOG(ERROR) << "Unable to write vendor overrides to file"; + PLOG(ERROR) << "Unable to write appcompat override property infos to file"; } selinux_android_restorecon(APPCOMPAT_OVERRIDE_PROP_TREE_FILE, 0); } @@ -1438,19 +1448,21 @@ static void HandleInitSocket() { } } -static void PropertyServiceThread() { +static void PropertyServiceThread(int fd, bool listen_init) { Epoll epoll; if (auto result = epoll.Open(); !result.ok()) { LOG(FATAL) << result.error(); } - if (auto result = epoll.RegisterHandler(property_set_fd, handle_property_set_fd); + if (auto result = epoll.RegisterHandler(fd, std::bind(handle_property_set_fd, fd)); !result.ok()) { LOG(FATAL) << result.error(); } - if (auto result = epoll.RegisterHandler(init_socket, HandleInitSocket); !result.ok()) { - LOG(FATAL) << result.error(); + if (listen_init) { + if (auto result = epoll.RegisterHandler(init_socket, HandleInitSocket); !result.ok()) { + LOG(FATAL) << result.error(); + } } while (true) { @@ -1499,6 +1511,23 @@ void PersistWriteThread::Write(std::string name, std::string value, SocketConnec cv_.notify_all(); } +void StartThread(const char* name, int mode, int gid, std::thread& t, bool listen_init) { + int fd = -1; + if (auto result = CreateSocket(name, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, + /*passcred=*/false, /*should_listen=*/false, mode, /*uid=*/0, + /*gid=*/gid, /*socketcon=*/{}); + result.ok()) { + fd = *result; + } else { + LOG(FATAL) << "start_property_service socket creation failed: " << result.error(); + } + + listen(fd, 8); + + auto new_thread = std::thread(PropertyServiceThread, fd, listen_init); + t.swap(new_thread); +} + void StartPropertyService(int* epoll_socket) { InitPropertySet("ro.property_service.version", "2"); @@ -1510,19 +1539,9 @@ void StartPropertyService(int* epoll_socket) { init_socket = sockets[1]; StartSendingMessages(); - if (auto result = CreateSocket(PROP_SERVICE_NAME, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, - /*passcred=*/false, /*should_listen=*/false, 0666, /*uid=*/0, - /*gid=*/0, /*socketcon=*/{}); - result.ok()) { - property_set_fd = *result; - } else { - LOG(FATAL) << "start_property_service socket creation failed: " << result.error(); - } - - listen(property_set_fd, 8); - - auto new_thread = std::thread{PropertyServiceThread}; - property_service_thread.swap(new_thread); + StartThread(PROP_SERVICE_FOR_SYSTEM_NAME, 0660, AID_SYSTEM, property_service_for_system_thread, + true); + StartThread(PROP_SERVICE_NAME, 0666, 0, property_service_thread, false); auto async_persist_writes = android::base::GetBoolProperty("ro.property_service.async_persist_writes", false); diff --git a/libcutils/ashmem-dev.cpp b/libcutils/ashmem-dev.cpp index 410dbfd5e..46b8ef263 100644 --- a/libcutils/ashmem-dev.cpp +++ b/libcutils/ashmem-dev.cpp @@ -301,6 +301,12 @@ static int memfd_create_region(const char* name, size_t size) { return -1; } + // forbid size changes to match ashmem behaviour + if (fcntl(fd, F_ADD_SEALS, F_SEAL_GROW | F_SEAL_SHRINK) == -1) { + ALOGE("memfd_create(%s, %zd) F_ADD_SEALS failed: %m", name, size); + return -1; + } + if (debug_log) { ALOGE("memfd_create(%s, %zd) success. fd=%d\n", name, size, fd.get()); } @@ -352,14 +358,29 @@ error: } static int memfd_set_prot_region(int fd, int prot) { - /* Only proceed if an fd needs to be write-protected */ + int seals = fcntl(fd, F_GET_SEALS); + if (seals == -1) { + ALOGE("memfd_set_prot_region(%d, %d): F_GET_SEALS failed: %s\n", fd, prot, strerror(errno)); + return -1; + } + if (prot & PROT_WRITE) { + /* Now we want the buffer to be read-write, let's check if the buffer + * has been previously marked as read-only before, if so return error + */ + if (seals & F_SEAL_FUTURE_WRITE) { + ALOGE("memfd_set_prot_region(%d, %d): region is write protected\n", fd, prot); + errno = EINVAL; // inline with ashmem error code, if already in + // read-only mode + return -1; + } return 0; } - if (fcntl(fd, F_ADD_SEALS, F_SEAL_FUTURE_WRITE) == -1) { - ALOGE("memfd_set_prot_region(%d, %d): F_SEAL_FUTURE_WRITE seal failed: %s\n", fd, prot, - strerror(errno)); + /* We would only allow read-only for any future file operations */ + if (fcntl(fd, F_ADD_SEALS, F_SEAL_FUTURE_WRITE | F_SEAL_SEAL) == -1) { + ALOGE("memfd_set_prot_region(%d, %d): F_SEAL_FUTURE_WRITE | F_SEAL_SEAL seal failed: %s\n", + fd, prot, strerror(errno)); return -1; }