Move __system_properties_reload to LIBC from LIBC_PLATFORM
The zygote cannot have visiblity to LIBC_PLATFORM methods. Therefore,
move __system_properties_reload to LIBC, and rename it
__system_properties_zygote_reload, and indicate in comments that it
should not be used by non-zygote apps
Bug: 291814949
Test: atest CtsBionicRootTestCases
Change-Id: Iee8fa0c76b740543c05a433393f2f4bef36d6d3d
diff --git a/libc/bionic/system_property_api.cpp b/libc/bionic/system_property_api.cpp
index 847473a..8fdea59 100644
--- a/libc/bionic/system_property_api.cpp
+++ b/libc/bionic/system_property_api.cpp
@@ -29,6 +29,7 @@
#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_
#include <sys/_system_properties.h>
+#include <async_safe/CHECK.h>
#include <system_properties/prop_area.h>
#include <system_properties/system_properties.h>
@@ -131,6 +132,7 @@
}
__BIONIC_WEAK_FOR_NATIVE_BRIDGE
-int __system_properties_reload() {
+int __system_properties_zygote_reload(void) {
+ CHECK(getpid() == gettid());
return system_properties.Reload(false) ? 0 : -1;
}
diff --git a/libc/include/sys/_system_properties.h b/libc/include/sys/_system_properties.h
index 098f355..30dea89 100644
--- a/libc/include/sys/_system_properties.h
+++ b/libc/include/sys/_system_properties.h
@@ -131,13 +131,15 @@
/*
* Reloads the system properties from disk.
+ * Not intended for use by any apps except the Zygote. Should only be called from the main thread.
*
* NOTE: Any pointers received from methods such as __system_property_find should be assumed to be
* invalid after this method is called.
*
- * Returns 0 on success, -1 otherwise
+ * Returns 0 on success, -1 if the system properties failed to re-initialize (same conditions as
+ * __system properties_init)
*/
-int __system_properties_reload();
+int __system_properties_zygote_reload(void); __INTRODUCED_IN(__ANDROID_API_V__)
/* Deprecated: use __system_property_wait instead. */
uint32_t __system_property_wait_any(uint32_t __old_serial);
diff --git a/libc/libc.map.txt b/libc/libc.map.txt
index d737946..156e9ee 100644
--- a/libc/libc.map.txt
+++ b/libc/libc.map.txt
@@ -1600,6 +1600,7 @@
tzalloc;
tzfree;
wcsrtombs_l;
+ __system_properties_zygote_reload; # apex
} LIBC_U;
LIBC_PRIVATE {
@@ -1806,7 +1807,6 @@
__system_property_area_init;
__system_property_set_filename;
__system_property_update;
- __system_properties_reload;
android_fdsan_get_fd_table;
android_fdtrack_compare_exchange_hook; # llndk
android_fdtrack_get_enabled; # llndk
diff --git a/libc/system_properties/system_properties.cpp b/libc/system_properties/system_properties.cpp
index e0b771c..9dd5e35 100644
--- a/libc/system_properties/system_properties.cpp
+++ b/libc/system_properties/system_properties.cpp
@@ -382,8 +382,8 @@
} else if (is_override) {
// We already wrote the ro.*, but appcompat_override.ro.* should override that. We don't
// need to do the usual dirty bit setting, as this only happens during the init process,
- // before any readers are started.
- CHECK(getpid() == 1);
+ // before any readers are started. Check that only init or root can write appcompat props.
+ CHECK(getpid() == 1 || getuid() == 0);
atomic_thread_fence(memory_order_release);
strlcpy(other_pi->value, value, valuelen + 1);
}
diff --git a/tests/system_properties_test.cpp b/tests/system_properties_test.cpp
index 960f689..0b7f5ae 100644
--- a/tests/system_properties_test.cpp
+++ b/tests/system_properties_test.cpp
@@ -34,6 +34,7 @@
#if defined(__BIONIC__)
#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_
+#include <stdlib.h>
#include <sys/_system_properties.h>
#include <sys/mount.h>
@@ -43,6 +44,7 @@
public:
SystemPropertiesTest() : SystemProperties(false) {
appcompat_path = android::base::StringPrintf("%s/appcompat_override", dir_.path);
+ mount_path = android::base::StringPrintf("%s/__properties__", dir_.path);
mkdir(appcompat_path.c_str(), S_IRWXU | S_IXGRP | S_IXOTH);
valid_ = AreaInit(dir_.path, nullptr, true);
}
@@ -50,7 +52,8 @@
if (valid_) {
contexts_->FreeAndUnmap();
}
- umount(dir_.path);
+ umount2(dir_.path, MNT_DETACH);
+ umount2(real_sysprop_dir.c_str(), MNT_DETACH);
}
bool valid() const {
@@ -61,7 +64,13 @@
const char* get_appcompat_path() const { return appcompat_path.c_str(); }
+ const char* get_mount_path() const { return mount_path.c_str(); }
+
+ const char* get_real_sysprop_dir() const { return real_sysprop_dir.c_str(); }
+
std::string appcompat_path;
+ std::string mount_path;
+ std::string real_sysprop_dir = "/dev/__properties__";
private:
TemporaryDir dir_;
@@ -591,3 +600,93 @@
GTEST_SKIP() << "bionic-only test";
#endif // __BIONIC__
}
+
+// Note that this test affects global state of the system
+// this tests tries to mitigate this by using utime+pid
+// prefix for the property name. It is still results in
+// pollution of property service since properties cannot
+// be removed.
+//
+// Note that there is also possibility to run into "out-of-memory"
+// if this test if it is executed often enough without reboot.
+TEST(properties, __system_property_reload_no_op) {
+#if defined(__BIONIC__)
+ std::string property_name =
+ android::base::StringPrintf("debug.test.%d.%" PRId64 ".property", getpid(), NanoTime());
+ ASSERT_EQ(0, __system_property_find(property_name.c_str()));
+ ASSERT_EQ(0, __system_property_set(property_name.c_str(), "test value"));
+ ASSERT_EQ(0, __system_properties_zygote_reload());
+ const prop_info* readptr = __system_property_find(property_name.c_str());
+ std::string expected_name = property_name;
+ __system_property_read_callback(
+ readptr,
+ [](void*, const char*, const char* value, unsigned) { ASSERT_STREQ("test value", value); },
+ &expected_name);
+#else // __BIONIC__
+ GTEST_SKIP() << "bionic-only test";
+#endif // __BIONIC__
+}
+
+TEST(properties, __system_property_reload_invalid) {
+#if defined(__BIONIC__)
+ if (getuid() != 0) GTEST_SKIP() << "test requires root";
+ SystemPropertiesTest system_properties;
+
+ // Create an invalid property_info file, so the system will attempt to initialize a
+ // ContextSerialized
+ std::string property_info_file =
+ android::base::StringPrintf("%s/property_info", system_properties.get_path());
+ fclose(fopen(property_info_file.c_str(), "w"));
+ int ret = mount(system_properties.get_path(), system_properties.get_real_sysprop_dir(), nullptr,
+ MS_BIND | MS_REC, nullptr);
+ if (ret != 0) {
+ ASSERT_ERRNO(0);
+ }
+
+ ASSERT_EQ(-1, __system_properties_zygote_reload());
+#else // __BIONIC__
+ GTEST_SKIP() << "bionic-only test";
+#endif // __BIONIC__
+}
+
+// Note that this test affects global state of the system
+// this tests tries to mitigate this by using utime+pid
+// prefix for the property name. It is still results in
+// pollution of property service since properties cannot
+// be removed.
+//
+// Note that there is also possibility to run into "out-of-memory"
+// if this test if it is executed often enough without reboot.
+TEST(properties, __system_property_reload_valid) {
+#if defined(__BIONIC__)
+ if (getuid() != 0) GTEST_SKIP() << "test requires root";
+ SystemPropertiesTest system_properties;
+
+ // Copy the system properties files into the temp directory
+ std::string shell_cmd = android::base::StringPrintf(
+ "cp -r %s %s", system_properties.get_real_sysprop_dir(), system_properties.get_path());
+ system(shell_cmd.c_str());
+
+ // Write a system property to the current set of system properties
+ std::string property_name =
+ android::base::StringPrintf("debug.test.%d.%" PRId64 ".property", getpid(), NanoTime());
+ ASSERT_EQ(0, __system_property_find(property_name.c_str()));
+ ASSERT_EQ(0, __system_property_set(property_name.c_str(), "test value"));
+
+ // Mount the temp directory (which doesn't have the property we just wrote) in place of the
+ // real one
+ int ret = mount(system_properties.get_mount_path(), system_properties.get_real_sysprop_dir(),
+ nullptr, MS_BIND | MS_REC, nullptr);
+ if (ret != 0) {
+ ASSERT_ERRNO(0);
+ }
+
+ // reload system properties in the new dir, and verify the property we wrote after we copied the
+ // files isn't there
+ ASSERT_EQ(0, __system_properties_zygote_reload());
+ ASSERT_EQ(0, __system_property_find(property_name.c_str()));
+
+#else // __BIONIC__
+ GTEST_SKIP() << "bionic-only test";
+#endif // __BIONIC__
+}
diff --git a/tests/system_properties_test2.cpp b/tests/system_properties_test2.cpp
index 0953bde..0795ccd 100644
--- a/tests/system_properties_test2.cpp
+++ b/tests/system_properties_test2.cpp
@@ -28,10 +28,6 @@
#if defined(__BIONIC__)
#include <sys/system_properties.h>
-int64_t NanoTime() {
- auto t = std::chrono::time_point_cast<std::chrono::nanoseconds>(std::chrono::steady_clock::now());
- return t.time_since_epoch().count();
-}
#endif
// Note that this test affects global state of the system
diff --git a/tests/utils.cpp b/tests/utils.cpp
index 0c7c552..e123b42 100644
--- a/tests/utils.cpp
+++ b/tests/utils.cpp
@@ -89,6 +89,11 @@
}
}
+int64_t NanoTime() {
+ auto t = std::chrono::time_point_cast<std::chrono::nanoseconds>(std::chrono::steady_clock::now());
+ return t.time_since_epoch().count();
+}
+
bool operator==(const Errno& lhs, const Errno& rhs) {
return lhs.errno_ == rhs.errno_;
}
diff --git a/tests/utils.h b/tests/utils.h
index f6b7174..dcb08f5 100644
--- a/tests/utils.h
+++ b/tests/utils.h
@@ -316,6 +316,8 @@
bool IsLowRamDevice();
+int64_t NanoTime();
+
class Errno {
public:
Errno(int e) : errno_(e) {}