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

Commit b2bbdaa4 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Fixed CursorWindow signed math for x86 builds. 

All tests for our recent CursorWindow changes have been passing for
ARM 64-bit builds, but they weren't executed against 32-bit x86
builds until after merged.

It's not actually safe to use the "off_t" type, so we need to cast
to "int32_t" when doing checks against possible-negative values,
such as in allocRow().

We also add tests that verify negative rows/columns are identified
as invalid positions, which requires that we check the resulting
pointer against both mSlotsEnd and mSlotsStart.

Bug: 169251528, 171276404, 171275409
Test: atest libandroidfw_tests:CursorWindowTest
Test: atest CtsDatabaseTestCases
Change-Id: Iea5f7546850f691e183fbb6e6d0952cd02b00d0f
parent c0e3a096

libs/androidfw/CursorWindow.cpp

+6 −6
Original line number Original line Diff line number Diff line
@@ -291,11 +291,11 @@ status_t CursorWindow::allocRow() { 
        return INVALID_OPERATION;

        return INVALID_OPERATION;

    }

    }

    size_t size = mNumColumns * kSlotSizeBytes;

    size_t size = mNumColumns * kSlotSizeBytes;

    off_t newOffset = mSlotsOffset - size;

    int32_t newOffset = mSlotsOffset - size;

    if (newOffset < mAllocOffset) {

    if (newOffset < (int32_t) mAllocOffset) {

        maybeInflate();

        maybeInflate();

        newOffset = mSlotsOffset - size;

        newOffset = mSlotsOffset - size;

        if (newOffset < mAllocOffset) {

        if (newOffset < (int32_t) mAllocOffset) {

            return NO_MEMORY;

            return NO_MEMORY;

        }

        }

    }

    }
@@ -311,7 +311,7 @@ status_t CursorWindow::freeLastRow() { 
        return INVALID_OPERATION;

        return INVALID_OPERATION;

    }

    }

    size_t size = mNumColumns * kSlotSizeBytes;

    size_t size = mNumColumns * kSlotSizeBytes;

    off_t newOffset = mSlotsOffset + size;

    size_t newOffset = mSlotsOffset + size;

    if (newOffset > mSize) {

    if (newOffset > mSize) {

        return NO_MEMORY;

        return NO_MEMORY;

    }

    }
@@ -326,7 +326,7 @@ status_t CursorWindow::alloc(size_t size, uint32_t* outOffset) { 
        return INVALID_OPERATION;

        return INVALID_OPERATION;

    }

    }

    size_t alignedSize = (size + 3) & ~3;

    size_t alignedSize = (size + 3) & ~3;

    off_t newOffset = mAllocOffset + alignedSize;

    size_t newOffset = mAllocOffset + alignedSize;

    if (newOffset > mSlotsOffset) {

    if (newOffset > mSlotsOffset) {

        maybeInflate();

        maybeInflate();

        newOffset = mAllocOffset + alignedSize;

        newOffset = mAllocOffset + alignedSize;
@@ -345,7 +345,7 @@ CursorWindow::FieldSlot* CursorWindow::getFieldSlot(uint32_t row, uint32_t colum 
    // see CursorWindow_bench.cpp for more details

    // see CursorWindow_bench.cpp for more details

    void *result = static_cast<uint8_t*>(mSlotsStart)

    void *result = static_cast<uint8_t*>(mSlotsStart)

            - (((row * mNumColumns) + column) << kSlotShift);

            - (((row * mNumColumns) + column) << kSlotShift);

    if (result < mSlotsEnd || column >= mNumColumns) {

    if (result < mSlotsEnd || result > mSlotsStart || column >= mNumColumns) {

        LOG(ERROR) << "Failed to read row " << row << ", column " << column

        LOG(ERROR) << "Failed to read row " << row << ", column " << column

                << " from a window with " << mNumRows << " rows, " << mNumColumns << " columns";

                << " from a window with " << mNumRows << " rows, " << mNumColumns << " columns";

        return nullptr;

        return nullptr;

libs/androidfw/tests/CursorWindow_test.cpp

+8 −0
Original line number Original line Diff line number Diff line
@@ -166,6 +166,14 @@ TEST(CursorWindowTest, StoreBounds) { 
    ASSERT_EQ(w->getFieldSlot(0, 3), nullptr);

    ASSERT_EQ(w->getFieldSlot(0, 3), nullptr);

    ASSERT_EQ(w->getFieldSlot(3, 0), nullptr);

    ASSERT_EQ(w->getFieldSlot(3, 0), nullptr);

    ASSERT_EQ(w->getFieldSlot(3, 3), nullptr);

    ASSERT_EQ(w->getFieldSlot(3, 3), nullptr);



    // Can't work with invalid indexes

    ASSERT_NE(w->putLong(-1, 0, 0xcafe), OK);

    ASSERT_NE(w->putLong(0, -1, 0xcafe), OK);

    ASSERT_NE(w->putLong(-1, -1, 0xcafe), OK);

    ASSERT_EQ(w->getFieldSlot(-1, 0), nullptr);

    ASSERT_EQ(w->getFieldSlot(0, -1), nullptr);

    ASSERT_EQ(w->getFieldSlot(-1, -1), nullptr);

}

}





TEST(CursorWindowTest, Inflate) {

TEST(CursorWindowTest, Inflate) {