From df549ea500d842d3c35303183a69b6474a8d47b7 Mon Sep 17 00:00:00 2001 From: Anh Pham Date: Mon, 22 Mar 2021 11:50:48 +0100 Subject: Revert "Resample sensor events for non-direct connections." This reverts commit 532e655e42fe641688a9bfa42c427505ae303255. See discussion at b/179649922. Test: atest CtsSensorTestCases CtsSensorRatePermissionTestCases Bug: 136069189 Bug: 179649922 Change-Id: I4f6957428b4ac438a1cc9d7ba3f333f7f43d2f16 --- services/sensorservice/SensorEventConnection.cpp | 36 ++++-------------------- services/sensorservice/SensorEventConnection.h | 6 ---- 2 files changed, 5 insertions(+), 37 deletions(-) diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp index 5225dd7a64..9ce8d9bb18 100644 --- a/services/sensorservice/SensorEventConnection.cpp +++ b/services/sensorservice/SensorEventConnection.cpp @@ -287,29 +287,6 @@ bool SensorService::SensorEventConnection::incrementPendingFlushCountIfHasAccess } } -// TODO(b/179649922): A better algorithm to guarantee that capped connections will get a sampling -// rate close to 200 Hz. With the current algorithm, apps might be punished unfairly: E.g.,two apps -// make requests to the sensor service at the same time, one is not capped and uses 250 Hz, and one -//is capped, the capped connection will only get 125 Hz. -void SensorService::SensorEventConnection::addSensorEventsToBuffer(bool shouldResample, - const sensors_event_t& sensorEvent, sensors_event_t* buffer, int* index) { - if (!shouldResample || !mService->isSensorInCappedSet(sensorEvent.type)) { - buffer[(*index)++] = sensorEvent; - } else { - int64_t lastTimestamp = -1; - auto entry = mSensorLastTimestamp.find(sensorEvent.sensor); - if (entry != mSensorLastTimestamp.end()) { - lastTimestamp = entry->second; - } - // Allow 10% headroom here because the clocks are not perfect. - if (lastTimestamp == -1 || sensorEvent.timestamp - lastTimestamp - >= 0.9 * SENSOR_SERVICE_CAPPED_SAMPLING_PERIOD_NS) { - mSensorLastTimestamp[sensorEvent.sensor] = sensorEvent.timestamp; - buffer[(*index)++] = sensorEvent; - } - } -} - status_t SensorService::SensorEventConnection::sendEvents( sensors_event_t const* buffer, size_t numEvents, sensors_event_t* scratch, @@ -318,8 +295,6 @@ status_t SensorService::SensorEventConnection::sendEvents( std::unique_ptr sanitizedBuffer; - bool shouldResample = mService->isMicSensorPrivacyEnabledForUid(mUid) || - mIsRateCappedBasedOnPermission; int count = 0; Mutex::Autolock _l(mConnectionLock); if (scratch) { @@ -373,7 +348,7 @@ status_t SensorService::SensorEventConnection::sendEvents( // Regular sensor event, just copy it to the scratch buffer after checking // the AppOp. if (hasSensorAccess() && noteOpIfRequired(buffer[i])) { - addSensorEventsToBuffer(shouldResample, buffer[i], scratch, &count); + scratch[count++] = buffer[i]; } } i++; @@ -383,13 +358,12 @@ status_t SensorService::SensorEventConnection::sendEvents( buffer[i].meta_data.sensor == sensor_handle))); } } else { - sanitizedBuffer.reset(new sensors_event_t[numEvents]); - scratch = sanitizedBuffer.get(); if (hasSensorAccess()) { - for (size_t i = 0; i < numEvents; i++) { - addSensorEventsToBuffer(shouldResample, buffer[i], scratch, &count); - } + scratch = const_cast(buffer); + count = numEvents; } else { + sanitizedBuffer.reset(new sensors_event_t[numEvents]); + scratch = sanitizedBuffer.get(); for (size_t i = 0; i < numEvents; i++) { if (buffer[i].type == SENSOR_TYPE_META_DATA) { scratch[count++] = buffer[i++]; diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h index 4e3f120ccc..909053be50 100644 --- a/services/sensorservice/SensorEventConnection.h +++ b/services/sensorservice/SensorEventConnection.h @@ -145,16 +145,10 @@ private: void capRates(); // Recover sensor connection previously capped by capRates(). void uncapRates(); - - // Add sensorEvent to buffer at position index if the sensorEvent satisfies throttling rules. - void addSensorEventsToBuffer(bool shouldResample, const sensors_event_t& sensorEvent, - sensors_event_t* buffer, int* index); sp const mService; sp mChannel; uid_t mUid; std::atomic_bool mIsRateCappedBasedOnPermission; - // Store a mapping of sensor to the timestamp of their last sensor event. - std::unordered_map mSensorLastTimestamp; mutable Mutex mConnectionLock; // Number of events from wake up sensors which are still pending and haven't been delivered to // the corresponding application. It is incremented by one unit for each write to the socket. -- cgit v1.2.3-59-g8ed1b From b04658bdf402b3a1dabd9f5722ec85b30bc35bbf Mon Sep 17 00:00:00 2001 From: Anh Pham Date: Mon, 22 Mar 2021 18:17:17 +0100 Subject: Clear calling ID before using SensorPrivacyManager This is needed because some methods of SensorPrivacyManager have been gated by a system permission. As a result, we have to clear binder caller identity before calling these methods so that they are executed with the same level of privilege as the SensorService. Otherwise, if an app A makes a request to the SensorService to set up a sensor data connection and the SensorService needs to call SensorPrivacyService e.g., to register for a mic toggle state listener, the SensorPrivacyService will think that it's app A that is calling, hence failing the permission check. Test: atest CtsSensorTestCases CtsSensorRatePermissionTestCases Bug: 136069189 Bug: 179649922 Change-Id: Ib665230fe4b6dd7c598289b8af62171222855e03 --- services/sensorservice/SensorService.cpp | 4 +++- services/sensorservice/SensorService.h | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp index 942b7ae76a..9955cdb080 100644 --- a/services/sensorservice/SensorService.cpp +++ b/services/sensorservice/SensorService.cpp @@ -2129,12 +2129,14 @@ status_t SensorService::adjustRateLevelBasedOnMicAndPermission(int* requestedRat } void SensorService::SensorPrivacyPolicy::registerSelf() { + AutoCallerClear acc; SensorPrivacyManager spm; mSensorPrivacyEnabled = spm.isSensorPrivacyEnabled(); spm.addSensorPrivacyListener(this); } void SensorService::SensorPrivacyPolicy::unregisterSelf() { + AutoCallerClear acc; SensorPrivacyManager spm; spm.removeSensorPrivacyListener(this); } @@ -2167,7 +2169,7 @@ binder::Status SensorService::SensorPrivacyPolicy::onSensorPrivacyChanged(bool e status_t SensorService::SensorPrivacyPolicy::registerSelfForIndividual(int userId) { Mutex::Autolock _l(mSensorPrivacyLock); - + AutoCallerClear acc; SensorPrivacyManager spm; status_t err = spm.addIndividualSensorPrivacyListener(userId, SensorPrivacyManager::INDIVIDUAL_SENSOR_MICROPHONE, this); diff --git a/services/sensorservice/SensorService.h b/services/sensorservice/SensorService.h index 9c5060a577..a563a60607 100644 --- a/services/sensorservice/SensorService.h +++ b/services/sensorservice/SensorService.h @@ -242,6 +242,22 @@ private: userid_t mUserId; }; + // A class automatically clearing and restoring binder caller identity inside + // a code block (scoped variable). + // Declare one systematically before calling SensorPrivacyManager methods so that they are + // executed with the same level of privilege as the SensorService process. + class AutoCallerClear { + public: + AutoCallerClear() : + mToken(IPCThreadState::self()->clearCallingIdentity()) {} + ~AutoCallerClear() { + IPCThreadState::self()->restoreCallingIdentity(mToken); + } + + private: + const int64_t mToken; + }; + enum Mode { // The regular operating mode where any application can register/unregister/call flush on // sensors. -- cgit v1.2.3-59-g8ed1b