Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libandroidfw: Remove pre-verification

Browse Source 
This added more up-front cost to loading an APK and didn't provide
a significant benefit to resource retrieval.

Test: make libandroidfw_tests
Change-Id: Idbf993abc433fa8c8950d106c66469b310b66f7f
This commit is contained in:
Adam Lesinski
2017-11-29 13:24:29 -08:00
parent 36f46a8731
commit 498f6053da
8 changed files with 35 additions and 212 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
libs/androidfw/AssetManager2.cpp
View File

@@ -299,10 +299,9 @@ ApkAssetsCookie AssetManager2::FindEntry(uint32_t resid, uint16_t density_overri
const PackageGroup& package_group = package_groups_[idx];
const size_t package_count = package_group.packages_.size();
FindEntryResult current_entry;
for (size_t i = 0; i < package_count; i++) {
const LoadedPackage* loaded_package = package_group.packages_[i];
FindEntryResult current_entry;
if (!loaded_package->FindEntry(type_idx, entry_id, *desired_config, &current_entry)) {
continue;
}
@@ -394,7 +393,7 @@ ApkAssetsCookie AssetManager2::GetResource(uint32_t resid, bool may_be_bag,
return kInvalidCookie;
}
if (dtohl(entry.entry->flags) & ResTable_entry::FLAG_COMPLEX) {
if (dtohs(entry.entry->flags) & ResTable_entry::FLAG_COMPLEX) {
if (!may_be_bag) {
LOG(ERROR) << base::StringPrintf("Resource %08x is a complex map type.", resid);
return kInvalidCookie;

127
libs/androidfw/LoadedArsc.cpp
View File

@@ -129,9 +129,14 @@ LoadedPackage::~LoadedPackage() = default;
// Precondition: The header passed in has already been verified, so reading any fields and trusting
// the ResChunk_header is safe.
static bool VerifyResTableType(const ResTable_type* header) {
if (header->id == 0) {
LOG(ERROR) << "RES_TABLE_TYPE_TYPE has invalid ID 0.";
return false;
}
const size_t entry_count = dtohl(header->entryCount);
if (entry_count > std::numeric_limits<uint16_t>::max()) {
LOG(ERROR) << "Too many entries in RES_TABLE_TYPE_TYPE.";
LOG(ERROR) << "RES_TABLE_TYPE_TYPE has too many entries (" << entry_count << ").";
return false;
}
@@ -141,17 +146,17 @@ static bool VerifyResTableType(const ResTable_type* header) {
const size_t offsets_length = sizeof(uint32_t) * entry_count;
if (offsets_offset > entries_offset || entries_offset - offsets_offset < offsets_length) {
LOG(ERROR) << "Entry offsets overlap actual entry data.";
LOG(ERROR) << "RES_TABLE_TYPE_TYPE entry offsets overlap actual entry data.";
return false;
}
if (entries_offset > dtohl(header->header.size)) {
LOG(ERROR) << "Entry offsets extend beyond chunk.";
LOG(ERROR) << "RES_TABLE_TYPE_TYPE entry offsets extend beyond chunk.";
return false;
}
if (entries_offset & 0x03) {
LOG(ERROR) << "Entries start at unaligned address.";
LOG(ERROR) << "RES_TABLE_TYPE_TYPE entries start at unaligned address.";
return false;
}
return true;
@@ -236,7 +241,6 @@ static bool VerifyResTableEntry(const ResTable_type* type, uint32_t entry_offset
return true;
}
template <bool Verified>
bool LoadedPackage::FindEntry(const TypeSpecPtr& type_spec_ptr, uint16_t entry_idx,
const ResTable_config& config, FindEntryResult* out_entry) const {
const ResTable_config* best_config = nullptr;
@@ -254,13 +258,6 @@ bool LoadedPackage::FindEntry(const TypeSpecPtr& type_spec_ptr, uint16_t entry_i
const size_t entry_count = dtohl(type_chunk->entryCount);
const size_t offsets_offset = dtohs(type_chunk->header.headerSize);
// If the package hasn't been verified, do bounds checking.
if (!Verified) {
if (!VerifyResTableType(type_chunk)) {
continue;
}
}
// Check if there is the desired entry in this type.
if (type_chunk->flags & ResTable_type::FLAG_SPARSE) {
@@ -282,7 +279,7 @@ bool LoadedPackage::FindEntry(const TypeSpecPtr& type_spec_ptr, uint16_t entry_i
// Extract the offset from the entry. Each offset must be a multiple of 4 so we store it as
// the real offset divided by 4.
best_offset = dtohs(result->offset) * 4u;
best_offset = uint32_t{dtohs(result->offset)} * 4u;
} else {
if (entry_idx >= entry_count) {
// This entry cannot be here.
@@ -309,10 +306,8 @@ bool LoadedPackage::FindEntry(const TypeSpecPtr& type_spec_ptr, uint16_t entry_i
return false;
}
if (!Verified) {
if (!VerifyResTableEntry(best_type, best_offset, entry_idx)) {
return false;
}
if (UNLIKELY(!VerifyResTableEntry(best_type, best_offset, entry_idx))) {
return false;
}
const ResTable_entry* best_entry = reinterpret_cast<const ResTable_entry*>(
@@ -334,7 +329,7 @@ bool LoadedPackage::FindEntry(uint8_t type_idx, uint16_t entry_idx, const ResTab
// If the type IDs are offset in this package, we need to take that into account when searching
// for a type.
const TypeSpecPtr& ptr = type_specs_[type_idx - type_id_offset_];
if (ptr == nullptr) {
if (UNLIKELY(ptr == nullptr)) {
return false;
}
@@ -345,54 +340,7 @@ bool LoadedPackage::FindEntry(uint8_t type_idx, uint16_t entry_idx, const ResTab
return false;
}
}
// Don't bother checking if the entry ID is larger than the number of entries.
if (entry_idx >= dtohl(ptr->type_spec->entryCount)) {
return false;
}
if (verified_) {
return FindEntry<true>(ptr, entry_idx, config, out_entry);
}
return FindEntry<false>(ptr, entry_idx, config, out_entry);
}
static bool VerifyType(const Chunk& chunk) {
ATRACE_CALL();
const ResTable_type* header = chunk.header<ResTable_type, kResTableTypeMinSize>();
if (!VerifyResTableType(header)) {
return false;
}
const size_t entry_count = dtohl(header->entryCount);
const size_t offsets_offset = chunk.header_size();
if (header->flags & ResTable_type::FLAG_SPARSE) {
// This is encoded as a sparse map, so perform a binary search.
const ResTable_sparseTypeEntry* sparse_indices =
reinterpret_cast<const ResTable_sparseTypeEntry*>(reinterpret_cast<const uint8_t*>(header) +
offsets_offset);
for (size_t i = 0; i < entry_count; i++) {
const uint32_t offset = uint32_t{dtohs(sparse_indices[i].offset)} * 4u;
if (!VerifyResTableEntry(header, offset, i)) {
return false;
}
}
} else {
// Check each entry offset.
const uint32_t* offsets = reinterpret_cast<const uint32_t*>(
reinterpret_cast<const uint8_t*>(header) + offsets_offset);
for (size_t i = 0; i < entry_count; i++) {
uint32_t offset = dtohl(offsets[i]);
if (offset != ResTable_type::NO_ENTRY) {
if (!VerifyResTableEntry(header, offset, i)) {
return false;
}
}
}
}
return true;
return FindEntry(ptr, entry_idx, config, out_entry);
}
void LoadedPackage::CollectConfigurations(bool exclude_mipmap,
@@ -507,7 +455,7 @@ std::unique_ptr<const LoadedPackage> LoadedPackage::Load(const Chunk& chunk,
sizeof(ResTable_package) - sizeof(ResTable_package::typeIdOffset);
const ResTable_package* header = chunk.header<ResTable_package, kMinPackageSize>();
if (header == nullptr) {
LOG(ERROR) << "Chunk RES_TABLE_PACKAGE_TYPE is too small.";
LOG(ERROR) << "RES_TABLE_PACKAGE_TYPE too small.";
return {};
}
@@ -529,7 +477,7 @@ std::unique_ptr<const LoadedPackage> LoadedPackage::Load(const Chunk& chunk,
if (header->header.headerSize >= sizeof(ResTable_package)) {
uint32_t type_id_offset = dtohl(header->typeIdOffset);
if (type_id_offset > std::numeric_limits<uint8_t>::max()) {
LOG(ERROR) << "Type ID offset in RES_TABLE_PACKAGE_TYPE is too large.";
LOG(ERROR) << "RES_TABLE_PACKAGE_TYPE type ID offset too large.";
return {};
}
loaded_package->type_id_offset_ = static_cast<int>(type_id_offset);
@@ -560,7 +508,7 @@ std::unique_ptr<const LoadedPackage> LoadedPackage::Load(const Chunk& chunk,
status_t err = loaded_package->type_string_pool_.setTo(
child_chunk.header<ResStringPool_header>(), child_chunk.size());
if (err != NO_ERROR) {
LOG(ERROR) << "Corrupt package type string pool.";
LOG(ERROR) << "RES_STRING_POOL_TYPE for types corrupt.";
return {};
}
} else if (pool_address == header_address + dtohl(header->keyStrings)) {
@@ -568,11 +516,11 @@ std::unique_ptr<const LoadedPackage> LoadedPackage::Load(const Chunk& chunk,
status_t err = loaded_package->key_string_pool_.setTo(
child_chunk.header<ResStringPool_header>(), child_chunk.size());
if (err != NO_ERROR) {
LOG(ERROR) << "Corrupt package key string pool.";
LOG(ERROR) << "RES_STRING_POOL_TYPE for keys corrupt.";
return {};
}
} else {
LOG(WARNING) << "Too many string pool chunks found in package.";
LOG(WARNING) << "Too many RES_STRING_POOL_TYPEs found in RES_TABLE_PACKAGE_TYPE.";
}
} break;
@@ -603,18 +551,18 @@ std::unique_ptr<const LoadedPackage> LoadedPackage::Load(const Chunk& chunk,
const ResTable_typeSpec* type_spec = child_chunk.header<ResTable_typeSpec>();
if (type_spec == nullptr) {
LOG(ERROR) << "Chunk RES_TABLE_TYPE_SPEC_TYPE is too small.";
LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE too small.";
return {};
}
if (type_spec->id == 0) {
LOG(ERROR) << "Chunk RES_TABLE_TYPE_SPEC_TYPE has invalid ID 0.";
LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE has invalid ID 0.";
return {};
}
if (loaded_package->type_id_offset_ + static_cast<int>(type_spec->id) >
std::numeric_limits<uint8_t>::max()) {
LOG(ERROR) << "Chunk RES_TABLE_TYPE_SPEC_TYPE has out of range ID.";
LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE has out of range ID.";
return {};
}
@@ -626,12 +574,12 @@ std::unique_ptr<const LoadedPackage> LoadedPackage::Load(const Chunk& chunk,
// There can only be 2^16 entries in a type, because that is the ID
// space for entries (EEEE) in the resource ID 0xPPTTEEEE.
if (entry_count > std::numeric_limits<uint16_t>::max()) {
LOG(ERROR) << "Too many entries in RES_TABLE_TYPE_SPEC_TYPE: " << entry_count << ".";
LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE has too many entries (" << entry_count << ").";
return {};
}
if (entry_count * sizeof(uint32_t) > chunk.data_size()) {
LOG(ERROR) << "Chunk too small to hold entries in RES_TABLE_TYPE_SPEC_TYPE.";
LOG(ERROR) << "RES_TABLE_TYPE_SPEC_TYPE too small to hold entries.";
return {};
}
@@ -650,41 +598,32 @@ std::unique_ptr<const LoadedPackage> LoadedPackage::Load(const Chunk& chunk,
case RES_TABLE_TYPE_TYPE: {
const ResTable_type* type = child_chunk.header<ResTable_type, kResTableTypeMinSize>();
if (type == nullptr) {
LOG(ERROR) << "Chunk RES_TABLE_TYPE_TYPE is too small.";
LOG(ERROR) << "RES_TABLE_TYPE_TYPE too small.";
return {};
}
if (type->id == 0) {
LOG(ERROR) << "Chunk RES_TABLE_TYPE_TYPE has invalid ID 0.";
if (!VerifyResTableType(type)) {
return {};
}
// Type chunks must be preceded by their TypeSpec chunks.
if (!types_builder || type->id - 1 != last_type_idx) {
LOG(ERROR) << "Found RES_TABLE_TYPE_TYPE chunk without RES_TABLE_TYPE_SPEC_TYPE.";
LOG(ERROR) << "RES_TABLE_TYPE_TYPE found without preceding RES_TABLE_TYPE_SPEC_TYPE.";
return {};
}
// Only verify the type if we haven't already failed verification.
if (loaded_package->verified_) {
if (!VerifyType(child_chunk)) {
LOG(WARNING) << "Package failed verification, resource retrieval may be slower";
loaded_package->verified_ = false;
}
}
types_builder->AddType(type);
} break;
case RES_TABLE_LIBRARY_TYPE: {
const ResTable_lib_header* lib = child_chunk.header<ResTable_lib_header>();
if (lib == nullptr) {
LOG(ERROR) << "Chunk RES_TABLE_LIBRARY_TYPE is too small.";
LOG(ERROR) << "RES_TABLE_LIBRARY_TYPE too small.";
return {};
}
if (child_chunk.data_size() / sizeof(ResTable_lib_entry) < dtohl(lib->count)) {
LOG(ERROR) << "Chunk too small to hold entries in RES_TABLE_LIBRARY_TYPE.";
LOG(ERROR) << "RES_TABLE_LIBRARY_TYPE too small to hold entries.";
return {};
}
@@ -751,7 +690,7 @@ bool LoadedArsc::FindEntry(uint32_t resid, const ResTable_config& config,
const uint8_t type_id = get_type_id(resid);
const uint16_t entry_id = get_entry_id(resid);
if (type_id == 0) {
if (UNLIKELY(type_id == 0)) {
LOG(ERROR) << base::StringPrintf("Invalid ID 0x%08x.", resid);
return false;
}
@@ -769,7 +708,7 @@ bool LoadedArsc::LoadTable(const Chunk& chunk, const LoadedIdmap* loaded_idmap,
ATRACE_CALL();
const ResTable_header* header = chunk.header<ResTable_header>();
if (header == nullptr) {
LOG(ERROR) << "Chunk RES_TABLE_TYPE is too small.";
LOG(ERROR) << "RES_TABLE_TYPE too small.";
return false;
}
@@ -788,11 +727,11 @@ bool LoadedArsc::LoadTable(const Chunk& chunk, const LoadedIdmap* loaded_idmap,
status_t err = global_string_pool_.setTo(child_chunk.header<ResStringPool_header>(),
child_chunk.size());
if (err != NO_ERROR) {
LOG(ERROR) << "Corrupt string pool.";
LOG(ERROR) << "RES_STRING_POOL_TYPE corrupt.";
return false;
}
} else {
LOG(WARNING) << "Multiple string pool chunks found in resource table.";
LOG(WARNING) << "Multiple RES_STRING_POOL_TYPEs found in RES_TABLE_TYPE.";
}
break;

7
libs/androidfw/include/androidfw/LoadedArsc.h
View File

@@ -119,11 +119,6 @@ class LoadedPackage {
return overlay_;
}
// Returns true if this package is verified to be valid.
inline bool IsVerified() const {
return verified_;
}
// Returns the map of package name to package ID used in this LoadedPackage. At runtime, a
// package could have been assigned a different package ID than what this LoadedPackage was
// compiled with. AssetManager rewrites the package IDs so that they are compatible at runtime.
@@ -145,7 +140,6 @@ class LoadedPackage {
LoadedPackage();
template <bool Verified>
bool FindEntry(const util::unique_cptr<TypeSpec>& type_spec_ptr, uint16_t entry_idx,
const ResTable_config& config, FindEntryResult* out_entry) const;
@@ -157,7 +151,6 @@ class LoadedPackage {
bool dynamic_ = false;
bool system_ = false;
bool overlay_ = false;
bool verified_ = true;
ByteBucketArray<util::unique_cptr<TypeSpec>> type_specs_;
std::vector<DynamicPackageEntry> dynamic_package_map_;

16
libs/androidfw/tests/ApkAssets_test.cpp
View File

@@ -39,7 +39,6 @@ TEST(ApkAssetsTest, LoadApk) {
const LoadedPackage* loaded_package = loaded_arsc->GetPackageForId(0x7f010000);
ASSERT_NE(nullptr, loaded_package);
EXPECT_TRUE(loaded_package->IsVerified());
std::unique_ptr<Asset> asset = loaded_apk->Open("res/layout/main.xml");
ASSERT_NE(nullptr, asset);
@@ -59,7 +58,6 @@ TEST(ApkAssetsTest, LoadApkFromFd) {
const LoadedPackage* loaded_package = loaded_arsc->GetPackageForId(0x7f010000);
ASSERT_NE(nullptr, loaded_package);
EXPECT_TRUE(loaded_package->IsVerified());
std::unique_ptr<Asset> asset = loaded_apk->Open("res/layout/main.xml");
ASSERT_NE(nullptr, asset);
@@ -114,20 +112,6 @@ TEST(ApkAssetsTest, LoadApkWithIdmap) {
ASSERT_NE(nullptr, loaded_overlay_apk);
}
TEST(ApkAssetsTest, LoadUnverifiableApk) {
std::unique_ptr<const ApkAssets> loaded_apk =
ApkAssets::Load(GetTestDataPath() + "/unverified/unverified.apk");
ASSERT_NE(nullptr, loaded_apk);
const LoadedArsc* loaded_arsc = loaded_apk->GetLoadedArsc();
ASSERT_NE(nullptr, loaded_arsc);
const LoadedPackage* loaded_package = loaded_arsc->GetPackageForId(0x7f010000);
ASSERT_NE(nullptr, loaded_package);
EXPECT_FALSE(loaded_package->IsVerified());
}
TEST(ApkAssetsTest, CreateAndDestroyAssetKeepsApkAssetsOpen) {
std::unique_ptr<const ApkAssets> loaded_apk =
ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk");

32
libs/androidfw/tests/AssetManager2_bench.cpp
View File

@@ -26,12 +26,10 @@
#include "data/basic/R.h"
#include "data/libclient/R.h"
#include "data/styles/R.h"
#include "data/unverified/R.h"
namespace app = com::android::app;
namespace basic = com::android::basic;
namespace libclient = com::android::libclient;
namespace unverified = com::android::unverified;
namespace android {
@@ -95,12 +93,6 @@ static void BM_AssetManagerGetResourceOld(benchmark::State& state) {
}
BENCHMARK(BM_AssetManagerGetResourceOld);
static void BM_AssetManagerGetResourceUnverified(benchmark::State& state) {
GetResourceBenchmark({GetTestDataPath() + "/unverified/unverified.apk"}, nullptr /*config*/,
unverified::R::integer::number1, state);
}
BENCHMARK(BM_AssetManagerGetResourceUnverified);
static void BM_AssetManagerGetLibraryResource(benchmark::State& state) {
GetResourceBenchmark(
{GetTestDataPath() + "/lib_two/lib_two.apk", GetTestDataPath() + "/lib_one/lib_one.apk",
@@ -183,30 +175,6 @@ static void BM_AssetManagerGetBagOld(benchmark::State& state) {
}
BENCHMARK(BM_AssetManagerGetBagOld);
static void BM_AssetManagerGetBagUnverified(benchmark::State& state) {
std::unique_ptr<const ApkAssets> apk =
ApkAssets::Load(GetTestDataPath() + "/unverified/unverified.apk");
if (apk == nullptr) {
state.SkipWithError("Failed to load assets");
return;
}
AssetManager2 assets;
assets.SetApkAssets({apk.get()});
while (state.KeepRunning()) {
const ResolvedBag* bag = assets.GetBag(unverified::R::array::integerArray1);
const auto bag_end = end(bag);
for (auto iter = begin(bag); iter != bag_end; ++iter) {
uint32_t key = iter->key;
Res_value value = iter->value;
benchmark::DoNotOptimize(key);
benchmark::DoNotOptimize(value);
}
}
}
BENCHMARK(BM_AssetManagerGetBagUnverified);
static void BM_AssetManagerGetResourceLocales(benchmark::State& state) {
std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(kFrameworkPath);
if (apk == nullptr) {

28
libs/androidfw/tests/AssetManager2_test.cpp
View File

@@ -28,7 +28,6 @@
#include "data/libclient/R.h"
#include "data/styles/R.h"
#include "data/system/R.h"
#include "data/unverified/R.h"
namespace app = com::android::app;
namespace appaslib = com::android::appaslib::app;
@@ -36,7 +35,6 @@ namespace basic = com::android::basic;
namespace lib_one = com::android::lib_one;
namespace lib_two = com::android::lib_two;
namespace libclient = com::android::libclient;
namespace unverified = com::android::unverified;
namespace android {
@@ -452,30 +450,4 @@ TEST_F(AssetManager2Test, OpensFileFromSingleApkAssets) {}
TEST_F(AssetManager2Test, OpensFileFromMultipleApkAssets) {}
TEST_F(AssetManager2Test, OperateOnUnverifiedApkAssets) {
std::unique_ptr<const ApkAssets> unverified_assets =
ApkAssets::Load(GetTestDataPath() + "/unverified/unverified.apk");
ASSERT_NE(nullptr, unverified_assets);
AssetManager2 assetmanager;
assetmanager.SetApkAssets({unverified_assets.get()});
Res_value value;
ResTable_config config;
uint32_t flags;
EXPECT_EQ(kInvalidCookie,
assetmanager.GetResource(unverified::R::string::test1, false /*may_be_bag*/, 0u, &value,
&config, &flags));
EXPECT_EQ(kInvalidCookie,
assetmanager.GetResource(unverified::R::string::test2, false /*may_be_bag*/, 0u, &value,
&config, &flags));
EXPECT_NE(kInvalidCookie,
assetmanager.GetResource(unverified::R::integer::number1, false /*may_be_bag*/, 0u,
&value, &config, &flags));
EXPECT_EQ(nullptr, assetmanager.GetBag(unverified::R::style::Theme1));
EXPECT_NE(nullptr, assetmanager.GetBag(unverified::R::array::integerArray1));
}
} // namespace android

32
libs/androidfw/tests/data/unverified/R.h
View File

@@ -1,32 +0,0 @@
/*
* Copyright (C) 2016 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#ifndef TESTS_DATA_UNVERIFIED_R_H_
#define TESTS_DATA_UNVERIFIED_R_H_
#include <cstdint>
#include "tests/data/basic/R.h"
namespace com {
namespace android {
namespace unverified = basic;
} // namespace android
} // namespace com
#endif /* TESTS_DATA_UNVERIFIED_R_H_ */

BIN
libs/androidfw/tests/data/unverified/unverified.apk
View File

Binary file not shown.