implement extended sysprop overrides for hide carrier info feature

benchmarks/property_benchmark.cpp

@@ -103,6 +103,9 @@ struct LocalPropertyTestState {
     if (system_properties_.appcompat_override_contexts_) {
       system_properties_.appcompat_override_contexts_->FreeAndUnmap();
     }
+    if (system_properties_.extended_override_contexts_) {
+      system_properties_.extended_override_contexts_->FreeAndUnmap();
+    }
 
     for (int i = 0; i < nprops; i++) {
       delete names[i];

libc/system_properties/include/system_properties/system_properties.h

@@ -83,13 +83,16 @@ class SystemProperties {
       MAX(sizeof(ContextsSerialized), MAX(sizeof(ContextsSplit), sizeof(ContextsPreSplit)));
   alignas(kMaxContextsAlign) char contexts_data_[kMaxContextsSize];
   alignas(kMaxContextsAlign) char appcompat_override_contexts_data_[kMaxContextsSize];
+  alignas(kMaxContextsAlign) char extended_override_contexts_data_[kMaxContextsSize];
   Contexts* contexts_;
   // See http://b/291816546#comment#3 for more explanation of appcompat_override
   Contexts* appcompat_override_contexts_;
+  Contexts* extended_override_contexts_;
 
   bool InitContexts(bool load_default_path);
 
   bool initialized_;
   PropertiesFilename properties_filename_;
   PropertiesFilename appcompat_filename_;
+  PropertiesFilename extended_filename_;
 };

libc/system_properties/system_properties.cpp

@@ -133,6 +133,19 @@ bool SystemProperties::AreaInit(const char* filename, bool* fsetxattr_failed,
     }
   }
 
+  extended_filename_ =
+      PropertiesFilename(properties_filename_.c_str(), "extended_override");
+  extended_override_contexts_ = nullptr;
+  if (access(extended_filename_.c_str(), F_OK) != -1) {
+    auto* extended_contexts = new (extended_override_contexts_data_) ContextsSerialized();
+    if (!extended_contexts->Initialize(true, extended_filename_.c_str(), fsetxattr_failed,
+                                       load_default_path)) {
+      return false;
+    } else {
+      extended_override_contexts_ = extended_contexts;
+    }
+  }
+
   initialized_ = true;
   return true;
 }
@@ -177,6 +190,26 @@ static bool is_appcompat_override(const char* name) {
   return strncmp(name, APPCOMPAT_PREFIX, strlen(APPCOMPAT_PREFIX)) == 0;
 }
 
+// needed to deny modifications to these props
+// we cant know the actually loaded props before the init
+static const char* const kHideCarrierInfoDeniedProps[] = {
+    "gsm.sim.operator.alpha",
+    "gsm.sim.operator.numeric",
+    "gsm.sim.operator.iso-country",
+    "gsm.operator.alpha",
+    "gsm.operator.numeric",
+    "gsm.operator.iso-country",
+    "gsm.operator.isroaming",
+    "gsm.sim.state",
+};
+
+static bool is_hide_carrier_info_denied(const char* name) {
+  for (const char* denied : kHideCarrierInfoDeniedProps) {
+    if (strcmp(name, denied) == 0) return true;
+  }
+  return false;
+}
+
 static bool is_read_only(const char* name) {
   return strncmp(name, "ro.", 3) == 0;
 }
@@ -276,23 +309,36 @@ int SystemProperties::Update(prop_info* pi, const char* value, unsigned int len)
     return -1;
   }
   bool have_override = appcompat_override_contexts_ != nullptr;
+  bool have_extended =
+      extended_override_contexts_ != nullptr && !is_hide_carrier_info_denied(pi->name);
 
   prop_area* serial_pa = contexts_->GetSerialPropArea();
   prop_area* override_serial_pa =
       have_override ? appcompat_override_contexts_->GetSerialPropArea() : nullptr;
+  prop_area* ext_serial_pa =
+      have_extended ? extended_override_contexts_->GetSerialPropArea() : nullptr;
   if (!serial_pa) {
     return -1;
   }
   prop_area* pa = contexts_->GetPropAreaForName(pi->name);
   prop_area* override_pa =
       have_override ? appcompat_override_contexts_->GetPropAreaForName(pi->name) : nullptr;
+  prop_area* ext_pa =
+      have_extended ? extended_override_contexts_->GetPropAreaForName(pi->name) : nullptr;
   if (__predict_false(!pa)) {
     async_safe_format_log(ANDROID_LOG_ERROR, "libc", "Could not find area for \"%s\"", pi->name);
     return -1;
   }
   CHECK(!have_override || (override_pa && override_serial_pa));
+  CHECK(!have_extended || (ext_pa && ext_serial_pa));
 
   auto* override_pi = const_cast<prop_info*>(have_override ? override_pa->find(pi->name) : nullptr);
+  auto* ext_pi = const_cast<prop_info*>(have_extended ? ext_pa->find(pi->name) : nullptr);
+  // ext_pi may be null if the prop was never mirrored into the extended area
+  // (e.g. denied by the HCI denylist, or added before the extended area existed)
+  if (have_extended && ext_pi == nullptr) {
+    have_extended = false;
+  }
 
   uint32_t serial = atomic_load_explicit(&pi->serial, memory_order_relaxed);
   unsigned int old_len = SERIAL_VALUE_LEN(serial);
@@ -305,6 +351,9 @@ int SystemProperties::Update(prop_info* pi, const char* value, unsigned int len)
   if (have_override) {
     memcpy(override_pa->dirty_backup_area(), override_pi->value, old_len + 1);
   }
+  if (have_extended) {
+    memcpy(ext_pa->dirty_backup_area(), ext_pi->value, old_len + 1);
+  }
   serial |= 1;
   atomic_store_explicit(&pi->serial, serial, memory_order_release);
   atomic_thread_fence(memory_order_release);  // Order preceding store w.r.t. memcpy().
@@ -316,13 +365,20 @@ int SystemProperties::Update(prop_info* pi, const char* value, unsigned int len)
     atomic_store_explicit(&override_pi->serial, serial, memory_order_relaxed);
     memcpy(override_pi->value, value, len + 1);
   }
+  if (have_extended) {
+    atomic_store_explicit(&ext_pi->serial, serial, memory_order_relaxed);
+    memcpy(ext_pi->value, value, len + 1);
+  }
   // Now the primary value property area is up-to-date. Let readers know that they should
   // look at the property value instead of the backup area.
   int new_serial = (len << 24) | ((serial + 1) & 0xffffff);
   atomic_store_explicit(&pi->serial, new_serial, memory_order_release);
   if (have_override) {
     atomic_store_explicit(&override_pi->serial, new_serial, memory_order_relaxed);
   }
+  if (have_extended) {
+    atomic_store_explicit(&ext_pi->serial, new_serial, memory_order_relaxed);
+  }
   // Implicitly includes a fence to ensure the serial number update becomes visible before
   // we reuse the backup area the next time.
   __futex_wake(&pi->serial, INT32_MAX);
@@ -334,6 +390,11 @@ int SystemProperties::Update(prop_info* pi, const char* value, unsigned int len)
                           atomic_load_explicit(serial_pa->serial(), memory_order_relaxed) + 1,
                           memory_order_release);
   }
+  if (have_extended) {
+    atomic_store_explicit(ext_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;
@@ -405,6 +466,44 @@ int SystemProperties::Add(const char* name, unsigned int namelen, const char* va
       CHECK(getpid() == 1 || getuid() == 0);
       atomic_thread_fence(memory_order_release);
       memcpy(other_pi->value, value, valuelen + 1);
+      // the high byte of serial encodes value length, we need to update it so readers
+      // (that use SERIAL_VALUE_LEN) return the full overridden string.
+      uint32_t old_serial = atomic_load_explicit(&other_pi->serial, memory_order_relaxed);
+      uint32_t new_serial = (valuelen << 24) | (old_serial & 0x00ffffff);
+      atomic_store_explicit(&other_pi->serial, new_serial, memory_order_release);
+
+    }
+  }
+
+  if (extended_override_contexts_ != nullptr) {
+    bool is_appcompat = is_appcompat_override(name);
+    const char* ext_name = name;
+    if (is_appcompat) {
+      ext_name += strlen(APPCOMPAT_PREFIX);
+    }
+    bool denied = is_hide_carrier_info_denied(ext_name);
+    if (!denied) {
+      prop_area* ext_pa = extended_override_contexts_->GetPropAreaForName(ext_name);
+      prop_area* ext_serial_pa = extended_override_contexts_->GetSerialPropArea();
+      CHECK(ext_pa && ext_serial_pa);
+      auto ext_pi = const_cast<prop_info*>(ext_pa->find(ext_name));
+      if (!ext_pi) {
+        if (ext_pa->add(ext_name, strlen(ext_name), value, valuelen)) {
+          atomic_store_explicit(
+              ext_serial_pa->serial(),
+              atomic_load_explicit(ext_serial_pa->serial(), memory_order_relaxed) + 1,
+              memory_order_release);
+        }
+      } else if (is_appcompat) {
+        CHECK(getpid() == 1 || getuid() == 0);
+        atomic_thread_fence(memory_order_release);
+        memcpy(ext_pi->value, value, valuelen + 1);
+        // the high byte of serial encodes value length, we need to update it so readers
+        // (that use SERIAL_VALUE_LEN) return the full overridden string.
+        uint32_t old_serial = atomic_load_explicit(&ext_pi->serial, memory_order_relaxed);
+        uint32_t new_serial = (valuelen << 24) | (old_serial & 0x00ffffff);
+        atomic_store_explicit(&ext_pi->serial, new_serial, memory_order_release);
+      }
     }
   }
 

tests/Android.bp

@@ -1305,6 +1305,54 @@ cc_test {
     ],
 }
 
+// minial static binary for just the system properties tests
+cc_test {
+    name: "bionic-sysprop-tests",
+    gtest: false,
+    defaults: [
+        "bionic_tests_defaults",
+        "large_system_property_node_defaults",
+    ],
+    host_supported: false,
+    test_suites: ["general-tests"],
+
+    srcs: [
+        "gtest_globals.cpp",
+        "gtest_main.cpp",
+        "system_properties_test.cpp",
+        "utils.cpp",
+    ],
+
+    include_dirs: [
+        "bionic/libc",
+    ],
+
+    target: {
+        bionic: {
+            whole_static_libs: [
+                "libasync_safe",
+                "libprocinfo",
+                "libsystemproperties",
+            ],
+        },
+    },
+
+    static_libs: [
+        "libm",
+        "libc",
+        "libdl",
+        "liblog",
+        "libbase",
+        "libgtest_isolated",
+    ],
+
+    static_executable: true,
+    stl: "libc++_static",
+    lto: {
+        never: true,
+    },
+}
+
 // -----------------------------------------------------------------------------
 // Tests to run on the host and linked against glibc. Run with:
 //   cd bionic/tests; mm bionic-unit-tests-glibc-run

tests/system_properties_test.cpp

@@ -43,8 +43,10 @@ class SystemPropertiesTest : public SystemProperties {
  public:
   SystemPropertiesTest() : SystemProperties(false) {
     appcompat_path = android::base::StringPrintf("%s/appcompat_override", dir_.path);
+    extended_path = android::base::StringPrintf("%s/extended_override", dir_.path);
     mount_path = android::base::StringPrintf("%s/__properties__", dir_.path);
     mkdir(appcompat_path.c_str(), S_IRWXU | S_IXGRP | S_IXOTH);
+    mkdir(extended_path.c_str(), S_IRWXU | S_IXGRP | S_IXOTH);
     valid_ = AreaInit(dir_.path, nullptr, true);
   }
   ~SystemPropertiesTest() {
@@ -63,11 +65,14 @@ class SystemPropertiesTest : public SystemProperties {
 
   const char* get_appcompat_path() const { return appcompat_path.c_str(); }
 
+  const char* get_extended_path() const { return extended_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 extended_path;
   std::string mount_path;
   std::string real_sysprop_dir = "/dev/__properties__";
 
@@ -198,6 +203,99 @@ TEST(properties, __system_property_add_appcompat) {
 #endif  // __BIONIC__
 }
 
+TEST(properties, __system_property_add_extended_override) {
+#if defined(__BIONIC__)
+    if (getuid() != 0) GTEST_SKIP() << "test requires root";
+    SystemPropertiesTest system_properties;
+    ASSERT_TRUE(system_properties.valid());
+
+    char appcompat_seed[] = "ro.appcompat_override.ro.property";
+    char real_name[] = "ro.property";
+    char denied_name[] = "gsm.sim.operator.numeric";
+    char unrelated[] = "ro.test.test";
+
+    ASSERT_EQ(0, system_properties.Add(real_name, strlen(real_name), "real", 4));
+    ASSERT_EQ(0, system_properties.Add(appcompat_seed, strlen(appcompat_seed), "compat", 6));
+    ASSERT_EQ(0, system_properties.Add(denied_name, strlen(denied_name), "123456", 6));
+    ASSERT_EQ(0, system_properties.Add(unrelated, strlen(unrelated), "value", 5));
+
+    char propvalue[PROP_VALUE_MAX];
+
+    ASSERT_EQ(4, system_properties.Get(real_name, propvalue));
+    ASSERT_STREQ(propvalue, "real");
+    ASSERT_EQ(6, system_properties.Get(denied_name, propvalue));
+    ASSERT_STREQ(propvalue, "123456");
+
+    int ret = mount(system_properties.get_extended_path(), system_properties.get_path(), nullptr,
+                    MS_BIND | MS_REC, nullptr);
+    if (ret != 0) {
+      ASSERT_ERRNO(0);
+    }
+    system_properties.Reload(true);
+
+    // real_name overridden by appcompat.
+    ASSERT_EQ(6, system_properties.Get(real_name, propvalue));
+    ASSERT_STREQ(propvalue, "compat");
+
+    // denied key blocked from extended_override
+    // on add; absent, so get returns "".
+    ASSERT_EQ(0, system_properties.Get(denied_name, propvalue));
+    ASSERT_STREQ(propvalue, "");
+
+    // unrelated prop mirrored
+    ASSERT_EQ(5, system_properties.Get(unrelated, propvalue));
+    ASSERT_STREQ(propvalue, "value");
+
+#else   // __BIONIC__
+    GTEST_SKIP() << "bionic-only test";
+#endif  // __BIONIC__
+}
+
+TEST(properties, __system_property_update_extended_override_denylist) {
+#if defined(__BIONIC__)
+    if (getuid() != 0) GTEST_SKIP() << "test requires root";
+    SystemPropertiesTest system_properties;
+    ASSERT_TRUE(system_properties.valid());
+
+    char denied_name[] = "gsm.sim.operator.numeric";
+    char allowed_name[] = "gsm.version.baseband";
+
+    ASSERT_EQ(0, system_properties.Add(denied_name, strlen(denied_name), "123456", 6));
+    ASSERT_EQ(0, system_properties.Add(allowed_name, strlen(allowed_name), "v1", 2));
+
+    const prop_info* pi = system_properties.Find(denied_name);
+    ASSERT_TRUE(pi != nullptr);
+    system_properties.Update(const_cast<prop_info*>(pi), "123456", 6);
+
+    pi = system_properties.Find(allowed_name);
+    ASSERT_TRUE(pi != nullptr);
+    system_properties.Update(const_cast<prop_info*>(pi), "v2", 2);
+
+    char propvalue[PROP_VALUE_MAX];
+
+    ASSERT_EQ(6, system_properties.Get(denied_name, propvalue));
+    ASSERT_STREQ(propvalue, "123456");
+    ASSERT_EQ(2, system_properties.Get(allowed_name, propvalue));
+    ASSERT_STREQ(propvalue, "v2");
+
+    int ret = mount(system_properties.get_extended_path(), system_properties.get_path(), nullptr,
+                    MS_BIND | MS_REC, nullptr);
+    if (ret != 0) {
+      ASSERT_ERRNO(0);
+    }
+    system_properties.Reload(true);
+
+    ASSERT_EQ(0, system_properties.Get(denied_name, propvalue));
+    ASSERT_STREQ(propvalue, "");
+
+    ASSERT_EQ(2, system_properties.Get(allowed_name, propvalue));
+    ASSERT_STREQ(propvalue, "v2");
+
+#else   // __BIONIC__
+    GTEST_SKIP() << "bionic-only test";
+#endif  // __BIONIC__
+}
+
 TEST(properties, __system_property_update) {
 #if defined(__BIONIC__)
     SystemPropertiesTest system_properties;