From c75c2e092218a7d77be39c89bfba7dd2b4823ac1 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Mon, 17 Aug 2020 08:42:48 -0700 Subject: 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 --- libs/androidfw/ApkAssets.cpp | 82 ++++++++++++++++++++------------------------ 1 file changed, 37 insertions(+), 45 deletions(-) (limited to 'libs/androidfw/ApkAssets.cpp') diff --git a/libs/androidfw/ApkAssets.cpp b/libs/androidfw/ApkAssets.cpp index e15b42d46f53..cb56a5172a45 100755 --- a/libs/androidfw/ApkAssets.cpp +++ b/libs/androidfw/ApkAssets.cpp @@ -25,13 +25,11 @@ #include "android-base/unique_fd.h" #include "android-base/utf8.h" #include "utils/Compat.h" -#include "utils/FileMap.h" #include "ziparchive/zip_archive.h" #include "androidfw/Asset.h" #include "androidfw/Idmap.h" #include "androidfw/misc.h" -#include "androidfw/ResourceTypes.h" #include "androidfw/Util.h" namespace android { @@ -161,50 +159,46 @@ class ZipAssetsProvider : public AssetsProvider { } const int fd = ::GetFileDescriptor(zip_handle_.get()); - const off64_t fd_offset = ::GetFileDescriptorOffset(zip_handle_.get()); + const off64_t fd_offset = ::GetFileDescriptorOffset(zip_handle_.get()); + incfs::IncFsFileMap asset_map; if (entry.method == kCompressDeflated) { - std::unique_ptr map = util::make_unique(); - if (!map->create(GetPath(), fd, entry.offset + fd_offset, entry.compressed_length, - true /*readOnly*/)) { + if (!asset_map.Create(fd, entry.offset + fd_offset, entry.compressed_length, GetPath())) { LOG(ERROR) << "Failed to mmap file '" << path << "' in APK '" << friendly_name_ << "'"; return {}; } std::unique_ptr asset = - Asset::createFromCompressedMap(std::move(map), entry.uncompressed_length, mode); + Asset::createFromCompressedMap(std::move(asset_map), entry.uncompressed_length, mode); if (asset == nullptr) { LOG(ERROR) << "Failed to decompress '" << path << "' in APK '" << friendly_name_ << "'"; return {}; } return asset; - } else { - std::unique_ptr map = util::make_unique(); - if (!map->create(GetPath(), fd, entry.offset + fd_offset, entry.uncompressed_length, - true /*readOnly*/)) { - LOG(ERROR) << "Failed to mmap file '" << path << "' in APK '" << friendly_name_ << "'"; - return {}; - } + } - unique_fd ufd; - if (!GetPath()) { - // If the `path` is not set, create a new `fd` for the new Asset to own in order to create - // new file descriptors using Asset::openFileDescriptor. If the path is set, it will be used - // to create new file descriptors. - ufd = unique_fd(dup(fd)); - if (!ufd.ok()) { - LOG(ERROR) << "Unable to dup fd '" << path << "' in APK '" << friendly_name_ << "'"; - return {}; - } - } + if (!asset_map.Create(fd, entry.offset + fd_offset, entry.uncompressed_length, GetPath())) { + LOG(ERROR) << "Failed to mmap file '" << path << "' in APK '" << friendly_name_ << "'"; + return {}; + } - std::unique_ptr asset = Asset::createFromUncompressedMap(std::move(map), - std::move(ufd), mode); - if (asset == nullptr) { - LOG(ERROR) << "Failed to mmap file '" << path << "' in APK '" << friendly_name_ << "'"; + unique_fd ufd; + if (!GetPath()) { + // If the `path` is not set, create a new `fd` for the new Asset to own in order to create + // new file descriptors using Asset::openFileDescriptor. If the path is set, it will be used + // to create new file descriptors. + ufd = unique_fd(dup(fd)); + if (!ufd.ok()) { + LOG(ERROR) << "Unable to dup fd '" << path << "' in APK '" << friendly_name_ << "'"; return {}; } - return asset; } + + auto asset = Asset::createFromUncompressedMap(std::move(asset_map), mode, std::move(ufd)); + if (asset == nullptr) { + LOG(ERROR) << "Failed to mmap file '" << path << "' in APK '" << friendly_name_ << "'"; + return {}; + } + return asset; } private: @@ -446,8 +440,8 @@ std::unique_ptr ApkAssets::CreateAssetFromFd(base::unique_fd fd, } } - std::unique_ptr file_map = util::make_unique(); - if (!file_map->create(path, fd, offset, static_cast(length), true /*readOnly*/)) { + incfs::IncFsFileMap file_map; + if (!file_map.Create(fd, offset, static_cast(length), path)) { LOG(ERROR) << "Failed to mmap file '" << ((path) ? path : "anon") << "': " << SystemErrorCodeToString(errno); return {}; @@ -456,8 +450,8 @@ std::unique_ptr ApkAssets::CreateAssetFromFd(base::unique_fd fd, // If `path` is set, do not pass ownership of the `fd` to the new Asset since // Asset::openFileDescriptor can use `path` to create new file descriptors. return Asset::createFromUncompressedMap(std::move(file_map), - (path) ? base::unique_fd(-1) : std::move(fd), - Asset::AccessMode::ACCESS_RANDOM); + Asset::AccessMode::ACCESS_RANDOM, + (path) ? base::unique_fd(-1) : std::move(fd)); } std::unique_ptr ApkAssets::LoadImpl( @@ -493,15 +487,14 @@ std::unique_ptr ApkAssets::LoadImpl( loaded_apk->idmap_asset_ = std::move(idmap_asset); loaded_apk->loaded_idmap_ = std::move(idmap); - const StringPiece data( - reinterpret_cast(loaded_apk->resources_asset_->getBuffer(true /*wordAligned*/)), - loaded_apk->resources_asset_->getLength()); - if (data.data() == nullptr || data.empty()) { + const auto data = loaded_apk->resources_asset_->getIncFsBuffer(true /* aligned */); + const size_t length = loaded_apk->resources_asset_->getLength(); + if (!data || length == 0) { LOG(ERROR) << "Failed to read '" << kResourcesArsc << "' data in APK '" << path << "'."; return {}; } - loaded_apk->loaded_arsc_ = LoadedArsc::Load(data, loaded_apk->loaded_idmap_.get(), + loaded_apk->loaded_arsc_ = LoadedArsc::Load(data, length, loaded_apk->loaded_idmap_.get(), property_flags); if (!loaded_apk->loaded_arsc_) { LOG(ERROR) << "Failed to load '" << kResourcesArsc << "' in APK '" << path << "'."; @@ -525,15 +518,15 @@ std::unique_ptr ApkAssets::LoadTableImpl( new ApkAssets(std::move(assets), path, last_mod_time, property_flags)); loaded_apk->resources_asset_ = std::move(resources_asset); - const StringPiece data( - reinterpret_cast(loaded_apk->resources_asset_->getBuffer(true /*wordAligned*/)), - loaded_apk->resources_asset_->getLength()); - if (data.data() == nullptr || data.empty()) { + const auto data = loaded_apk->resources_asset_->getIncFsBuffer(true /* aligned */); + const size_t length = loaded_apk->resources_asset_->getLength(); + if (!data || length == 0) { LOG(ERROR) << "Failed to read resources table data in '" << path << "'."; return {}; } - loaded_apk->loaded_arsc_ = LoadedArsc::Load(data, nullptr, property_flags); + loaded_apk->loaded_arsc_ = LoadedArsc::Load(data, length, nullptr /* loaded_idmap */, + property_flags); if (loaded_apk->loaded_arsc_ == nullptr) { LOG(ERROR) << "Failed to read resources table in '" << path << "'."; return {}; @@ -550,7 +543,6 @@ bool ApkAssets::IsUpToDate() const { } return (!loaded_idmap_ || loaded_idmap_->IsUpToDate()) && last_mod_time_ == getFileModDate(path_.c_str()); - } } // namespace android -- cgit v1.2.3-59-g8ed1b