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

Commit 3de9119c authored by Anthony Stange's avatar Anthony Stange
Browse files

Stop illegal FD from causing SensorService crash 

If an illegal FD is passed into the SensorService code that sets up
direct connections, it doesn't do any verification before using some
ashmem helpers which will crash the service if an illegal fd is passed
in. Verify any FD is valid before this step to avoid the crash.

Bug: 143896234
Test: Ran newly added code to sensorservicetest and verified system
server crash doesn't occur

Change-Id: If44481de8c7fce5e01c8e1c9d90f01241d1f1cc5
parent cca0f3c8

services/sensorservice/SensorService.cpp

+5 −0
Original line number Diff line number Diff line
@@ -1173,6 +1173,11 @@ sp<ISensorEventConnection> SensorService::createSensorDirectConnection( 
                return nullptr;

            }

            int fd = resource->data[0];

            if (!ashmem_valid(fd)) {

                ALOGE("Supplied Ashmem memory region is invalid");

                return nullptr;

            }



            int size2 = ashmem_get_size_region(fd);

            // check size consistency

            if (size2 < static_cast<int64_t>(size)) {

services/sensorservice/tests/sensorservicetest.cpp

+36 −4
Original line number Diff line number Diff line
@@ -15,19 +15,20 @@ 
 */



#include <inttypes.h>

#include <android/hardware_buffer.h>

#include <android/sensor.h>

#include <sensor/Sensor.h>

#include <sensor/SensorManager.h>

#include <sensor/SensorEventQueue.h>

#include <utils/Looper.h>

#include <vndk/hardware_buffer.h>



using namespace android;



static nsecs_t sStartTime = 0;





int receiver(__unused int fd, __unused int events, void* data)

{

int receiver(__unused int fd, __unused int events, void* data) {

    sp<SensorEventQueue> q((SensorEventQueue*)data);

    ssize_t n;

    ASensorEvent buffer[8];
@@ -59,11 +60,42 @@ int receiver(__unused int fd, __unused int events, void* data) 
    return 1;

}



void testInvalidSharedMem_NoCrash(SensorManager &mgr) {

    AHardwareBuffer *hardwareBuffer;

    char* buffer;



    constexpr size_t kEventSize = sizeof(ASensorEvent);

    constexpr size_t kNEvent = 4096; // enough to contain 1.5 * 800 * 2.2 events

    constexpr size_t kMemSize = kEventSize * kNEvent;

    AHardwareBuffer_Desc desc = {

            .width = static_cast<uint32_t>(kMemSize),

            .height = 1,

            .layers = 1,

            .format = AHARDWAREBUFFER_FORMAT_BLOB,

            .usage = AHARDWAREBUFFER_USAGE_SENSOR_DIRECT_DATA

                        | AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN,

    };



    AHardwareBuffer_allocate(&desc, &hardwareBuffer);

    AHardwareBuffer_lock(hardwareBuffer, AHARDWAREBUFFER_USAGE_CPU_READ_RARELY,

                         -1, nullptr, reinterpret_cast<void **>(&buffer));



    const native_handle_t *resourceHandle = AHardwareBuffer_getNativeHandle(hardwareBuffer);



    // Pass in AHardwareBuffer, but with the wrong DIRECT_CHANNEL_TYPE to see

    // if anything in the Sensor framework crashes

    int ret = mgr.createDirectChannel(

            kMemSize, ASENSOR_DIRECT_CHANNEL_TYPE_SHARED_MEMORY, resourceHandle);



    // Should print -22 (BAD_VALUE) and the device runtime shouldn't restart

    printf("createInvalidDirectChannel=%d\n", ret);

}



int main()

{

int main() {

    SensorManager& mgr = SensorManager::getInstanceForPackage(String16("Sensor Service Test"));



    testInvalidSharedMem_NoCrash(mgr);



    Sensor const* const* list;

    ssize_t count = mgr.getSensorList(&list);

    printf("numSensors=%d\n", int(count));