Remove some globals from system_properties
pa_size should be static to prop_area, so make it so.
__system_property_area__ was reused for various purposes, but
realistically is a deprecated symbol and this finally separates us
from it.
Bug: 36001741
Test: boot bullhead, system property unit tests
Change-Id: I39663cc3b613093fa4c728b21d8ba58754f8e105
diff --git a/libc/system_properties/context_node.cpp b/libc/system_properties/context_node.cpp
index 440b865..13cef75 100644
--- a/libc/system_properties/context_node.cpp
+++ b/libc/system_properties/context_node.cpp
@@ -28,12 +28,11 @@
#include "context_node.h"
-#include <sys/mman.h>
#include <unistd.h>
#include <async_safe/log.h>
-#include "system_property_globals.h"
+#include "property_filename.h"
// pthread_mutex_lock() calls into system_properties in the case of contention.
// This creates a risk of dead lock if any system_properties functions
@@ -96,10 +95,5 @@
}
void ContextNode::Unmap() {
- if (!pa_) {
- return;
- }
-
- munmap(pa_, pa_size);
- pa_ = nullptr;
+ prop_area::unmap_prop_area(&pa_);
}
diff --git a/libc/system_properties/contexts.h b/libc/system_properties/contexts.h
index 9910ed1..5df9d96 100644
--- a/libc/system_properties/contexts.h
+++ b/libc/system_properties/contexts.h
@@ -39,6 +39,7 @@
virtual bool Initialize(bool writable) = 0;
virtual prop_area* GetPropAreaForName(const char* name) = 0;
+ virtual prop_area* GetSerialPropArea() = 0;
virtual void ForEach(void (*propfn)(const prop_info* pi, void* cookie), void* cookie) = 0;
virtual void ResetAccess() = 0;
virtual void FreeAndUnmap() = 0;
diff --git a/libc/system_properties/contexts_pre_split.h b/libc/system_properties/contexts_pre_split.h
index 7e83a37..bbc8529 100644
--- a/libc/system_properties/contexts_pre_split.h
+++ b/libc/system_properties/contexts_pre_split.h
@@ -29,12 +29,10 @@
#ifndef SYSTEM_PROPERTIES_CONTEXTS_PRE_SPLIT_H
#define SYSTEM_PROPERTIES_CONTEXTS_PRE_SPLIT_H
-#include <sys/mman.h>
-
#include "contexts.h"
#include "prop_area.h"
#include "prop_info.h"
-#include "system_property_globals.h"
+#include "property_filename.h"
class ContextsPreSplit : public Contexts {
public:
@@ -43,16 +41,20 @@
// We'll never initialize this legacy option as writable, so don't even check the arg.
virtual bool Initialize(bool) override {
- __system_property_area__ = prop_area::map_prop_area(property_filename);
- return __system_property_area__ != nullptr;
+ pre_split_prop_area_ = prop_area::map_prop_area(property_filename);
+ return pre_split_prop_area_ != nullptr;
}
virtual prop_area* GetPropAreaForName(const char*) override {
- return __system_property_area__;
+ return pre_split_prop_area_;
+ }
+
+ virtual prop_area* GetSerialPropArea() override {
+ return pre_split_prop_area_;
}
virtual void ForEach(void (*propfn)(const prop_info* pi, void* cookie), void* cookie) override {
- __system_property_area__->foreach (propfn, cookie);
+ pre_split_prop_area_->foreach (propfn, cookie);
}
// This is a no-op for pre-split properties as there is only one property file and it is
@@ -61,11 +63,11 @@
}
virtual void FreeAndUnmap() override {
- if (__system_property_area__ != nullptr) {
- munmap(__system_property_area__, pa_size);
- __system_property_area__ = nullptr;
- }
+ prop_area::unmap_prop_area(&pre_split_prop_area_);
}
+
+ private:
+ prop_area* pre_split_prop_area_ = nullptr;
};
#endif
diff --git a/libc/system_properties/contexts_split.cpp b/libc/system_properties/contexts_split.cpp
index 964a1e4..77f2069 100644
--- a/libc/system_properties/contexts_split.cpp
+++ b/libc/system_properties/contexts_split.cpp
@@ -31,13 +31,12 @@
#include <ctype.h>
#include <stdlib.h>
#include <string.h>
-#include <sys/mman.h>
#include <sys/stat.h>
#include <async_safe/log.h>
#include "context_node.h"
-#include "system_property_globals.h"
+#include "property_filename.h"
class ContextListNode : public ContextNode {
public:
@@ -193,22 +192,22 @@
return items;
}
-static bool MapSystemPropertyArea(bool access_rw, bool* fsetxattr_failed) {
+bool ContextsSplit::MapSerialPropertyArea(bool access_rw, bool* fsetxattr_failed) {
char filename[PROP_FILENAME_MAX];
int len = async_safe_format_buffer(filename, sizeof(filename), "%s/properties_serial",
property_filename);
if (len < 0 || len > PROP_FILENAME_MAX) {
- __system_property_area__ = nullptr;
+ serial_prop_area_ = nullptr;
return false;
}
if (access_rw) {
- __system_property_area__ =
+ serial_prop_area_ =
prop_area::map_prop_area_rw(filename, "u:object_r:properties_serial:s0", fsetxattr_failed);
} else {
- __system_property_area__ = prop_area::map_prop_area(filename);
+ serial_prop_area_ = prop_area::map_prop_area(filename);
}
- return __system_property_area__;
+ return serial_prop_area_;
}
bool ContextsSplit::InitializePropertiesFromFile(const char* filename) {
@@ -300,14 +299,14 @@
open_failed = true;
}
});
- if (open_failed || !MapSystemPropertyArea(true, &fsetxattr_failed)) {
+ if (open_failed || !MapSerialPropertyArea(true, &fsetxattr_failed)) {
FreeAndUnmap();
return false;
}
return !fsetxattr_failed;
} else {
- if (!MapSystemPropertyArea(false, nullptr)) {
+ if (!MapSerialPropertyArea(false, nullptr)) {
FreeAndUnmap();
return false;
}
@@ -348,8 +347,5 @@
void ContextsSplit::FreeAndUnmap() {
ListFree(&prefixes_);
ListFree(&contexts_);
- if (__system_property_area__) {
- munmap(__system_property_area__, pa_size);
- __system_property_area__ = nullptr;
- }
+ prop_area::unmap_prop_area(&serial_prop_area_);
}
diff --git a/libc/system_properties/contexts_split.h b/libc/system_properties/contexts_split.h
index fced33c..f98eb44 100644
--- a/libc/system_properties/contexts_split.h
+++ b/libc/system_properties/contexts_split.h
@@ -41,16 +41,21 @@
virtual bool Initialize(bool writable) override;
virtual prop_area* GetPropAreaForName(const char* name) override;
+ virtual prop_area* GetSerialPropArea() override {
+ return serial_prop_area_;
+ }
virtual void ForEach(void (*propfn)(const prop_info* pi, void* cookie), void* cookie) override;
virtual void ResetAccess() override;
virtual void FreeAndUnmap() override;
private:
+ bool MapSerialPropertyArea(bool access_rw, bool* fsetxattr_failed);
bool InitializePropertiesFromFile(const char* filename);
bool InitializeProperties();
PrefixNode* prefixes_ = nullptr;
ContextListNode* contexts_ = nullptr;
+ prop_area* serial_prop_area_ = nullptr;
};
#endif
diff --git a/libc/system_properties/prop_area.cpp b/libc/system_properties/prop_area.cpp
index f6c7fa1..e11f292 100644
--- a/libc/system_properties/prop_area.cpp
+++ b/libc/system_properties/prop_area.cpp
@@ -29,7 +29,6 @@
#include <fcntl.h>
#include <stdlib.h>
#include <sys/cdefs.h>
-#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/xattr.h>
@@ -40,13 +39,13 @@
#include <async_safe/log.h>
#include "prop_area.h"
-#include "system_property_globals.h"
constexpr size_t PA_SIZE = 128 * 1024;
constexpr uint32_t PROP_AREA_MAGIC = 0x504f5250;
constexpr uint32_t PROP_AREA_VERSION = 0xfc6ed0ab;
-static size_t pa_data_size;
+size_t prop_area::pa_size_ = 0;
+size_t prop_area::pa_data_size_ = 0;
prop_area* prop_area::map_prop_area_rw(const char* filename, const char* context,
bool* fsetxattr_failed) {
@@ -89,10 +88,10 @@
return nullptr;
}
- pa_size = PA_SIZE;
- pa_data_size = pa_size - sizeof(prop_area);
+ pa_size_ = PA_SIZE;
+ pa_data_size_ = pa_size_ - sizeof(prop_area);
- void* const memory_area = mmap(nullptr, pa_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+ void* const memory_area = mmap(nullptr, pa_size_, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
if (memory_area == MAP_FAILED) {
close(fd);
return nullptr;
@@ -116,17 +115,17 @@
return nullptr;
}
- pa_size = fd_stat.st_size;
- pa_data_size = pa_size - sizeof(prop_area);
+ pa_size_ = fd_stat.st_size;
+ pa_data_size_ = pa_size_ - sizeof(prop_area);
- void* const map_result = mmap(nullptr, pa_size, PROT_READ, MAP_SHARED, fd, 0);
+ void* const map_result = mmap(nullptr, pa_size_, PROT_READ, MAP_SHARED, fd, 0);
if (map_result == MAP_FAILED) {
return nullptr;
}
prop_area* pa = reinterpret_cast<prop_area*>(map_result);
if ((pa->magic() != PROP_AREA_MAGIC) || (pa->version() != PROP_AREA_VERSION)) {
- munmap(pa, pa_size);
+ munmap(pa, pa_size_);
return nullptr;
}
@@ -145,7 +144,7 @@
void* prop_area::allocate_obj(const size_t size, uint_least32_t* const off) {
const size_t aligned = __BIONIC_ALIGN(size, sizeof(uint_least32_t));
- if (bytes_used_ + aligned > pa_data_size) {
+ if (bytes_used_ + aligned > pa_data_size_) {
return nullptr;
}
@@ -195,7 +194,7 @@
}
void* prop_area::to_prop_obj(uint_least32_t off) {
- if (off > pa_data_size) return nullptr;
+ if (off > pa_data_size_) return nullptr;
return (data_ + off);
}
diff --git a/libc/system_properties/prop_area.h b/libc/system_properties/prop_area.h
index 6b74e10..10c1adb 100644
--- a/libc/system_properties/prop_area.h
+++ b/libc/system_properties/prop_area.h
@@ -29,6 +29,7 @@
#include <stdatomic.h>
#include <stdint.h>
#include <string.h>
+#include <sys/mman.h>
#include "private/bionic_macros.h"
@@ -94,6 +95,12 @@
static prop_area* map_prop_area_rw(const char* filename, const char* context,
bool* fsetxattr_failed);
static prop_area* map_prop_area(const char* filename);
+ static void unmap_prop_area(prop_area** pa) {
+ if (*pa) {
+ munmap(*pa, pa_size_);
+ *pa = nullptr;
+ }
+ }
prop_area(const uint32_t magic, const uint32_t version) : magic_(magic), version_(version) {
atomic_init(&serial_, 0);
@@ -138,6 +145,13 @@
bool foreach_property(prop_bt* const trie, void (*propfn)(const prop_info* pi, void* cookie),
void* cookie);
+ // The original design doesn't include pa_size or pa_data_size in the prop_area struct itself.
+ // Since we'll need to be backwards compatible with that design, we don't gain much by adding it
+ // now, especially since we don't have any plans to make different property areas different sizes,
+ // and thus we share these two variables among all instances.
+ static size_t pa_size_;
+ static size_t pa_data_size_;
+
uint32_t bytes_used_;
atomic_uint_least32_t serial_;
uint32_t magic_;
diff --git a/libc/system_properties/system_property_globals.h b/libc/system_properties/property_filename.h
similarity index 85%
rename from libc/system_properties/system_property_globals.h
rename to libc/system_properties/property_filename.h
index e37a4ac..0f7bcf7 100644
--- a/libc/system_properties/system_property_globals.h
+++ b/libc/system_properties/property_filename.h
@@ -26,18 +26,12 @@
* SUCH DAMAGE.
*/
-#ifndef SYSTEM_PROPERTIES_SYSTEM_PROPERTY_GLOBALS_H
-#define SYSTEM_PROPERTIES_SYSTEM_PROPERTY_GLOBALS_H
+#ifndef SYSTEM_PROPERTIES_PROPERTY_FILENAME_H
+#define SYSTEM_PROPERTIES_PROPERTY_FILENAME_H
-#include "prop_area.h"
-
-// These were essentially globals before the refactoring.
-
+// These are globals set by __system_property_set_filename().
+// There isn't huge benefit in refactoring them, so they're alone in this header.
constexpr int PROP_FILENAME_MAX = 1024;
-
extern char property_filename[PROP_FILENAME_MAX];
-extern size_t pa_size;
-
-extern prop_area* __system_property_area__;
#endif
diff --git a/libc/system_properties/system_properties.cpp b/libc/system_properties/system_properties.cpp
index 5d13404..3f6601d 100644
--- a/libc/system_properties/system_properties.cpp
+++ b/libc/system_properties/system_properties.cpp
@@ -60,7 +60,7 @@
#include "contexts_split.h"
#include "prop_area.h"
#include "prop_info.h"
-#include "system_property_globals.h"
+#include "property_filename.h"
// We don't want to use new or malloc in properties (b/31659220), and since these classes are
// small enough and don't have non-trivial constructors, it's easier to just statically declare
@@ -76,11 +76,11 @@
static const char* kServiceVersionPropertyName = "ro.property_service.version";
// This is public because it was exposed in the NDK. As of 2017-01, ~60 apps reference this symbol.
+// It is set to nullptr and never modified.
__BIONIC_WEAK_VARIABLE_FOR_NATIVE_BRIDGE
prop_area* __system_property_area__ = nullptr;
char property_filename[PROP_FILENAME_MAX] = PROP_FILENAME;
-size_t pa_size;
class PropertyServiceConnection {
public:
@@ -314,10 +314,15 @@
__BIONIC_WEAK_FOR_NATIVE_BRIDGE
uint32_t __system_property_area_serial() {
- prop_area* pa = __system_property_area__;
+ if (contexts == nullptr) {
+ return -1;
+ }
+
+ prop_area* pa = contexts->GetSerialPropArea();
if (!pa) {
return -1;
}
+
// Make sure this read fulfilled before __system_property_serial
return atomic_load_explicit(pa->serial(), memory_order_acquire);
}
@@ -521,8 +526,11 @@
return -1;
}
- prop_area* pa = __system_property_area__;
+ if (contexts == nullptr) {
+ return -1;
+ }
+ prop_area* pa = contexts->GetSerialPropArea();
if (!pa) {
return -1;
}
@@ -557,12 +565,16 @@
return -1;
}
- if (__system_property_area__ == nullptr || contexts == nullptr) {
+ if (contexts == nullptr) {
+ return -1;
+ }
+
+ prop_area* serial_pa = contexts->GetSerialPropArea();
+ if (serial_pa == nullptr) {
return -1;
}
prop_area* pa = contexts->GetPropAreaForName(name);
-
if (!pa) {
async_safe_format_log(ANDROID_LOG_ERROR, "libc", "Access denied adding property \"%s\"", name);
return -1;
@@ -575,11 +587,10 @@
// There is only a single mutator, but we want to make sure that
// updates are visible to a reader waiting for the update.
- atomic_store_explicit(
- __system_property_area__->serial(),
- atomic_load_explicit(__system_property_area__->serial(), memory_order_relaxed) + 1,
- memory_order_release);
- __futex_wake(__system_property_area__->serial(), INT32_MAX);
+ atomic_store_explicit(serial_pa->serial(),
+ atomic_load_explicit(serial_pa->serial(), memory_order_relaxed) + 1,
+ memory_order_release);
+ __futex_wake(serial_pa->serial(), INT32_MAX);
return 0;
}
@@ -607,8 +618,16 @@
// Are we waiting on the global serial or a specific serial?
atomic_uint_least32_t* serial_ptr;
if (pi == nullptr) {
- if (__system_property_area__ == nullptr) return -1;
- serial_ptr = __system_property_area__->serial();
+ if (contexts == nullptr) {
+ return -1;
+ }
+
+ prop_area* serial_pa = contexts->GetSerialPropArea();
+ if (serial_pa == nullptr) {
+ return -1;
+ }
+
+ serial_ptr = serial_pa->serial();
} else {
serial_ptr = const_cast<atomic_uint_least32_t*>(&pi->serial);
}