From 54aacdf96a7f57b97f8b6f1a69a8b39ae5ed4b18 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 Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 Change-Id: I0e255c64243c38876fb657cbf942fc1613363216 (cherry picked from commit aeec1802f7befc8fbb18313ad3ac0969c3811870) Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 --- libs/sensor/Sensor.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/sensor/Sensor.cpp b/libs/sensor/Sensor.cpp index 0a49008584..efb18c0e31 100644 --- a/libs/sensor/Sensor.cpp +++ b/libs/sensor/Sensor.cpp @@ -612,7 +612,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 cf83b16a403bfa8adf5a1490bbd14c8459e19140 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 Merged-In: I509cceb41f56ca117d9475f6f6674244560fe582 Change-Id: I509cceb41f56ca117d9475f6f6674244560fe582 (cherry picked from commit c95fa0f0e7c7b73746ff850b85a79fc5f92b784e) Merged-In: 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 62f4b4e3e2..625612c0c9 100644 --- a/libs/sensor/SensorManager.cpp +++ b/libs/sensor/SensorManager.cpp @@ -162,6 +162,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 16af3e02e5864c3d97548f0acd05baf5e3341950 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 Change-Id: I52185f74ae8d28b379440235ca6f03c5089081f5 (cherry picked from commit 9532f7c682fdd4b1e6e553cd6f61fc0cf2555902) Merged-In: 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 625612c0c9..cc5bcd8422 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 09ac7edf27..b2a9e1eb1b 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 From d1fe36445549e104ea6f984073e663b524b6f2f6 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 Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 Change-Id: I0e255c64243c38876fb657cbf942fc1613363216 (cherry picked from commit aeec1802f7befc8fbb18313ad3ac0969c3811870) Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 --- libs/sensor/Sensor.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/sensor/Sensor.cpp b/libs/sensor/Sensor.cpp index 0a49008584..efb18c0e31 100644 --- a/libs/sensor/Sensor.cpp +++ b/libs/sensor/Sensor.cpp @@ -612,7 +612,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 6447432d8ec653b43efaa7cd1baf069ddae006ae 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 Merged-In: I509cceb41f56ca117d9475f6f6674244560fe582 Change-Id: I509cceb41f56ca117d9475f6f6674244560fe582 (cherry picked from commit c95fa0f0e7c7b73746ff850b85a79fc5f92b784e) Merged-In: 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 62f4b4e3e2..625612c0c9 100644 --- a/libs/sensor/SensorManager.cpp +++ b/libs/sensor/SensorManager.cpp @@ -162,6 +162,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 dd5384d875d0f56c201792b373f8fa4f69966531 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 Change-Id: I52185f74ae8d28b379440235ca6f03c5089081f5 (cherry picked from commit 9532f7c682fdd4b1e6e553cd6f61fc0cf2555902) Merged-In: 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 625612c0c9..cc5bcd8422 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 09ac7edf27..b2a9e1eb1b 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 From 962b5a4d37bb13568ae4d93d10db9a3eb5166a38 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 Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 Change-Id: I0e255c64243c38876fb657cbf942fc1613363216 (cherry picked from commit aeec1802f7befc8fbb18313ad3ac0969c3811870) Merged-In: 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 2f8fa1367c62365cdd0a2de78a1c25cdcc599430 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 Merged-In: I509cceb41f56ca117d9475f6f6674244560fe582 Change-Id: I509cceb41f56ca117d9475f6f6674244560fe582 (cherry picked from commit c95fa0f0e7c7b73746ff850b85a79fc5f92b784e) Merged-In: 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 39e2df73052b5079d6b16a1535b332d6bda5bd89 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 Change-Id: I52185f74ae8d28b379440235ca6f03c5089081f5 (cherry picked from commit 9532f7c682fdd4b1e6e553cd6f61fc0cf2555902) Merged-In: 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 From 9170e60bf6306e65f936e9ec723ccfb054c181dd 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 Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 Change-Id: I0e255c64243c38876fb657cbf942fc1613363216 (cherry picked from commit aeec1802f7befc8fbb18313ad3ac0969c3811870) Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 --- libs/sensor/Sensor.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/sensor/Sensor.cpp b/libs/sensor/Sensor.cpp index 0a49008584..efb18c0e31 100644 --- a/libs/sensor/Sensor.cpp +++ b/libs/sensor/Sensor.cpp @@ -612,7 +612,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 e54698d02ce9a0058478c838545aa6ba52ca96c1 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 Merged-In: I509cceb41f56ca117d9475f6f6674244560fe582 Change-Id: I509cceb41f56ca117d9475f6f6674244560fe582 (cherry picked from commit c95fa0f0e7c7b73746ff850b85a79fc5f92b784e) Merged-In: 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 62f4b4e3e2..625612c0c9 100644 --- a/libs/sensor/SensorManager.cpp +++ b/libs/sensor/SensorManager.cpp @@ -162,6 +162,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 972675ed8101c0f0ed98688c9ffb071d9c0bb872 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 Change-Id: I52185f74ae8d28b379440235ca6f03c5089081f5 (cherry picked from commit 9532f7c682fdd4b1e6e553cd6f61fc0cf2555902) Merged-In: 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 625612c0c9..cc5bcd8422 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 09ac7edf27..b2a9e1eb1b 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 From 6d2f8c55984e13ad0cb94a2d45d1f39318ffac4b 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 Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 Change-Id: I0e255c64243c38876fb657cbf942fc1613363216 (cherry picked from commit aeec1802f7befc8fbb18313ad3ac0969c3811870) Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 --- libs/sensor/Sensor.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/sensor/Sensor.cpp b/libs/sensor/Sensor.cpp index 9d817ae0bd..d71f27106e 100644 --- a/libs/sensor/Sensor.cpp +++ b/libs/sensor/Sensor.cpp @@ -595,7 +595,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 93a74aaca9b6edcd7cead607a515c111d616870f 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 Merged-In: I509cceb41f56ca117d9475f6f6674244560fe582 Change-Id: I509cceb41f56ca117d9475f6f6674244560fe582 (cherry picked from commit c95fa0f0e7c7b73746ff850b85a79fc5f92b784e) Merged-In: 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 8ed09f8ff0..2ec3310639 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 a4a5d135c0..e639498c28 100644 --- a/libs/sensor/SensorManager.cpp +++ b/libs/sensor/SensorManager.cpp @@ -162,6 +162,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 be6b8a58e6472e1af0acb59fc4e6e795a3a4c125 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 Change-Id: I52185f74ae8d28b379440235ca6f03c5089081f5 (cherry picked from commit 9532f7c682fdd4b1e6e553cd6f61fc0cf2555902) Merged-In: 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 e639498c28..f5dd36adae 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 f09c9c67b4..127f74f4b2 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 From f1aa5fb53437ec2fabc9be00099af836da5f07f2 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 Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 Change-Id: I0e255c64243c38876fb657cbf942fc1613363216 (cherry picked from commit aeec1802f7befc8fbb18313ad3ac0969c3811870) Merged-In: I0e255c64243c38876fb657cbf942fc1613363216 --- libs/sensor/Sensor.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/sensor/Sensor.cpp b/libs/sensor/Sensor.cpp index 9d817ae0bd..d71f27106e 100644 --- a/libs/sensor/Sensor.cpp +++ b/libs/sensor/Sensor.cpp @@ -595,7 +595,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 ceb0d52273256c6a5c5622bf81b0ac4ba106faa1 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 Merged-In: I509cceb41f56ca117d9475f6f6674244560fe582 Change-Id: I509cceb41f56ca117d9475f6f6674244560fe582 (cherry picked from commit c95fa0f0e7c7b73746ff850b85a79fc5f92b784e) Merged-In: 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 8ed09f8ff0..2ec3310639 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 a4a5d135c0..e639498c28 100644 --- a/libs/sensor/SensorManager.cpp +++ b/libs/sensor/SensorManager.cpp @@ -162,6 +162,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 4521fbf8095439a1c1681b5c709b306a5dc1d1e3 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 Change-Id: I52185f74ae8d28b379440235ca6f03c5089081f5 (cherry picked from commit 9532f7c682fdd4b1e6e553cd6f61fc0cf2555902) Merged-In: 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 e639498c28..f5dd36adae 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 f09c9c67b4..127f74f4b2 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