summaryrefslogtreecommitdiff
path: root/libs/androidfw/ChunkIterator.cpp
diff options
context:
space:
mode:
authorRyan Mitchell <rtmitchell@google.com>2020-08-17 08:42:48 -0700
committerRyan Mitchell <rtmitchell@google.com>2020-11-12 08:13:44 -0800
commitc75c2e092218a7d77be39c89bfba7dd2b4823ac1 (patch)
treeda96bc5ed4429763b2aec93021ec7d0340ce0b98 /libs/androidfw/ChunkIterator.cpp
parent373d5d59f274a217537029ef0c5c4d4b8ebee002 (diff)
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
Diffstat (limited to 'libs/androidfw/ChunkIterator.cpp')
-rw-r--r--libs/androidfw/ChunkIterator.cpp27
1 files changed, 19 insertions, 8 deletions
diff --git a/libs/androidfw/ChunkIterator.cpp b/libs/androidfw/ChunkIterator.cpp
index 8fc321968055..25c8aa64e492 100644
--- a/libs/androidfw/ChunkIterator.cpp
+++ b/libs/androidfw/ChunkIterator.cpp
@@ -15,6 +15,7 @@
*/
#include "androidfw/Chunk.h"
+#include "androidfw/Util.h"
#include "android-base/logging.h"
@@ -23,11 +24,11 @@ namespace android {
Chunk ChunkIterator::Next() {
CHECK(len_ != 0) << "called Next() after last chunk";
- const ResChunk_header* this_chunk = next_chunk_;
+ const incfs::map_ptr<ResChunk_header> this_chunk = next_chunk_;
+ CHECK((bool) this_chunk) << "Next() called without verifying next chunk";
// We've already checked the values of this_chunk, so safely increment.
- next_chunk_ = reinterpret_cast<const ResChunk_header*>(
- reinterpret_cast<const uint8_t*>(this_chunk) + dtohl(this_chunk->size));
+ next_chunk_ = this_chunk.offset(dtohl(this_chunk->size)).convert<ResChunk_header>();
len_ -= dtohl(this_chunk->size);
if (len_ != 0) {
@@ -36,7 +37,7 @@ Chunk ChunkIterator::Next() {
VerifyNextChunk();
}
}
- return Chunk(this_chunk);
+ return Chunk(this_chunk.verified());
}
// TODO(b/111401637) remove this and have full resource file verification
@@ -47,6 +48,13 @@ bool ChunkIterator::VerifyNextChunkNonFatal() {
last_error_was_fatal_ = false;
return false;
}
+
+ if (!next_chunk_) {
+ last_error_ = "failed to read chunk from data";
+ last_error_was_fatal_ = false;
+ return false;
+ }
+
const size_t size = dtohl(next_chunk_->size);
if (size > len_) {
last_error_ = "chunk size is bigger than given data";
@@ -58,12 +66,10 @@ bool ChunkIterator::VerifyNextChunkNonFatal() {
// Returns false if there was an error.
bool ChunkIterator::VerifyNextChunk() {
- const uintptr_t header_start = reinterpret_cast<uintptr_t>(next_chunk_);
-
// This data must be 4-byte aligned, since we directly
// access 32-bit words, which must be aligned on
// certain architectures.
- if (header_start & 0x03) {
+ if (!util::IsFourByteAligned(next_chunk_)) {
last_error_ = "header not aligned on 4-byte boundary";
return false;
}
@@ -73,6 +79,11 @@ bool ChunkIterator::VerifyNextChunk() {
return false;
}
+ if (!next_chunk_) {
+ last_error_ = "failed to read chunk from data";
+ return false;
+ }
+
const size_t header_size = dtohs(next_chunk_->headerSize);
const size_t size = dtohl(next_chunk_->size);
if (header_size < sizeof(ResChunk_header)) {
@@ -90,7 +101,7 @@ bool ChunkIterator::VerifyNextChunk() {
return false;
}
- if ((size | header_size) & 0x03) {
+ if ((size | header_size) & 0x03U) {
last_error_ = "header sizes are not aligned on 4-byte boundary";
return false;
}