Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9b9584b1 authored by Muhammad Qureshi's avatar Muhammad Qureshi
Browse files

Increase size limit for pulled AStatsEvent

Increase AStatsEvent max byte size to 50 KB for pulled events.
All AStatsEvent instances are treated as pulled events unless
AStatsEvent_write() is called.

- Set the default max size to 50 KB
- The starting buffer size is still ~4 KB for pushed and pulled events.
- If a write would exceed the buffer bounds, double buffer size until
write fits or 50 KB limit is exceeded in which case the overflow bit is
set to true.
- If AStatsEvent_write() is called, max size is set to ~4 KB. And if
the current payload exceeds this limit, set overflow bit to true.

- Fix error mask checking in stats_event_test.
- Set ERROR_NO_ATOM_ID when atom id is missing.
- Make sure tests don't hit ERROR_TOO_MANY_FIELDS when testing buffer
overflow.
- Rename event->size to event->numBytesWritten

Fixes: 158214941
Test: libstatssocket_test
Change-Id: Ia26aeb775f7e4f2ffe87707bab6d0119e21da10e
parent 84db2e40
Loading
Loading
Loading
Loading
+0 −1
Original line number Original line Diff line number Diff line
@@ -224,7 +224,6 @@ void StatsEventCompat::addInt32Annotation(uint8_t annotationId, int32_t value) {


int StatsEventCompat::writeToSocket() {
int StatsEventCompat::writeToSocket() {
    if (useRSchema()) {
    if (useRSchema()) {
        mAStatsEventApi.build(mEventR);
        return mAStatsEventApi.write(mEventR);
        return mAStatsEventApi.write(mEventR);
    }
    }


+3 −3
Original line number Original line Diff line number Diff line
@@ -35,7 +35,6 @@
 *      AStatsEvent_addInt32Annotation(event, 2, 128);
 *      AStatsEvent_addInt32Annotation(event, 2, 128);
 *      AStatsEvent_writeFloat(event, 2.0);
 *      AStatsEvent_writeFloat(event, 2.0);
 *
 *
 *      AStatsEvent_build(event);
 *      AStatsEvent_write(event);
 *      AStatsEvent_write(event);
 *      AStatsEvent_release(event);
 *      AStatsEvent_release(event);
 *
 *
@@ -61,10 +60,11 @@ typedef struct AStatsEvent AStatsEvent;
AStatsEvent* AStatsEvent_obtain();
AStatsEvent* AStatsEvent_obtain();


/**
/**
 * Builds and finalizes the StatsEvent.
 * Builds and finalizes the AStatsEvent for a pulled event.
 * This should only be called for pulled AStatsEvents.
 *
 *
 * After this function, the StatsEvent must not be modified in any way other than calling release or
 * After this function, the StatsEvent must not be modified in any way other than calling release or
 * write. Build must be always be called before AStatsEvent_write.
 * write.
 *
 *
 * Build can be called multiple times without error.
 * Build can be called multiple times without error.
 * If the event has been built before, this function is a no-op.
 * If the event has been built before, this function is a no-op.
+54 −27
Original line number Original line Diff line number Diff line
@@ -23,7 +23,9 @@
#define LOGGER_ENTRY_MAX_PAYLOAD 4068
#define LOGGER_ENTRY_MAX_PAYLOAD 4068
// Max payload size is 4 bytes less as 4 bytes are reserved for stats_eventTag.
// Max payload size is 4 bytes less as 4 bytes are reserved for stats_eventTag.
// See android_util_Stats_Log.cpp
// See android_util_Stats_Log.cpp
#define MAX_EVENT_PAYLOAD (LOGGER_ENTRY_MAX_PAYLOAD - 4)
#define MAX_PUSH_EVENT_PAYLOAD (LOGGER_ENTRY_MAX_PAYLOAD - 4)

#define MAX_PULL_EVENT_PAYLOAD (50 * 1024)  // 50 KB


/* POSITIONS */
/* POSITIONS */
#define POS_NUM_ELEMENTS 1
#define POS_NUM_ELEMENTS 1
@@ -70,12 +72,13 @@ struct AStatsEvent {
    // metadata field (e.g. timestamp) or an atom field.
    // metadata field (e.g. timestamp) or an atom field.
    size_t lastFieldPos;
    size_t lastFieldPos;
    // Number of valid bytes within the buffer.
    // Number of valid bytes within the buffer.
    size_t size;
    size_t numBytesWritten;
    uint32_t numElements;
    uint32_t numElements;
    uint32_t atomId;
    uint32_t atomId;
    uint32_t errors;
    uint32_t errors;
    bool truncate;
    bool truncate;
    bool built;
    bool built;
    size_t bufSize;
};
};


static int64_t get_elapsed_realtime_ns() {
static int64_t get_elapsed_realtime_ns() {
@@ -87,14 +90,15 @@ static int64_t get_elapsed_realtime_ns() {


AStatsEvent* AStatsEvent_obtain() {
AStatsEvent* AStatsEvent_obtain() {
    AStatsEvent* event = malloc(sizeof(AStatsEvent));
    AStatsEvent* event = malloc(sizeof(AStatsEvent));
    event->buf = (uint8_t*)calloc(MAX_EVENT_PAYLOAD, 1);
    event->lastFieldPos = 0;
    event->lastFieldPos = 0;
    event->size = 2;  // reserve first two bytes for outer event type and number of elements
    event->numBytesWritten = 2;  // reserve first 2 bytes for root event type and number of elements
    event->numElements = 0;
    event->numElements = 0;
    event->atomId = 0;
    event->atomId = 0;
    event->errors = 0;
    event->errors = 0;
    event->truncate = true;  // truncate for both pulled and pushed atoms
    event->truncate = true;  // truncate for both pulled and pushed atoms
    event->built = false;
    event->built = false;
    event->bufSize = MAX_PUSH_EVENT_PAYLOAD;
    event->buf = (uint8_t*)calloc(event->bufSize, 1);


    event->buf[0] = OBJECT_TYPE;
    event->buf[0] = OBJECT_TYPE;
    AStatsEvent_writeInt64(event, get_elapsed_realtime_ns());  // write the timestamp
    AStatsEvent_writeInt64(event, get_elapsed_realtime_ns());  // write the timestamp
@@ -128,19 +132,33 @@ void AStatsEvent_overwriteTimestamp(AStatsEvent* event, uint64_t timestampNs) {


// Side-effect: modifies event->errors if the buffer would overflow
// Side-effect: modifies event->errors if the buffer would overflow
static bool overflows(AStatsEvent* event, size_t size) {
static bool overflows(AStatsEvent* event, size_t size) {
    if (event->size + size > MAX_EVENT_PAYLOAD) {
    const size_t totalBytesNeeded = event->numBytesWritten + size;
    if (totalBytesNeeded > MAX_PULL_EVENT_PAYLOAD) {
        event->errors |= ERROR_OVERFLOW;
        event->errors |= ERROR_OVERFLOW;
        return true;
        return true;
    }
    }

    // Expand buffer if needed.
    if (event->bufSize < MAX_PULL_EVENT_PAYLOAD && totalBytesNeeded > event->bufSize) {
        do {
            event->bufSize *= 2;
        } while (event->bufSize <= totalBytesNeeded);

        if (event->bufSize > MAX_PULL_EVENT_PAYLOAD) {
            event->bufSize = MAX_PULL_EVENT_PAYLOAD;
        }

        event->buf = (uint8_t*)realloc(event->buf, event->bufSize);
    }
    return false;
    return false;
}
}


// Side-effect: all append functions increment event->size if there is
// Side-effect: all append functions increment event->numBytesWritten if there is
// sufficient space within the buffer to place the value
// sufficient space within the buffer to place the value
static void append_byte(AStatsEvent* event, uint8_t value) {
static void append_byte(AStatsEvent* event, uint8_t value) {
    if (!overflows(event, sizeof(value))) {
    if (!overflows(event, sizeof(value))) {
        event->buf[event->size] = value;
        event->buf[event->numBytesWritten] = value;
        event->size += sizeof(value);
        event->numBytesWritten += sizeof(value);
    }
    }
}
}


@@ -150,36 +168,36 @@ static void append_bool(AStatsEvent* event, bool value) {


static void append_int32(AStatsEvent* event, int32_t value) {
static void append_int32(AStatsEvent* event, int32_t value) {
    if (!overflows(event, sizeof(value))) {
    if (!overflows(event, sizeof(value))) {
        memcpy(&event->buf[event->size], &value, sizeof(value));
        memcpy(&event->buf[event->numBytesWritten], &value, sizeof(value));
        event->size += sizeof(value);
        event->numBytesWritten += sizeof(value);
    }
    }
}
}


static void append_int64(AStatsEvent* event, int64_t value) {
static void append_int64(AStatsEvent* event, int64_t value) {
    if (!overflows(event, sizeof(value))) {
    if (!overflows(event, sizeof(value))) {
        memcpy(&event->buf[event->size], &value, sizeof(value));
        memcpy(&event->buf[event->numBytesWritten], &value, sizeof(value));
        event->size += sizeof(value);
        event->numBytesWritten += sizeof(value);
    }
    }
}
}


static void append_float(AStatsEvent* event, float value) {
static void append_float(AStatsEvent* event, float value) {
    if (!overflows(event, sizeof(value))) {
    if (!overflows(event, sizeof(value))) {
        memcpy(&event->buf[event->size], &value, sizeof(value));
        memcpy(&event->buf[event->numBytesWritten], &value, sizeof(value));
        event->size += sizeof(float);
        event->numBytesWritten += sizeof(float);
    }
    }
}
}


static void append_byte_array(AStatsEvent* event, const uint8_t* buf, size_t size) {
static void append_byte_array(AStatsEvent* event, const uint8_t* buf, size_t size) {
    if (!overflows(event, size)) {
    if (!overflows(event, size)) {
        memcpy(&event->buf[event->size], buf, size);
        memcpy(&event->buf[event->numBytesWritten], buf, size);
        event->size += size;
        event->numBytesWritten += size;
    }
    }
}
}


// Side-effect: modifies event->errors if buf is not properly null-terminated
// Side-effect: modifies event->errors if buf is not properly null-terminated
static void append_string(AStatsEvent* event, const char* buf) {
static void append_string(AStatsEvent* event, const char* buf) {
    size_t size = strnlen(buf, MAX_EVENT_PAYLOAD);
    size_t size = strnlen(buf, MAX_PULL_EVENT_PAYLOAD);
    if (size == MAX_EVENT_PAYLOAD) {
    if (size == MAX_PULL_EVENT_PAYLOAD) {
        event->errors |= ERROR_STRING_NOT_NULL_TERMINATED;
        event->errors |= ERROR_STRING_NOT_NULL_TERMINATED;
        return;
        return;
    }
    }
@@ -189,7 +207,7 @@ static void append_string(AStatsEvent* event, const char* buf) {
}
}


static void start_field(AStatsEvent* event, uint8_t typeId) {
static void start_field(AStatsEvent* event, uint8_t typeId) {
    event->lastFieldPos = event->size;
    event->lastFieldPos = event->numBytesWritten;
    append_byte(event, typeId);
    append_byte(event, typeId);
    event->numElements++;
    event->numElements++;
}
}
@@ -292,7 +310,7 @@ uint32_t AStatsEvent_getAtomId(AStatsEvent* event) {
}
}


uint8_t* AStatsEvent_getBuffer(AStatsEvent* event, size_t* size) {
uint8_t* AStatsEvent_getBuffer(AStatsEvent* event, size_t* size) {
    if (size) *size = event->size;
    if (size) *size = event->numBytesWritten;
    return event->buf;
    return event->buf;
}
}


@@ -304,10 +322,10 @@ void AStatsEvent_truncateBuffer(AStatsEvent* event, bool truncate) {
    event->truncate = truncate;
    event->truncate = truncate;
}
}


void AStatsEvent_build(AStatsEvent* event) {
static void build_internal(AStatsEvent* event, const bool push) {
    if (event->built) return;

    if (event->numElements > MAX_BYTE_VALUE) event->errors |= ERROR_TOO_MANY_FIELDS;
    if (event->numElements > MAX_BYTE_VALUE) event->errors |= ERROR_TOO_MANY_FIELDS;
    if (0 == event->atomId) event->errors |= ERROR_NO_ATOM_ID;
    if (push && event->numBytesWritten > MAX_PUSH_EVENT_PAYLOAD) event->errors |= ERROR_OVERFLOW;


    // If there are errors, rewrite buffer.
    // If there are errors, rewrite buffer.
    if (event->errors) {
    if (event->errors) {
@@ -317,7 +335,7 @@ void AStatsEvent_build(AStatsEvent* event) {
        // Reset number of atom-level annotations to 0.
        // Reset number of atom-level annotations to 0.
        event->buf[POS_ATOM_ID] = INT32_TYPE;
        event->buf[POS_ATOM_ID] = INT32_TYPE;
        // Now, write errors to the buffer immediately after the atom id.
        // Now, write errors to the buffer immediately after the atom id.
        event->size = POS_ATOM_ID + sizeof(uint8_t) + sizeof(uint32_t);
        event->numBytesWritten = POS_ATOM_ID + sizeof(uint8_t) + sizeof(uint32_t);
        start_field(event, ERROR_TYPE);
        start_field(event, ERROR_TYPE);
        append_int32(event, event->errors);
        append_int32(event, event->errors);
    }
    }
@@ -326,12 +344,21 @@ void AStatsEvent_build(AStatsEvent* event) {


    // Truncate the buffer to the appropriate length in order to limit our
    // Truncate the buffer to the appropriate length in order to limit our
    // memory usage.
    // memory usage.
    if (event->truncate) event->buf = (uint8_t*)realloc(event->buf, event->size);
    if (event->truncate) {
        event->buf = (uint8_t*)realloc(event->buf, event->numBytesWritten);
        event->bufSize = event->numBytesWritten;
    }
}

void AStatsEvent_build(AStatsEvent* event) {
    if (event->built) return;

    build_internal(event, false /* push */);


    event->built = true;
    event->built = true;
}
}


int AStatsEvent_write(AStatsEvent* event) {
int AStatsEvent_write(AStatsEvent* event) {
    AStatsEvent_build(event);
    build_internal(event, true /* push */);
    return write_buffer_to_statsd(event->buf, event->size, event->atomId);
    return write_buffer_to_statsd(event->buf, event->numBytesWritten, event->atomId);
}
}
+71 −7
Original line number Original line Diff line number Diff line
@@ -343,24 +343,88 @@ TEST(StatsEventTest, TestNoAtomIdError) {
    AStatsEvent_build(event);
    AStatsEvent_build(event);


    uint32_t errors = AStatsEvent_getErrors(event);
    uint32_t errors = AStatsEvent_getErrors(event);
    EXPECT_NE(errors | ERROR_NO_ATOM_ID, 0);
    EXPECT_EQ(errors & ERROR_NO_ATOM_ID, ERROR_NO_ATOM_ID);


    AStatsEvent_release(event);
    AStatsEvent_release(event);
}
}


TEST(StatsEventTest, TestOverflowError) {
TEST(StatsEventTest, TestPushOverflowError) {
    const char* str = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
    const int writeCount = 120;  // Number of times to write str in the event.

    AStatsEvent* event = AStatsEvent_obtain();
    AStatsEvent* event = AStatsEvent_obtain();
    AStatsEvent_setAtomId(event, 100);
    AStatsEvent_setAtomId(event, 100);
    // Add 1000 int32s to the event. Each int32 takes 5 bytes so this will

    // Add str to the event 120 times. Each str takes >35 bytes so this will
    // overflow the 4068 byte buffer.
    // overflow the 4068 byte buffer.
    for (int i = 0; i < 1000; i++) {
    // We want to keep writeCount less than 127 to avoid hitting
        AStatsEvent_writeInt32(event, 0);
    // ERROR_TOO_MANY_FIELDS.
    for (int i = 0; i < writeCount; i++) {
        AStatsEvent_writeString(event, str);
    }
    AStatsEvent_write(event);

    uint32_t 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 */);
    const int writeCount = 120;  // Number of times to write bytes in the event.

    AStatsEvent* event = AStatsEvent_obtain();
    AStatsEvent_setAtomId(event, atomId);

    // Add bytes to the event 120 times. Size of bytes is 430 so this will
    // overflow the 50 KB pulled event buffer.
    // We want to keep writeCount less than 127 to avoid hitting
    // ERROR_TOO_MANY_FIELDS.
    for (int i = 0; i < writeCount; i++) {
        AStatsEvent_writeByteArray(event, bytes.data(), bytes.size());
    }
    }
    AStatsEvent_build(event);
    AStatsEvent_build(event);


    uint32_t errors = AStatsEvent_getErrors(event);
    uint32_t errors = AStatsEvent_getErrors(event);
    EXPECT_NE(errors | ERROR_OVERFLOW, 0);
    EXPECT_EQ(errors & ERROR_OVERFLOW, ERROR_OVERFLOW);

    AStatsEvent_release(event);
}

TEST(StatsEventTest, TestLargePull) {
    const uint32_t atomId = 100;
    const string str = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
    const int writeCount = 120;  // Number of times to write str in the event.
    const int64_t startTime = android::elapsedRealtimeNano();


    AStatsEvent* event = AStatsEvent_obtain();
    AStatsEvent_setAtomId(event, atomId);

    // Add str to the event 120 times.
    // We want to keep writeCount less than 127 to avoid hitting
    // ERROR_TOO_MANY_FIELDS.
    for (int i = 0; i < writeCount; i++) {
        AStatsEvent_writeString(event, str.c_str());
    }
    AStatsEvent_build(event);
    int64_t endTime = android::elapsedRealtimeNano();

    size_t bufferSize;
    uint8_t* buffer = AStatsEvent_getBuffer(event, &bufferSize);
    uint8_t* bufferEnd = buffer + bufferSize;

    checkMetadata(&buffer, writeCount, startTime, endTime, atomId);

    // Check all instances of str have been written.
    for (int i = 0; i < writeCount; i++) {
        checkTypeHeader(&buffer, STRING_TYPE);
        checkString(&buffer, str);
    }

    EXPECT_EQ(buffer, bufferEnd);  // Ensure that we have read the entire buffer.
    EXPECT_EQ(AStatsEvent_getErrors(event), 0);
    AStatsEvent_release(event);
    AStatsEvent_release(event);
}
}


@@ -372,7 +436,7 @@ TEST(StatsEventTest, TestAtomIdInvalidPositionError) {
    AStatsEvent_build(event);
    AStatsEvent_build(event);


    uint32_t errors = AStatsEvent_getErrors(event);
    uint32_t errors = AStatsEvent_getErrors(event);
    EXPECT_NE(errors | ERROR_ATOM_ID_INVALID_POSITION, 0);
    EXPECT_EQ(errors & ERROR_ATOM_ID_INVALID_POSITION, ERROR_ATOM_ID_INVALID_POSITION);


    AStatsEvent_release(event);
    AStatsEvent_release(event);
}
}
+1 −4
Original line number Original line Diff line number Diff line
@@ -20,12 +20,9 @@
#include "stats_socket.h"
#include "stats_socket.h"


TEST(StatsWriterTest, TestSocketClose) {
TEST(StatsWriterTest, TestSocketClose) {
    EXPECT_TRUE(stats_log_is_closed());

    AStatsEvent* event = AStatsEvent_obtain();
    AStatsEvent* event = AStatsEvent_obtain();
    AStatsEvent_setAtomId(event, 100);
    AStatsEvent_setAtomId(event, 100);
    AStatsEvent_writeInt32(event, 5);
    AStatsEvent_writeInt32(event, 5);
    AStatsEvent_build(event);
    int successResult = AStatsEvent_write(event);
    int successResult = AStatsEvent_write(event);
    AStatsEvent_release(event);
    AStatsEvent_release(event);