Snap for 11651058 from 67e304ad9153b3d23e14d375cb1ccb498bb49a9d to mainline-rkpd-release
Change-Id: Ie101f3ff0532855871c211ec48c3f828c23f68dc
diff --git a/lib/libstatssocket/stats_event.c b/lib/libstatssocket/stats_event.c
index ade1b93..1c8aaf2 100644
--- a/lib/libstatssocket/stats_event.c
+++ b/lib/libstatssocket/stats_event.c
@@ -325,6 +325,9 @@
// Side-effect: modifies event->errors if field has too many annotations
static void increment_annotation_count(AStatsEvent* event) {
+ if (event->lastFieldPos >= event->bufSize) {
+ return;
+ }
uint8_t fieldType = event->buf[event->lastFieldPos] & 0x0F;
uint32_t oldAnnotationCount = (event->buf[event->lastFieldPos] & 0xF0) >> 4;
uint32_t newAnnotationCount = oldAnnotationCount + 1;
diff --git a/lib/libstatssocket/tests/stats_buffer_writer_queue_test.cpp b/lib/libstatssocket/tests/stats_buffer_writer_queue_test.cpp
index eff4cfe..008009b 100644
--- a/lib/libstatssocket/tests/stats_buffer_writer_queue_test.cpp
+++ b/lib/libstatssocket/tests/stats_buffer_writer_queue_test.cpp
@@ -29,7 +29,9 @@
using testing::Return;
using testing::StrictMock;
-constexpr static int WAIT_MS = 200;
+namespace {
+
+constexpr static int WAIT_MS = 100;
static AStatsEvent* generateTestEvent() {
AStatsEvent* event = AStatsEvent_obtain();
@@ -47,6 +49,8 @@
typedef StrictMock<BasicBufferWriterQueueMock> BufferWriterQueueMock;
+} // namespace
+
TEST(StatsBufferWriterQueueTest, TestWriteSuccess) {
AStatsEvent* event = generateTestEvent();
@@ -65,6 +69,9 @@
EXPECT_TRUE(addedToQueue);
// to yeld to the queue worker thread
std::this_thread::sleep_for(std::chrono::milliseconds(WAIT_MS));
+
+ queue.drainQueue();
+ EXPECT_EQ(queue.getQueueSize(), 0);
}
TEST(StatsBufferWriterQueueTest, TestWriteOverflow) {
@@ -90,6 +97,9 @@
EXPECT_FALSE(addedToQueue);
EXPECT_EQ(queue.getQueueSize(), BufferWriterQueueMock::kQueueMaxSizeLimit);
+
+ queue.drainQueue();
+ EXPECT_EQ(queue.getQueueSize(), 0);
}
TEST(StatsBufferWriterQueueTest, TestSleepOnOverflow) {
@@ -122,10 +132,13 @@
EXPECT_TRUE(addedToQueue);
}
AStatsEvent_release(event);
-
// to yeld to the queue worker thread
std::this_thread::sleep_for(std::chrono::milliseconds(WAIT_MS));
+ // to eliminate extra commands handling on the worker thread
+ queue.drainQueue();
+ EXPECT_EQ(queue.getQueueSize(), 0);
+
EXPECT_GE(attemptsTs.size(), 2);
for (int i = 0; i < attemptsTs.size() - 1; i++) {
EXPECT_GE(attemptsTs[i + 1] - attemptsTs[i],
diff --git a/lib/libstatssocket/tests/stats_event_test.cpp b/lib/libstatssocket/tests/stats_event_test.cpp
index 93a99f1..dea81c2 100644
--- a/lib/libstatssocket/tests/stats_event_test.cpp
+++ b/lib/libstatssocket/tests/stats_event_test.cpp
@@ -536,6 +536,50 @@
AStatsEvent_release(event);
}
+TEST(StatsEventTest, TestHeapBufferOverflowError) {
+ const std::string testString(4039, 'A');
+ const std::string testString2(47135, 'B');
+
+ AStatsEvent* event = AStatsEvent_obtain();
+ AStatsEvent_setAtomId(event, 100);
+
+ AStatsEvent_writeString(event, testString.c_str());
+ size_t bufferSize = 0;
+ AStatsEvent_getBuffer(event, &bufferSize);
+ EXPECT_EQ(bufferSize, 4060);
+ uint32_t errors = AStatsEvent_getErrors(event);
+ EXPECT_EQ(errors, 0);
+
+ // expand the buffer and fill with data up to the very last byte
+ AStatsEvent_writeString(event, testString2.c_str());
+ bufferSize = 0;
+ AStatsEvent_getBuffer(event, &bufferSize);
+ EXPECT_EQ(bufferSize, 50 * 1024);
+
+ errors = AStatsEvent_getErrors(event);
+ EXPECT_EQ(errors, 0);
+
+ // this write is no-op due to buffer reached its max capacity
+ // should set the overflow flag
+ AStatsEvent_writeString(event, testString2.c_str());
+ bufferSize = 0;
+ AStatsEvent_getBuffer(event, &bufferSize);
+ EXPECT_EQ(bufferSize, 50 * 1024);
+
+ errors = AStatsEvent_getErrors(event);
+ EXPECT_EQ(errors & ERROR_OVERFLOW, ERROR_OVERFLOW);
+
+ // here should be crash
+ AStatsEvent_addBoolAnnotation(event, 1, false);
+
+ AStatsEvent_write(event);
+
+ errors = AStatsEvent_getErrors(event);
+ EXPECT_EQ(errors & ERROR_OVERFLOW, ERROR_OVERFLOW);
+
+ AStatsEvent_release(event);
+}
+
TEST(StatsEventTest, TestPullOverflowError) {
const uint32_t atomId = 10100;
const vector<uint8_t> bytes(430 /* number of elements */, 1 /* value of each element */);
diff --git a/statsd/src/logd/LogEvent.cpp b/statsd/src/logd/LogEvent.cpp
index 94fe5cf..affb080 100644
--- a/statsd/src/logd/LogEvent.cpp
+++ b/statsd/src/logd/LogEvent.cpp
@@ -266,7 +266,7 @@
}
// Assumes that mValues is not empty
-bool LogEvent::checkPreviousValueType(Type expected) {
+bool LogEvent::checkPreviousValueType(Type expected) const {
return mValues[mValues.size() - 1].mValue.getType() == expected;
}
@@ -283,7 +283,7 @@
}
// Allowed types: INT, repeated INT
- if (numElements > mValues.size() || !checkPreviousValueType(INT) ||
+ if (mValues.empty() || numElements > mValues.size() || !checkPreviousValueType(INT) ||
annotationType != BOOL_TYPE) {
VLOG("Atom ID %d error while parseIsUidAnnotation()", mTagId);
mValid = false;
diff --git a/statsd/src/logd/LogEvent.h b/statsd/src/logd/LogEvent.h
index c34d34e..db2bc4e 100644
--- a/statsd/src/logd/LogEvent.h
+++ b/statsd/src/logd/LogEvent.h
@@ -296,7 +296,7 @@
void parseStateNestedAnnotation(uint8_t annotationType, std::optional<uint8_t> numElements);
void parseRestrictionCategoryAnnotation(uint8_t annotationType);
void parseFieldRestrictionAnnotation(uint8_t annotationType);
- bool checkPreviousValueType(Type expected);
+ bool checkPreviousValueType(Type expected) const;
bool getRestrictedMetricsFlag();
/**
diff --git a/tests/src/android/cts/statsd/atom/ProcStateTestCase.java b/tests/src/android/cts/statsd/atom/ProcStateTestCase.java
index d5038f9..09c508b 100644
--- a/tests/src/android/cts/statsd/atom/ProcStateTestCase.java
+++ b/tests/src/android/cts/statsd/atom/ProcStateTestCase.java
@@ -80,10 +80,10 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
ConfigUtils.removeConfig(getDevice());
ReportUtils.clearReports(getDevice());
DeviceUtils.uninstallTestApp(getDevice(), MetricsUtils.DEVICE_SIDE_TEST_PACKAGE);
+ super.tearDown();
}
@Override
diff --git a/tests/src/android/cts/statsd/metadata/MetadataTestCase.java b/tests/src/android/cts/statsd/metadata/MetadataTestCase.java
index 0e957b5..1f0e5a1 100644
--- a/tests/src/android/cts/statsd/metadata/MetadataTestCase.java
+++ b/tests/src/android/cts/statsd/metadata/MetadataTestCase.java
@@ -75,10 +75,10 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
ConfigUtils.removeConfig(getDevice());
ReportUtils.clearReports(getDevice());
DeviceUtils.uninstallTestApp(getDevice(), MetricsUtils.DEVICE_SIDE_TEST_PACKAGE);
+ super.tearDown();
}
@Override
diff --git a/tests/src/android/cts/statsd/metric/CountMetricsTests.java b/tests/src/android/cts/statsd/metric/CountMetricsTests.java
index 0024953..8e9b5a3 100644
--- a/tests/src/android/cts/statsd/metric/CountMetricsTests.java
+++ b/tests/src/android/cts/statsd/metric/CountMetricsTests.java
@@ -64,10 +64,10 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
ConfigUtils.removeConfig(getDevice());
ReportUtils.clearReports(getDevice());
DeviceUtils.uninstallTestApp(getDevice(), MetricsUtils.DEVICE_SIDE_TEST_PACKAGE);
+ super.tearDown();
}
@Override
diff --git a/tests/src/android/cts/statsd/metric/DurationMetricsTests.java b/tests/src/android/cts/statsd/metric/DurationMetricsTests.java
index a40dbf1..89c9bbc 100644
--- a/tests/src/android/cts/statsd/metric/DurationMetricsTests.java
+++ b/tests/src/android/cts/statsd/metric/DurationMetricsTests.java
@@ -56,9 +56,9 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
ConfigUtils.removeConfig(getDevice());
ReportUtils.clearReports(getDevice());
+ super.tearDown();
}
public void testDurationMetric() throws Exception {
diff --git a/tests/src/android/cts/statsd/metric/GaugeMetricsTests.java b/tests/src/android/cts/statsd/metric/GaugeMetricsTests.java
index db0a39f..a037a69 100644
--- a/tests/src/android/cts/statsd/metric/GaugeMetricsTests.java
+++ b/tests/src/android/cts/statsd/metric/GaugeMetricsTests.java
@@ -67,9 +67,9 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
ConfigUtils.removeConfig(getDevice());
ReportUtils.clearReports(getDevice());
+ super.tearDown();
}
public void testGaugeMetric() throws Exception {
diff --git a/tests/src/android/cts/statsd/metric/MetricActivationTests.java b/tests/src/android/cts/statsd/metric/MetricActivationTests.java
index 7cee7eea..5c6f2fb 100644
--- a/tests/src/android/cts/statsd/metric/MetricActivationTests.java
+++ b/tests/src/android/cts/statsd/metric/MetricActivationTests.java
@@ -74,9 +74,9 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
ConfigUtils.removeConfig(getDevice());
ReportUtils.clearReports(getDevice());
+ super.tearDown();
}
private StatsdConfig.Builder createConfig(final int act1TtlSecs, final int act2TtlSecs) {
diff --git a/tests/src/android/cts/statsd/metric/ValueMetricsTests.java b/tests/src/android/cts/statsd/metric/ValueMetricsTests.java
index 6cd6873..4ac1d5e 100644
--- a/tests/src/android/cts/statsd/metric/ValueMetricsTests.java
+++ b/tests/src/android/cts/statsd/metric/ValueMetricsTests.java
@@ -61,9 +61,9 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
ConfigUtils.removeConfig(getDevice());
ReportUtils.clearReports(getDevice());
+ super.tearDown();
}
diff --git a/tests/src/android/cts/statsd/restricted/ReadRestrictedStatsPermissionTest.java b/tests/src/android/cts/statsd/restricted/ReadRestrictedStatsPermissionTest.java
index 1b646f5..8e563b9 100644
--- a/tests/src/android/cts/statsd/restricted/ReadRestrictedStatsPermissionTest.java
+++ b/tests/src/android/cts/statsd/restricted/ReadRestrictedStatsPermissionTest.java
@@ -28,8 +28,8 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
DeviceUtils.uninstallTestApp(getDevice(), MetricsUtils.DEVICE_SIDE_TEST_PACKAGE);
+ super.tearDown();
}
@Override
diff --git a/tests/src/android/cts/statsd/uidmap/UidMapTests.java b/tests/src/android/cts/statsd/uidmap/UidMapTests.java
index 57745f9..595a987 100644
--- a/tests/src/android/cts/statsd/uidmap/UidMapTests.java
+++ b/tests/src/android/cts/statsd/uidmap/UidMapTests.java
@@ -58,10 +58,10 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
ConfigUtils.removeConfig(getDevice());
ReportUtils.clearReports(getDevice());
DeviceUtils.uninstallTestApp(getDevice(), MetricsUtils.DEVICE_SIDE_TEST_PACKAGE);
+ super.tearDown();
}
@Override
diff --git a/tests/src/android/cts/statsd/validation/DirectoryValidationTest.java b/tests/src/android/cts/statsd/validation/DirectoryValidationTest.java
index 4a41e6e..4fa5e2a 100644
--- a/tests/src/android/cts/statsd/validation/DirectoryValidationTest.java
+++ b/tests/src/android/cts/statsd/validation/DirectoryValidationTest.java
@@ -28,8 +28,8 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
DeviceUtils.uninstallTestApp(getDevice(), MetricsUtils.DEVICE_SIDE_TEST_PACKAGE);
+ super.tearDown();
}
@Override
diff --git a/tests/src/android/cts/statsd/validation/StatsFrameworkInitializerTest.java b/tests/src/android/cts/statsd/validation/StatsFrameworkInitializerTest.java
index 10dcaab..2a67748 100644
--- a/tests/src/android/cts/statsd/validation/StatsFrameworkInitializerTest.java
+++ b/tests/src/android/cts/statsd/validation/StatsFrameworkInitializerTest.java
@@ -41,8 +41,8 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
DeviceUtils.uninstallTestApp(getDevice(), MetricsUtils.DEVICE_SIDE_TEST_PACKAGE);
+ super.tearDown();
}
@Override
diff --git a/tests/src/android/cts/statsd/validation/ValidationTests.java b/tests/src/android/cts/statsd/validation/ValidationTests.java
index dacb008..a3f0109 100644
--- a/tests/src/android/cts/statsd/validation/ValidationTests.java
+++ b/tests/src/android/cts/statsd/validation/ValidationTests.java
@@ -88,13 +88,13 @@
@Override
protected void tearDown() throws Exception {
- super.setUp();
ConfigUtils.removeConfig(getDevice());
ReportUtils.clearReports(getDevice());
DeviceUtils.uninstallTestApp(getDevice(), MetricsUtils.DEVICE_SIDE_TEST_PACKAGE);
DeviceUtils.resetBatteryStatus(getDevice());
DeviceUtils.turnScreenOn(getDevice());
DeviceUtils.turnBatteryStatsAutoResetOn(getDevice());
+ super.tearDown();
}
@Override
