MiPad4/android_device_xiaomi_sdm660-common
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fixing the location api level race condition on callbacks

Browse source 
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.

Change-Id: I611db590d1fadbe43c74db71a1ea906dbe067c6d
CRs-Fixed: 2144976
This commit is contained in:
Kevin Tang 2017-12-15 18:53:25 -08:00
parent 53a1aa5ed5
commit 03bc75f298
4 changed files with 36 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
android/location_api/GnssAPIClient.cpp
View file

@ -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__);
@ -415,7 +424,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;
@ -430,7 +441,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;
@ -447,7 +460,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);
@ -466,7 +481,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);

5
android/location_api/GnssAPIClient.h
View file

@ -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;
};

10
android/location_api/MeasurementAPIClient.cpp
View file

@ -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());

4
android/location_api/MeasurementAPIClient.h
View file

@ -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;
};