Merge tag 'android-security-10.0.0_r53' into int/10/fp2
Android security 10.0.0 release 53
* tag 'android-security-10.0.0_r53':
Use strong pointer in ALooper to avoid use-after-free
Ensure looper isn't used after invalidateSensorQueue
Change-Id: Ieac41ca5cb83e3caf8f25f67909ffd7d38b6a7c9
diff --git a/sensorservice/libsensorndkbridge/ALooper.cpp b/sensorservice/libsensorndkbridge/ALooper.cpp
index 9930ad4..d3ddbff 100644
--- a/sensorservice/libsensorndkbridge/ALooper.cpp
+++ b/sensorservice/libsensorndkbridge/ALooper.cpp
@@ -23,12 +23,14 @@
#include <android-base/logging.h>
using android::Mutex;
+using android::sp;
+using android::wp;
ALooper::ALooper()
: mAwoken(false) {
}
-void ALooper::signalSensorEvents(ASensorEventQueue *queue) {
+void ALooper::signalSensorEvents(wp<ASensorEventQueue> queue) {
Mutex::Autolock autoLock(mLock);
mReadyQueues.insert(queue);
mCondition.signal();
@@ -71,8 +73,11 @@
if (!mReadyQueues.empty()) {
result = ALOOPER_POLL_CALLBACK;
- for (auto queue : mReadyQueues) {
- queue->dispatchCallback();
+ for (auto& queue : mReadyQueues) {
+ sp<ASensorEventQueue> promotedQueue = queue.promote();
+ if (promotedQueue != nullptr) {
+ promotedQueue->dispatchCallback();
+ }
}
mReadyQueues.clear();
@@ -86,8 +91,7 @@
return result;
}
-void ALooper::invalidateSensorQueue(ASensorEventQueue *queue) {
+void ALooper::invalidateSensorQueue(wp<ASensorEventQueue> queue) {
Mutex::Autolock autoLock(mLock);
mReadyQueues.erase(queue);
}
-
diff --git a/sensorservice/libsensorndkbridge/ALooper.h b/sensorservice/libsensorndkbridge/ALooper.h
index 601eca9..aa14e03 100644
--- a/sensorservice/libsensorndkbridge/ALooper.h
+++ b/sensorservice/libsensorndkbridge/ALooper.h
@@ -21,6 +21,7 @@
#include <android-base/macros.h>
#include <utils/Condition.h>
#include <utils/Mutex.h>
+#include <utils/RefBase.h>
#include <set>
@@ -29,18 +30,18 @@
struct ALooper {
ALooper();
- void signalSensorEvents(ASensorEventQueue *queue);
+ void signalSensorEvents(android::wp<ASensorEventQueue> queue);
void wake();
int pollOnce(int timeoutMillis, int *outFd, int *outEvents, void **outData);
- void invalidateSensorQueue(ASensorEventQueue *queue);
+ void invalidateSensorQueue(android::wp<ASensorEventQueue> queue);
private:
android::Mutex mLock;
android::Condition mCondition;
- std::set<ASensorEventQueue *> mReadyQueues;
+ std::set<android::wp<ASensorEventQueue>> mReadyQueues;
bool mAwoken;
DISALLOW_COPY_AND_ASSIGN(ALooper);
diff --git a/sensorservice/libsensorndkbridge/ASensorEventQueue.cpp b/sensorservice/libsensorndkbridge/ASensorEventQueue.cpp
index d109426..bf24f68 100644
--- a/sensorservice/libsensorndkbridge/ASensorEventQueue.cpp
+++ b/sensorservice/libsensorndkbridge/ASensorEventQueue.cpp
@@ -30,7 +30,11 @@
using android::hardware::Return;
ASensorEventQueue::ASensorEventQueue(ALooper* looper, ALooper_callbackFunc callback, void* data)
- : mLooper(looper), mCallback(callback), mData(data), mRequestAdditionalInfo(false) {}
+ : mLooper(looper),
+ mCallback(callback),
+ mData(data),
+ mRequestAdditionalInfo(false),
+ mValid(true) {}
void ASensorEventQueue::setImpl(const sp<IEventQueue> &queueImpl) {
mQueueImpl = queueImpl;
@@ -106,17 +110,31 @@
LOG(VERBOSE) << "ASensorEventQueue::onEvent";
if (static_cast<int32_t>(event.sensorType) != ASENSOR_TYPE_ADDITIONAL_INFO ||
- mRequestAdditionalInfo.load()) {
- {
- Mutex::Autolock autoLock(mLock);
+ mRequestAdditionalInfo.load()) {
+ {
+ // Only lock the mutex in this block to avoid the following deadlock scenario:
+ //
+ // ASensorEventQueue::onEvent is called which grabs ASensorEventQueue::mLock followed
+ // by ALooper::mLock via ALooper::signalSensorEvents.
+ //
+ // Meanwhile
+ //
+ // ASensorEventQueue::dispatchCallback is invoked from ALooper::pollOnce which has
+ // has ALooper::mLock locked and the dispatched callback invokes
+ // ASensorEventQueue::getEvents which would try to grab ASensorEventQueue::mLock
+ // resulting in a deadlock.
+ Mutex::Autolock autoLock(mLock);
mQueue.emplace_back();
sensors_event_t* sensorEvent = &mQueue[mQueue.size() - 1];
android::hardware::sensors::V1_0::implementation::convertToSensorEvent(event,
sensorEvent);
}
- mLooper->signalSensorEvents(this);
+ Mutex::Autolock autoLock(mValidLock);
+ if (mValid) {
+ mLooper->signalSensorEvents(this);
+ }
}
return android::hardware::Void();
@@ -134,6 +152,13 @@
}
void ASensorEventQueue::invalidate() {
+ {
+ // Only lock within this context to avoid locking while calling invalidateSensorQueue which
+ // also holds a lock. This is safe to do because mValid can't be made true after it's false
+ // so onEvent will never signal new sensor events after mValid is false.
+ Mutex::Autolock autoLock(mValidLock);
+ mValid = false;
+ }
mLooper->invalidateSensorQueue(this);
setImpl(nullptr);
}
diff --git a/sensorservice/libsensorndkbridge/ASensorEventQueue.h b/sensorservice/libsensorndkbridge/ASensorEventQueue.h
index 4937276..7139d34 100644
--- a/sensorservice/libsensorndkbridge/ASensorEventQueue.h
+++ b/sensorservice/libsensorndkbridge/ASensorEventQueue.h
@@ -73,6 +73,8 @@
std::vector<sensors_event_t> mQueue;
std::atomic_bool mRequestAdditionalInfo;
+ android::Mutex mValidLock;
+ bool mValid;
DISALLOW_COPY_AND_ASSIGN(ASensorEventQueue);
};
diff --git a/sensorservice/libsensorndkbridge/ASensorManager.h b/sensorservice/libsensorndkbridge/ASensorManager.h
index 818429d..4e91122 100644
--- a/sensorservice/libsensorndkbridge/ASensorManager.h
+++ b/sensorservice/libsensorndkbridge/ASensorManager.h
@@ -44,6 +44,8 @@
ALooper_callbackFunc callback,
void *data);
+ // This must not be called from inside ALooper_callbackFunc to avoid deadlocking inside of the
+ // ALooper.
void destroyEventQueue(ASensorEventQueue *queue);
private: