From fb4585b3dc8e5015dc91469ae004337601172aa9 Mon Sep 17 00:00:00 2001 From: Devin Moore Date: Fri, 17 Feb 2023 17:12:46 +0000 Subject: Check for malformed Sensor Flattenable Test: libsensorserviceaidl_fuzzer with testcase from bug Bug: 269014004 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:aeec1802f7befc8fbb18313ad3ac0969c3811870) Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 Change-Id: I0e255c64243c38876fb657cbf942fc1613363216 --- libs/sensor/Sensor.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/sensor/Sensor.cpp b/libs/sensor/Sensor.cpp index ec0ced8663..b865c4d5d6 100644 --- a/libs/sensor/Sensor.cpp +++ b/libs/sensor/Sensor.cpp @@ -632,7 +632,13 @@ bool Sensor::unflattenString8(void const*& buffer, size_t& size, String8& output return false; } outputString8.setTo(static_cast(buffer), len); + + if (size < FlattenableUtils::align<4>(len)) { + ALOGE("Malformed Sensor String8 field. Should be in a 4-byte aligned buffer but is not."); + return false; + } FlattenableUtils::advance(buffer, size, FlattenableUtils::align<4>(len)); + return true; } -- cgit v1.2.3 From edb3f8e57fc25d3b0abd8cfc601cea814e90fcce Mon Sep 17 00:00:00 2001 From: Devin Moore Date: Fri, 17 Feb 2023 19:35:25 +0000 Subject: Remove some new memory leaks from SensorManager After catching an error in Sensor::unflatten, there are memory leaks caught by the fuzzer in the same test case. Test: libsensorserviceaidl_fuzzer with testcase from bug Bug: 269014004 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:c95fa0f0e7c7b73746ff850b85a79fc5f92b784e) Merged-In: I509cceb41f56ca117d9475f6f6674244560fe582 Change-Id: I509cceb41f56ca117d9475f6f6674244560fe582 --- libs/sensor/ISensorServer.cpp | 12 ++++++++++-- libs/sensor/SensorManager.cpp | 5 +++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/libs/sensor/ISensorServer.cpp b/libs/sensor/ISensorServer.cpp index a6cacad374..93c95b98c5 100644 --- a/libs/sensor/ISensorServer.cpp +++ b/libs/sensor/ISensorServer.cpp @@ -66,7 +66,11 @@ public: v.setCapacity(n); while (n) { n--; - reply.read(s); + if(reply.read(s) != OK) { + ALOGE("Failed to read reply from getSensorList"); + v.clear(); + break; + } v.add(s); } return v; @@ -84,7 +88,11 @@ public: v.setCapacity(n); while (n) { n--; - reply.read(s); + if(reply.read(s) != OK) { + ALOGE("Failed to read reply from getDynamicSensorList"); + v.clear(); + break; + } v.add(s); } return v; diff --git a/libs/sensor/SensorManager.cpp b/libs/sensor/SensorManager.cpp index 0ba9704263..c0525d4f5d 100644 --- a/libs/sensor/SensorManager.cpp +++ b/libs/sensor/SensorManager.cpp @@ -166,6 +166,11 @@ status_t SensorManager::assertStateLocked() { mSensors = mSensorServer->getSensorList(mOpPackageName); size_t count = mSensors.size(); + if (count == 0) { + ALOGE("Failed to get Sensor list"); + mSensorServer.clear(); + return UNKNOWN_ERROR; + } mSensorList = static_cast(malloc(count * sizeof(Sensor*))); LOG_ALWAYS_FATAL_IF(mSensorList == nullptr, "mSensorList NULL"); -- cgit v1.2.3 From 455ada53fcc0d3d06671f41993c6fd6c33d5db02 Mon Sep 17 00:00:00 2001 From: Anthony Stange Date: Tue, 21 Feb 2023 17:57:38 +0000 Subject: Add removeInstanceForPackageMethod to SensorManager In order to ensure that clients don't leak their sensor manager instance that we currently store in a static map, they need to be able to remove their instance. Otherwise, this instance is never removed from the list and will hang around until our SensorManage instance is destroyed. Bug: 269014004 Test: Run ./libsensorserviceaidl_fuzzer (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:9532f7c682fdd4b1e6e553cd6f61fc0cf2555902) Merged-In: I52185f74ae8d28b379440235ca6f03c5089081f5 Change-Id: I52185f74ae8d28b379440235ca6f03c5089081f5 --- libs/sensor/SensorManager.cpp | 10 ++++++++++ libs/sensor/include/sensor/SensorManager.h | 1 + services/sensorservice/hidl/SensorManager.cpp | 3 +++ 3 files changed, 14 insertions(+) diff --git a/libs/sensor/SensorManager.cpp b/libs/sensor/SensorManager.cpp index c0525d4f5d..40061cde61 100644 --- a/libs/sensor/SensorManager.cpp +++ b/libs/sensor/SensorManager.cpp @@ -92,6 +92,16 @@ SensorManager& SensorManager::getInstanceForPackage(const String16& packageName) return *sensorManager; } +void SensorManager::removeInstanceForPackage(const String16& packageName) { + Mutex::Autolock _l(sLock); + auto iterator = sPackageInstances.find(packageName); + if (iterator != sPackageInstances.end()) { + SensorManager* sensorManager = iterator->second; + delete sensorManager; + sPackageInstances.erase(iterator); + } +} + SensorManager::SensorManager(const String16& opPackageName) : mSensorList(nullptr), mOpPackageName(opPackageName), mDirectConnectionHandle(1) { Mutex::Autolock _l(mLock); diff --git a/libs/sensor/include/sensor/SensorManager.h b/libs/sensor/include/sensor/SensorManager.h index 8d0a8a45d9..7c9d604ff7 100644 --- a/libs/sensor/include/sensor/SensorManager.h +++ b/libs/sensor/include/sensor/SensorManager.h @@ -54,6 +54,7 @@ class SensorManager : public ASensorManager { public: static SensorManager& getInstanceForPackage(const String16& packageName); + static void removeInstanceForPackage(const String16& packageName); ~SensorManager(); ssize_t getSensorList(Sensor const* const** list); diff --git a/services/sensorservice/hidl/SensorManager.cpp b/services/sensorservice/hidl/SensorManager.cpp index 938060063f..0a4e68412d 100644 --- a/services/sensorservice/hidl/SensorManager.cpp +++ b/services/sensorservice/hidl/SensorManager.cpp @@ -60,6 +60,9 @@ SensorManager::~SensorManager() { if (mPollThread.joinable()) { mPollThread.join(); } + + ::android::SensorManager::removeInstanceForPackage( + String16(ISensorManager::descriptor)); } // Methods from ::android::frameworks::sensorservice::V1_0::ISensorManager follow. -- cgit v1.2.3