GraphicBuffer unflatten test does not detect patched Kitkat

[DjinnG]

Running VTS on KTU84P 4.4.4_r1 shows that unflatten is still unpatched even though the patch has been applied (as well as the CVE-2015-1528 patch).

Assuming I did not miss anything....

According to VTS code, the test seems to rely on the unflatten() return value, and expects patched code to return BAD_VALUE, but the test uses values that do not trigger the code that returns BAD_VALUE.
Even more so, the values given (r1[6] = 0x1000, r1[7] = 0xFF5, r2[0] = 0x20) result in (size < sizeNeeded) which returns NO_MEMORY, in both patched and unpatched code.

It almost seems as if the values of 0x1000 and 0xFF5 have been selected to test for the patch section that handles native_handle_create's failure (by triggering the CVE-2015-1528 patch), but that code also returns NO_MEMORY, which would still give the indication that the code is unpatched.

IMHO, r1[6], r1[7] and r2[0] values should be changed to (UINT_MAX / sizeof(int)) which should trigger the patched BAD_VALUE return code.
The other option would be to set r2[0] to be larger than 0x1000, but then the patched code would return NO_MEMORY, and unpached code would crash.

HTH.

[giantpune]

This issue was reported in #21 . It was supposedly fixed there. Can you show which device you are using and possibly your code with the patch applied?

[DjinnG]

Thanks for your reply.
The VTS I've used is the one published on google play store (v.12). Maybe it is not up to date?
Anyhow, the VTS test code I've commented about above was from GitHub and is the latest AFAICT.

The device is a nexus 5 and this is my patched unflatten code:

status_t GraphicBuffer::unflatten(
void const_& buffer, size_t& size, int const_& fds, size_t& count) {
if (size < 8*sizeof(int)) return NO_MEMORY;

int const* buf = static_cast<int const*>(buffer);
if (buf[0] != 'GBFR') return BAD_TYPE;

const size_t numFds  = buf[6];
const size_t numInts = buf[7];

const size_t maxNumber = UINT_MAX / sizeof(int);
if (numFds >= maxNumber || numInts >= (maxNumber - 8)) {
    width = height = stride = format = usage = 0;
    handle = NULL;
    ALOGE("unflatten: numFds or numInts is too large: %d, %d",
            numFds, numInts);
    return BAD_VALUE;
}

const size_t sizeNeeded = (8 + numInts) * sizeof(int);
if (size < sizeNeeded) return NO_MEMORY;

size_t fdCountNeeded = numFds;
if (count < fdCountNeeded) return NO_MEMORY;

if (handle) {
    // free previous handle if any
    free_handle();
}

if (numFds || numInts) {
    width  = buf[1];
    height = buf[2];
    stride = buf[3];
    format = buf[4];
    usage  = buf[5];
    native_handle* h = native_handle_create(numFds, numInts);
    if (!h) {
        width = height = stride = format = usage = 0;
        handle = NULL;
        ALOGE("unflatten: native_handle_create failed");
        return NO_MEMORY;
    }
    memcpy(h->data,          fds,     numFds*sizeof(int));
    memcpy(h->data + numFds, &buf[8], numInts*sizeof(int));
    handle = h;
} else {
    width = height = stride = format = usage = 0;
    handle = NULL;
}

mOwner = ownHandle;

if (handle != 0) {
    status_t err = mBufferMapper.registerBuffer(handle);
    if (err != NO_ERROR) {
        ALOGE("unflatten: registerBuffer failed: %s (%d)",
                strerror(-err), err);
        return err;
    }
}

buffer = reinterpret_cast<void const*>(static_cast<int const*>(buffer) + sizeNeeded);
size -= sizeNeeded;
fds += numFds;
count -= numFds;

return NO_ERROR;

}