fixing the location api level race condition on callbacks

At the layer right under HIDL impl, where the callback objs
are received from HIDL and used by HAL layer, there is race
condition volnerability which could yield using a incompletely
copied sp obj. Added mutex protection.

Bug: 79541486

Change-Id: I611db590d1fadbe43c74db71a1ea906dbe067c6d
CRs-Fixed: 2144976
(cherry picked from commit f1ee0f801e75dc89ff776bdefd148c63d128b6b4)

4 files changed, 36 insertions, 14 deletions

diff --git a/msm8909w_3100/android/location_api/GnssAPIClient.cpp b/msm8909w_3100/android/location_api/GnssAPIClient.cpp
index 2745f56..2e44c2e 100644
--- a/msm8909w_3100/android/location_api/GnssAPIClient.cpp
+++ b/msm8909w_3100/android/location_api/GnssAPIClient.cpp
@@ -81,8 +81,10 @@ void GnssAPIClient::gnssUpdateCallbacks(const sp<IGnssCallback>& gpsCb,
 {
     LOC_LOGD("%s]: (%p %p)", __FUNCTION__, &gpsCb, &niCb);
 
+    mMutex.lock();
     mGnssCbIface = gpsCb;
     mGnssNiCbIface = niCb;
+    mMutex.unlock();
 
     LocationCallbacks locationCallbacks;
     memset(&locationCallbacks, 0, sizeof(LocationCallbacks));
@@ -278,7 +280,10 @@ void GnssAPIClient::onCapabilitiesCb(LocationCapabilitiesMask capabilitiesMask)
     LOC_LOGD("%s]: (%02x)", __FUNCTION__, capabilitiesMask);
     mLocationCapabilitiesMask = capabilitiesMask;
     mLocationCapabilitiesCached = true;
-    sp<IGnssCallback> gnssCbIface = mGnssCbIface;
+
+    mMutex.lock();
+    auto gnssCbIface(mGnssCbIface);
+    mMutex.unlock();
 
     if (gnssCbIface != nullptr) {
         uint32_t data = 0;
@@ -322,7 +327,9 @@ void GnssAPIClient::onCapabilitiesCb(LocationCapabilitiesMask capabilitiesMask)
 void GnssAPIClient::onTrackingCb(Location location)
 {
     LOC_LOGD("%s]: (flags: %02x)", __FUNCTION__, location.flags);
-    sp<IGnssCallback> gnssCbIface = mGnssCbIface;
+    mMutex.lock();
+    auto gnssCbIface(mGnssCbIface);
+    mMutex.unlock();
 
     if (gnssCbIface != nullptr) {
         GnssLocation gnssLocation;
@@ -338,7 +345,9 @@ void GnssAPIClient::onTrackingCb(Location location)
 void GnssAPIClient::onGnssNiCb(uint32_t id, GnssNiNotification gnssNiNotification)
 {
     LOC_LOGD("%s]: (id: %d)", __FUNCTION__, id);
-    sp<IGnssNiCallback> gnssNiCbIface = mGnssNiCbIface;
+    mMutex.lock();
+    auto gnssNiCbIface(mGnssNiCbIface);
+    mMutex.unlock();
 
     if (gnssNiCbIface == nullptr) {
         LOC_LOGE("%s]: mGnssNiCbIface is nullptr", __FUNCTION__);
@@ -411,7 +420,9 @@ void GnssAPIClient::onGnssNiCb(uint32_t id, GnssNiNotification gnssNiNotificatio
 void GnssAPIClient::onGnssSvCb(GnssSvNotification gnssSvNotification)
 {
     LOC_LOGD("%s]: (count: %zu)", __FUNCTION__, gnssSvNotification.count);
-    sp<IGnssCallback> gnssCbIface = mGnssCbIface;
+    mMutex.lock();
+    auto gnssCbIface(mGnssCbIface);
+    mMutex.unlock();
 
     if (gnssCbIface != nullptr) {
         IGnssCallback::GnssSvStatus svStatus;
@@ -426,7 +437,9 @@ void GnssAPIClient::onGnssSvCb(GnssSvNotification gnssSvNotification)
 
 void GnssAPIClient::onGnssNmeaCb(GnssNmeaNotification gnssNmeaNotification)
 {
-    sp<IGnssCallback> gnssCbIface = mGnssCbIface;
+    mMutex.lock();
+    auto gnssCbIface(mGnssCbIface);
+    mMutex.unlock();
 
     if (gnssCbIface != nullptr) {
         android::hardware::hidl_string nmeaString;
@@ -443,7 +456,9 @@ void GnssAPIClient::onGnssNmeaCb(GnssNmeaNotification gnssNmeaNotification)
 void GnssAPIClient::onStartTrackingCb(LocationError error)
 {
     LOC_LOGD("%s]: (%d)", __FUNCTION__, error);
-    sp<IGnssCallback> gnssCbIface = mGnssCbIface;
+    mMutex.lock();
+    auto gnssCbIface(mGnssCbIface);
+    mMutex.unlock();
 
     if (error == LOCATION_ERROR_SUCCESS && gnssCbIface != nullptr) {
         auto r = gnssCbIface->gnssStatusCb(IGnssCallback::GnssStatusValue::ENGINE_ON);
@@ -462,7 +477,9 @@ void GnssAPIClient::onStartTrackingCb(LocationError error)
 void GnssAPIClient::onStopTrackingCb(LocationError error)
 {
     LOC_LOGD("%s]: (%d)", __FUNCTION__, error);
-    sp<IGnssCallback> gnssCbIface = mGnssCbIface;
+    mMutex.lock();
+    auto gnssCbIface(mGnssCbIface);
+    mMutex.unlock();
 
     if (error == LOCATION_ERROR_SUCCESS && gnssCbIface != nullptr) {
         auto r = gnssCbIface->gnssStatusCb(IGnssCallback::GnssStatusValue::SESSION_END);
diff --git a/msm8909w_3100/android/location_api/GnssAPIClient.h b/msm8909w_3100/android/location_api/GnssAPIClient.h
index 295a9e9..b5cffb1 100644
--- a/msm8909w_3100/android/location_api/GnssAPIClient.h
+++ b/msm8909w_3100/android/location_api/GnssAPIClient.h
@@ -30,7 +30,7 @@
 #ifndef GNSS_API_CLINET_H
 #define GNSS_API_CLINET_H
 
-
+#include <mutex>
 #include <android/hardware/gnss/1.0/IGnss.h>
 #include <android/hardware/gnss/1.0/IGnssCallback.h>
 #include <android/hardware/gnss/1.0/IGnssNiCallback.h>
@@ -91,11 +91,10 @@ public:
 private:
     sp<IGnssCallback> mGnssCbIface;
     sp<IGnssNiCallback> mGnssNiCbIface;
-
+    std::mutex mMutex;
     LocationAPIControlClient* mControlClient;
     LocationCapabilitiesMask mLocationCapabilitiesMask;
     bool mLocationCapabilitiesCached;
-
     LocationOptions mLocationOptions;
 };
 
diff --git a/msm8909w_3100/android/location_api/MeasurementAPIClient.cpp b/msm8909w_3100/android/location_api/MeasurementAPIClient.cpp
index dd7ceac..731c7ed 100644
--- a/msm8909w_3100/android/location_api/MeasurementAPIClient.cpp
+++ b/msm8909w_3100/android/location_api/MeasurementAPIClient.cpp
@@ -66,7 +66,9 @@ MeasurementAPIClient::measurementSetCallback(const sp<IGnssMeasurementCallback>&
 {
     LOC_LOGD("%s]: (%p)", __FUNCTION__, &callback);
 
+    mMutex.lock();
     mGnssMeasurementCbIface = callback;
+    mMutex.unlock();
 
     LocationCallbacks locationCallbacks;
     memset(&locationCallbacks, 0, sizeof(LocationCallbacks));
@@ -116,10 +118,14 @@ void MeasurementAPIClient::onGnssMeasurementsCb(
     LOC_LOGD("%s]: (count: %zu active: %zu)",
             __FUNCTION__, gnssMeasurementsNotification.count, mTracking);
     if (mTracking) {
-        if (mGnssMeasurementCbIface != nullptr) {
+        mMutex.lock();
+        auto gnssMeasurementCbIface(mGnssMeasurementCbIface);
+        mMutex.unlock();
+
+        if (gnssMeasurementCbIface != nullptr) {
             IGnssMeasurementCallback::GnssData gnssData;
             convertGnssData(gnssMeasurementsNotification, gnssData);
-            auto r = mGnssMeasurementCbIface->GnssMeasurementCb(gnssData);
+            auto r = gnssMeasurementCbIface->GnssMeasurementCb(gnssData);
             if (!r.isOk()) {
                 LOC_LOGE("%s] Error from GnssMeasurementCb description=%s",
                     __func__, r.description().c_str());
diff --git a/msm8909w_3100/android/location_api/MeasurementAPIClient.h b/msm8909w_3100/android/location_api/MeasurementAPIClient.h
index 422564d..8de1326 100644
--- a/msm8909w_3100/android/location_api/MeasurementAPIClient.h
+++ b/msm8909w_3100/android/location_api/MeasurementAPIClient.h
@@ -30,7 +30,7 @@
 #ifndef MEASUREMENT_API_CLINET_H
 #define MEASUREMENT_API_CLINET_H
 
-
+#include <mutex>
 #include <android/hardware/gnss/1.0/IGnssMeasurement.h>
 #include <android/hardware/gnss/1.0/IGnssMeasurementCallback.h>
 #include <LocationAPIClientBase.h>
@@ -63,7 +63,7 @@ public:
 
 private:
     sp<IGnssMeasurementCallback> mGnssMeasurementCbIface;
-
+    std::mutex mMutex;
     bool mTracking;
 };