libandroidfw hardening for IncFs
Migrate libandroifw to using incfs::util::map_ptr to prevent processes
from crashing when parsing the resources.arsc, parsing compiled xml,
files, and retrieving resource values.
This change propagates incremental failures to the JNI level where they
are raised as ResourcesNotFoundException.
Performance of ResourcesPerfWorkloads without change (time in
nanoseconds):
[1/3] com.android.resources.perf.PerfTest#youtube: PASSED (11.883s)
youtube_ns_median: 93812805
youtube_ns_standardDeviation: 4387062
youtube_ns_mean: 94455597
[2/3] com.android.resources.perf.PerfTest#maps: PASSED (11.265s)
maps_ns_standardDeviation: 2997543
maps_ns_mean: 83480371
maps_ns_median: 82210941
[3/3] com.android.resources.perf.PerfTest#gmail: PASSED (24.963s)
gmail_ns_median: 266141091
gmail_ns_standardDeviation: 3492043
gmail_ns_mean: 267472765
With change and verification forcibly enabled for all apks
(including the framework-res.apk):
[1/3] com.android.resources.perf.PerfTest#youtube: PASSED (11.646s)
youtube_ns_median: 101999396
youtube_ns_standardDeviation: 4625782
youtube_ns_mean: 102631770
[2/3] com.android.resources.perf.PerfTest#maps: PASSED (11.286s)
maps_ns_standardDeviation: 2692088
maps_ns_mean: 91326538
maps_ns_median: 90519884
[3/3] com.android.resources.perf.PerfTest#gmail: PASSED (24.694s)
gmail_ns_median: 290284442
gmail_ns_standardDeviation: 5764632
gmail_ns_mean: 291660464
With change and verification disabled:
[1/3] com.android.resources.perf.PerfTest#youtube: PASSED (11.748s)
youtube_ns_median: 95490747
youtube_ns_standardDeviation: 7282249
youtube_ns_mean: 98442515
[2/3] com.android.resources.perf.PerfTest#maps: PASSED (10.862s)
maps_ns_standardDeviation: 4484213
maps_ns_mean: 87912988
maps_ns_median: 86325549
[3/3] com.android.resources.perf.PerfTest#gmail: PASSED (24.034s)
gmail_ns_median: 282175838
gmail_ns_standardDeviation: 6560876
gmail_ns_mean: 282869146
These tests were done on a Pixel 3 and with cpu settings configured by
libs/hwui/tests/scripts/prep_generic.sh:
Locked CPUs 4,5,6,7 to 1459200 / 2803200 KHz
Disabled CPUs 0,1,2,3
Bug: 160635104
Bug: 169423204
Test: boot device && atest ResourcesPerfWorkloads
Change-Id: I5cd1bc8a2257bffaba6ca4a1c96f4e6640106866
diff --git a/cmds/idmap2/idmap2/Lookup.cpp b/cmds/idmap2/idmap2/Lookup.cpp
index c441709..8323d0b 100644
--- a/cmds/idmap2/idmap2/Lookup.cpp
+++ b/cmds/idmap2/idmap2/Lookup.cpp
@@ -45,11 +45,8 @@
using android::ApkAssetsCookie;
using android::AssetManager2;
using android::ConfigDescription;
-using android::is_valid_resid;
-using android::kInvalidCookie;
using android::Res_value;
using android::ResStringPool;
-using android::ResTable_config;
using android::StringPiece16;
using android::base::StringPrintf;
using android::idmap2::CommandLineOptions;
@@ -59,7 +56,6 @@
using android::idmap2::Result;
using android::idmap2::Unit;
using android::idmap2::utils::ExtractOverlayManifestInfo;
-using android::util::Utf16ToUtf8;
namespace {
@@ -69,25 +65,23 @@
// first, try to parse as a hex number
char* endptr = nullptr;
- ResourceId resid;
- resid = strtol(res.c_str(), &endptr, kBaseHex);
+ const ResourceId parsed_resid = strtol(res.c_str(), &endptr, kBaseHex);
if (*endptr == '\0') {
- return resid;
+ return parsed_resid;
}
// next, try to parse as a package:type/name string
- resid = am.GetResourceId(res, "", fallback_package);
- if (is_valid_resid(resid)) {
- return resid;
+ if (auto resid = am.GetResourceId(res, "", fallback_package)) {
+ return *resid;
}
// end of the road: res could not be parsed
return Error("failed to obtain resource id for %s", res.c_str());
}
-void PrintValue(AssetManager2* const am, const Res_value& value, const ApkAssetsCookie& cookie,
+void PrintValue(AssetManager2* const am, const AssetManager2::SelectedValue& value,
std::string* const out) {
- switch (value.dataType) {
+ switch (value.type) {
case Res_value::TYPE_INT_DEC:
out->append(StringPrintf("%d", value.data));
break;
@@ -98,30 +92,21 @@
out->append(value.data != 0 ? "true" : "false");
break;
case Res_value::TYPE_STRING: {
- const ResStringPool* pool = am->GetStringPoolForCookie(cookie);
+ const ResStringPool* pool = am->GetStringPoolForCookie(value.cookie);
out->append("\"");
- size_t len;
- if (pool->isUTF8()) {
- const char* str = pool->string8At(value.data, &len);
- out->append(str, len);
- } else {
- const char16_t* str16 = pool->stringAt(value.data, &len);
- out->append(Utf16ToUtf8(StringPiece16(str16, len)));
+ if (auto str = pool->string8ObjectAt(value.data)) {
+ out->append(*str);
}
- out->append("\"");
} break;
default:
- out->append(StringPrintf("dataType=0x%02x data=0x%08x", value.dataType, value.data));
+ out->append(StringPrintf("dataType=0x%02x data=0x%08x", value.type, value.data));
break;
}
}
Result<std::string> WARN_UNUSED GetValue(AssetManager2* const am, ResourceId resid) {
- Res_value value;
- ResTable_config config;
- uint32_t flags;
- ApkAssetsCookie cookie = am->GetResource(resid, true, 0, &value, &config, &flags);
- if (cookie == kInvalidCookie) {
+ auto value = am->GetResource(resid);
+ if (!value.has_value()) {
return Error("no resource 0x%08x in asset manager", resid);
}
@@ -129,41 +114,35 @@
// TODO(martenkongstad): use optional parameter GetResource(..., std::string*
// stacktrace = NULL) instead
- out.append(StringPrintf("cookie=%d ", cookie));
+ out.append(StringPrintf("cookie=%d ", value->cookie));
out.append("config='");
- out.append(config.toString().c_str());
+ out.append(value->config.toString().c_str());
out.append("' value=");
- if (value.dataType == Res_value::TYPE_REFERENCE) {
- const android::ResolvedBag* bag = am->GetBag(static_cast<uint32_t>(value.data));
- if (bag == nullptr) {
- out.append(StringPrintf("dataType=0x%02x data=0x%08x", value.dataType, value.data));
+ if (value->type == Res_value::TYPE_REFERENCE) {
+ auto bag_result = am->GetBag(static_cast<uint32_t>(value->data));
+ if (!bag_result.has_value()) {
+ out.append(StringPrintf("dataType=0x%02x data=0x%08x", value->type, value->data));
return out;
}
+
out.append("[");
- Res_value bag_val;
- ResTable_config selected_config;
- uint32_t flags;
- uint32_t ref;
- ApkAssetsCookie bag_cookie;
+ const android::ResolvedBag* bag = bag_result.value();
for (size_t i = 0; i < bag->entry_count; ++i) {
- const android::ResolvedBag::Entry& entry = bag->entries[i];
- bag_val = entry.value;
- bag_cookie = am->ResolveReference(entry.cookie, &bag_val, &selected_config, &flags, &ref);
- if (bag_cookie == kInvalidCookie) {
- out.append(
- StringPrintf("Error: dataType=0x%02x data=0x%08x", bag_val.dataType, bag_val.data));
+ AssetManager2::SelectedValue entry(bag, bag->entries[i]);
+ if (am->ResolveReference(entry).has_value()) {
+ out.append(StringPrintf("Error: dataType=0x%02x data=0x%08x", entry.type, entry.data));
continue;
}
- PrintValue(am, bag_val, bag_cookie, &out);
+ PrintValue(am, entry, &out);
if (i != bag->entry_count - 1) {
out.append(", ");
}
}
out.append("]");
} else {
- PrintValue(am, value, cookie, &out);
+ PrintValue(am, *value, &out);
}
return out;
diff --git a/cmds/idmap2/idmap2d/Idmap2Service.h b/cmds/idmap2/idmap2d/Idmap2Service.h
index 1d2090c..0127e87 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.h
+++ b/cmds/idmap2/idmap2d/Idmap2Service.h
@@ -17,8 +17,6 @@
#ifndef IDMAP2_IDMAP2D_IDMAP2SERVICE_H_
#define IDMAP2_IDMAP2D_IDMAP2SERVICE_H_
-#include <string>
-
#include <android-base/unique_fd.h>
#include <binder/BinderService.h>
diff --git a/cmds/idmap2/include/idmap2/ResourceMapping.h b/cmds/idmap2/include/idmap2/ResourceMapping.h
index 0a58ec4..f66916c 100644
--- a/cmds/idmap2/include/idmap2/ResourceMapping.h
+++ b/cmds/idmap2/include/idmap2/ResourceMapping.h
@@ -117,7 +117,8 @@
static Result<ResourceMapping> CreateResourceMappingLegacy(const AssetManager2* target_am,
const AssetManager2* overlay_am,
const LoadedPackage* target_package,
- const LoadedPackage* overlay_package);
+ const LoadedPackage* overlay_package,
+ LogInfo& log_info);
// Removes resources that do not pass policy or overlayable checks of the target package.
void FilterOverlayableResources(const AssetManager2* target_am,
diff --git a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
index a93202a..3037a79 100644
--- a/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
+++ b/cmds/idmap2/libidmap2/PrettyPrintVisitor.cpp
@@ -100,10 +100,9 @@
stream_ << TAB << base::StringPrintf("0x%08x -> ", target_entry.target_id)
<< utils::DataTypeToString(target_entry.value.data_type);
- size_t unused;
if (target_entry.value.data_type == Res_value::TYPE_STRING) {
- auto str = string_pool.stringAt(target_entry.value.data_value - string_pool_offset, &unused);
- stream_ << " \"" << StringPiece16(str) << "\"";
+ auto str = string_pool.stringAt(target_entry.value.data_value - string_pool_offset);
+ stream_ << " \"" << str.value_or(StringPiece16(u"")) << "\"";
} else {
stream_ << " " << base::StringPrintf("0x%08x", target_entry.value.data_value);
}
diff --git a/cmds/idmap2/libidmap2/ResourceMapping.cpp b/cmds/idmap2/libidmap2/ResourceMapping.cpp
index 31f1c16..d777cbf 100644
--- a/cmds/idmap2/libidmap2/ResourceMapping.cpp
+++ b/cmds/idmap2/libidmap2/ResourceMapping.cpp
@@ -71,9 +71,10 @@
if (!target_package.DefinesOverlayable()) {
return (sDefaultPolicies & fulfilled_policies) != 0
? Result<Unit>({})
- : Error("overlay must be preinstalled, signed with the same signature as the target,"
- " or signed with the same signature as the package referenced through"
- " <overlay-config-signature>.");
+ : Error(
+ "overlay must be preinstalled, signed with the same signature as the target,"
+ " or signed with the same signature as the package referenced through"
+ " <overlay-config-signature>.");
}
const OverlayableInfo* overlayable_info = target_package.GetOverlayableInfo(target_resource);
@@ -119,32 +120,25 @@
Result<std::unique_ptr<Asset>> OpenNonAssetFromResource(const ResourceId& resource_id,
const AssetManager2& asset_manager) {
- Res_value value{};
- ResTable_config selected_config{};
- uint32_t flags;
- auto cookie =
- asset_manager.GetResource(resource_id, /* may_be_bag */ false,
- /* density_override */ 0U, &value, &selected_config, &flags);
- if (cookie == kInvalidCookie) {
+ auto value = asset_manager.GetResource(resource_id);
+ if (!value.has_value()) {
return Error("failed to find resource for id 0x%08x", resource_id);
}
- if (value.dataType != Res_value::TYPE_STRING) {
+ if (value->type != Res_value::TYPE_STRING) {
return Error("resource for is 0x%08x is not a file", resource_id);
}
- auto string_pool = asset_manager.GetStringPoolForCookie(cookie);
- size_t len;
- auto file_path16 = string_pool->stringAt(value.data, &len);
- if (file_path16 == nullptr) {
- return Error("failed to find string for index %d", value.data);
+ auto string_pool = asset_manager.GetStringPoolForCookie(value->cookie);
+ auto file = string_pool->string8ObjectAt(value->data);
+ if (!file.has_value()) {
+ return Error("failed to find string for index %d", value->data);
}
// Load the overlay resource mappings from the file specified using android:resourcesMap.
- auto file_path = String8(String16(file_path16));
- auto asset = asset_manager.OpenNonAsset(file_path.c_str(), Asset::AccessMode::ACCESS_BUFFER);
+ auto asset = asset_manager.OpenNonAsset(file->c_str(), Asset::AccessMode::ACCESS_BUFFER);
if (asset == nullptr) {
- return Error("file \"%s\" not found", file_path.c_str());
+ return Error("file \"%s\" not found", file->c_str());
}
return asset;
@@ -190,16 +184,16 @@
return Error(R"(<item> tag missing expected attribute "value")");
}
- ResourceId target_id =
+ auto target_id_result =
target_am->GetResourceId(*target_resource, "", target_package->GetPackageName());
- if (target_id == 0U) {
+ if (!target_id_result.has_value()) {
log_info.Warning(LogMessage() << "failed to find resource \"" << *target_resource
<< "\" in target resources");
continue;
}
// Retrieve the compile-time resource id of the target resource.
- target_id = REWRITE_PACKAGE(target_id, target_package_id);
+ uint32_t target_id = REWRITE_PACKAGE(*target_id_result, target_package_id);
if (overlay_resource->dataType == Res_value::TYPE_STRING) {
overlay_resource->data += string_pool_offset;
@@ -220,7 +214,7 @@
Result<ResourceMapping> ResourceMapping::CreateResourceMappingLegacy(
const AssetManager2* target_am, const AssetManager2* overlay_am,
- const LoadedPackage* target_package, const LoadedPackage* overlay_package) {
+ const LoadedPackage* target_package, const LoadedPackage* overlay_package, LogInfo& log_info) {
ResourceMapping resource_mapping;
const uint8_t target_package_id = target_package->GetPackageId();
const auto end = overlay_package->end();
@@ -234,13 +228,15 @@
// Find the resource with the same type and entry name within the target package.
const std::string full_name =
base::StringPrintf("%s:%s", target_package->GetPackageName().c_str(), name->c_str());
- ResourceId target_resource = target_am->GetResourceId(full_name);
- if (target_resource == 0U) {
+ auto target_resource_result = target_am->GetResourceId(full_name);
+ if (!target_resource_result.has_value()) {
+ log_info.Warning(LogMessage() << "failed to find resource \"" << full_name
+ << "\" in target resources");
continue;
}
// Retrieve the compile-time resource id of the target resource.
- target_resource = REWRITE_PACKAGE(target_resource, target_package_id);
+ ResourceId target_resource = REWRITE_PACKAGE(*target_resource_result, target_package_id);
resource_mapping.AddMapping(target_resource, overlay_resid,
false /* rewrite_overlay_reference */);
}
@@ -347,7 +343,9 @@
auto& string_pool = (*parser)->get_strings();
string_pool_data_length = string_pool.bytes();
string_pool_data.reset(new uint8_t[string_pool_data_length]);
- memcpy(string_pool_data.get(), string_pool.data(), string_pool_data_length);
+
+ // Overlays should not be incrementally installed, so calling unsafe_ptr is fine here.
+ memcpy(string_pool_data.get(), string_pool.data().unsafe_ptr(), string_pool_data_length);
// Offset string indices by the size of the overlay resource table string pool.
string_pool_offset = overlay_arsc->GetStringPool()->size();
@@ -358,7 +356,7 @@
// If no file is specified using android:resourcesMap, it is assumed that the overlay only
// defines resources intended to override target resources of the same type and name.
resource_mapping = CreateResourceMappingLegacy(&target_asset_manager, &overlay_asset_manager,
- target_pkg, overlay_pkg);
+ target_pkg, overlay_pkg, log_info);
}
if (!resource_mapping) {
diff --git a/cmds/idmap2/libidmap2/ResourceUtils.cpp b/cmds/idmap2/libidmap2/ResourceUtils.cpp
index 98d026b..e817140 100644
--- a/cmds/idmap2/libidmap2/ResourceUtils.cpp
+++ b/cmds/idmap2/libidmap2/ResourceUtils.cpp
@@ -72,21 +72,21 @@
}
Result<std::string> ResToTypeEntryName(const AssetManager2& am, uint32_t resid) {
- AssetManager2::ResourceName name;
- if (!am.GetResourceName(resid, &name)) {
+ const auto name = am.GetResourceName(resid);
+ if (!name.has_value()) {
return Error("no resource 0x%08x in asset manager", resid);
}
std::string out;
- if (name.type != nullptr) {
- out.append(name.type, name.type_len);
+ if (name->type != nullptr) {
+ out.append(name->type, name->type_len);
} else {
- out += Utf16ToUtf8(StringPiece16(name.type16, name.type_len));
+ out += Utf16ToUtf8(StringPiece16(name->type16, name->type_len));
}
out.append("/");
- if (name.entry != nullptr) {
- out.append(name.entry, name.entry_len);
+ if (name->entry != nullptr) {
+ out.append(name->entry, name->entry_len);
} else {
- out += Utf16ToUtf8(StringPiece16(name.entry16, name.entry_len));
+ out += Utf16ToUtf8(StringPiece16(name->entry16, name->entry_len));
}
return out;
}
diff --git a/cmds/idmap2/libidmap2/XmlParser.cpp b/cmds/idmap2/libidmap2/XmlParser.cpp
index 526a560..7c55b64 100644
--- a/cmds/idmap2/libidmap2/XmlParser.cpp
+++ b/cmds/idmap2/libidmap2/XmlParser.cpp
@@ -98,18 +98,19 @@
switch ((*value).dataType) {
case Res_value::TYPE_STRING: {
- size_t len;
- const String16 value16(parser_.getStrings().stringAt((*value).data, &len));
- return std::string(String8(value16).c_str());
+ if (auto str = parser_.getStrings().string8ObjectAt((*value).data)) {
+ return std::string(str->string());
+ }
+ break;
}
case Res_value::TYPE_INT_DEC:
case Res_value::TYPE_INT_HEX:
case Res_value::TYPE_INT_BOOLEAN: {
return std::to_string((*value).data);
}
- default:
- return Error(R"(Failed to convert attribute "%s" value to a string)", name.c_str());
}
+
+ return Error(R"(Failed to convert attribute "%s" value to a string)", name.c_str());
}
Result<Res_value> XmlParser::Node::GetAttributeValue(const std::string& name) const {