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

Commit 0a4cebbc authored by Brian Duddie's avatar Brian Duddie
Browse files

Use fdsan in direct channel registration 

To help catch future bugs related to misuse of file descriptors.

Fixes: 244214188
Test: run test-sensorservice with & without fix for bug 234456046,
      confirm reliable crash without the fix + no crash with it
Test: atest CtsSensorTestCases
Change-Id: I7aca5830e02e6bde988e89d54e7008500d0db26f
parent 997aaf22

libs/sensor/ISensorServer.cpp

+4 −3
Original line number Diff line number Diff line
@@ -22,12 +22,12 @@ 
#include <cutils/native_handle.h>

#include <utils/Errors.h>

#include <utils/RefBase.h>

#include <utils/Vector.h>

#include <utils/Timers.h>

#include <utils/Vector.h>



#include <binder/Parcel.h>

#include <binder/IInterface.h>

#include <binder/IResultReceiver.h>

#include <binder/Parcel.h>



#include <sensor/Sensor.h>

#include <sensor/ISensorEventConnection.h>
@@ -205,9 +205,10 @@ status_t BnSensorServer::onTransact( 
            if (resource == nullptr) {

                return BAD_VALUE;

            }

            native_handle_set_fdsan_tag(resource);

            sp<ISensorEventConnection> ch =

                    createSensorDirectConnection(opPackageName, size, type, format, resource);

            native_handle_close(resource);

            native_handle_close_with_tag(resource);

            native_handle_delete(resource);

            reply->writeStrongBinder(IInterface::asBinder(ch));

            return NO_ERROR;

services/sensorservice/SensorDevice.cpp

+0 −1
Original line number Diff line number Diff line
@@ -39,7 +39,6 @@ 
#include <thread>



using namespace android::hardware::sensors;

using android::hardware::Return;

using android::util::ProtoOutputStream;



namespace android {

services/sensorservice/SensorDirectConnection.cpp

+2 −2
Original line number Diff line number Diff line
@@ -14,11 +14,11 @@ 
 * limitations under the License.

 */



#include "SensorDevice.h"

#include "SensorDirectConnection.h"

#include <android/util/ProtoOutputStream.h>

#include <frameworks/base/core/proto/android/service/sensor_service.proto.h>

#include <hardware/sensors.h>

#include "SensorDevice.h"



#define UNUSED(x) (void)(x)
@@ -51,7 +51,7 @@ void SensorService::SensorDirectConnection::destroy() { 
    stopAll();

    mService->cleanupConnection(this);

    if (mMem.handle != nullptr) {

        native_handle_close(mMem.handle);

        native_handle_close_with_tag(mMem.handle);

        native_handle_delete(const_cast<struct native_handle*>(mMem.handle));

    }

    mDestroyed = true;

services/sensorservice/SensorService.cpp

+3 −2
Original line number Diff line number Diff line
@@ -16,7 +16,6 @@ 
#include <android-base/strings.h>

#include <android/content/pm/IPackageManagerNative.h>

#include <android/util/ProtoOutputStream.h>

#include <frameworks/base/core/proto/android/service/sensor_service.proto.h>

#include <binder/ActivityManager.h>

#include <binder/BinderService.h>

#include <binder/IServiceManager.h>
@@ -25,6 +24,7 @@ 
#include <cutils/ashmem.h>

#include <cutils/misc.h>

#include <cutils/properties.h>

#include <frameworks/base/core/proto/android/service/sensor_service.proto.h>

#include <hardware/sensors.h>

#include <hardware_legacy/power.h>

#include <log/log.h>
@@ -1475,6 +1475,7 @@ sp<ISensorEventConnection> SensorService::createSensorDirectConnection( 
    if (!clone) {

        return nullptr;

    }

    native_handle_set_fdsan_tag(clone);



    sp<SensorDirectConnection> conn;

    SensorDevice& dev(SensorDevice::getInstance());
@@ -1488,7 +1489,7 @@ sp<ISensorEventConnection> SensorService::createSensorDirectConnection( 
    }



    if (conn == nullptr) {

        native_handle_close(clone);

        native_handle_close_with_tag(clone);

        native_handle_delete(clone);

    } else {

        // add to list of direct connections

services/sensorservice/tests/sensorservicetest.cpp

+11 −0
Original line number Diff line number Diff line
@@ -89,6 +89,17 @@ void testInvalidSharedMem_NoCrash(SensorManager &mgr) { 


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

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



    // Secondary test: correct channel creation & destruction (should print 0)

    ret = mgr.createDirectChannel(kMemSize, ASENSOR_DIRECT_CHANNEL_TYPE_HARDWARE_BUFFER,

                                  resourceHandle);

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



    // Third test: double-destroy (should not crash)

    mgr.destroyDirectChannel(ret);

    AHardwareBuffer_release(hardwareBuffer);

    printf("duplicate destroyDirectChannel...\n");

    mgr.destroyDirectChannel(ret);

}



int main() {