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/libs/androidfw/ChunkIterator.cpp b/libs/androidfw/ChunkIterator.cpp
index 8fc3219..25c8aa6 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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
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;
}