-
Notifications
You must be signed in to change notification settings - Fork 249
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
ASB-August 2024 Security Patches integration
Integrating Google Android Security Bulletin Patches Test done: STS r29 TCs Passed. Tracked-On: OAM-122613 Signed-off-by: Alam, Sahibex <[email protected]>
- Loading branch information
Showing
15 changed files
with
1,476 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
163 changes: 163 additions & 0 deletions
163
...ks/av/0031-libmediatranscoding-handle-death-recipient-cookie-ownership-dif.bulletin.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
From f21c760322b58d7cb92579cb18ff75869ddd345c Mon Sep 17 00:00:00 2001 | ||
From: Devin Moore <[email protected]> | ||
Date: Fri, 23 Feb 2024 19:19:38 +0000 | ||
Subject: [PATCH] libmediatranscoding: handle death recipient cookie ownership | ||
differently | ||
|
||
The ownership of the death recipient cookie is now limited to the | ||
TranscodingResourcePolicy object and the binderDied callback. | ||
They both must be able to delete the cookie object and they both must be | ||
aware of it already being deleted. | ||
|
||
In all cases, the TranscodingResourcePolicy object that needs to be | ||
unregistered will outlive the cookie and the death recipient. | ||
|
||
Calling unlinkToDeath is unneccessary because the last strong ref to the | ||
binder that was linked to death is removed in the unregisterSelf method | ||
which will unlink the binder and death recipient. | ||
|
||
Test: atest CtsMediaTranscodingTestCases MediaSampleReaderNDKTests | ||
Test: adb shell kill -9 `pid media.resource_observer` | ||
Test: delete mResourcePolicy.get() to force destructor after linkToDeath | ||
Bug: 319210610 | ||
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0c674f5ff68daa64b90e1a234061ba9bebe6173c) | ||
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0213a7d62ea5545b954145cc53e2bd65ed5dc609) | ||
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:686987b9613678fe6540079e85e0d94519551738) | ||
Merged-In: I8e6ba40fe3da30bf8753e7a16ad5c8cd5dfda40b | ||
Change-Id: I8e6ba40fe3da30bf8753e7a16ad5c8cd5dfda40b | ||
--- | ||
.../TranscodingResourcePolicy.cpp | 58 +++++++++++++++++-- | ||
.../include/media/TranscodingResourcePolicy.h | 4 ++ | ||
2 files changed, 57 insertions(+), 5 deletions(-) | ||
|
||
diff --git a/media/libmediatranscoding/TranscodingResourcePolicy.cpp b/media/libmediatranscoding/TranscodingResourcePolicy.cpp | ||
index af53f64671..1b91362463 100644 | ||
--- a/media/libmediatranscoding/TranscodingResourcePolicy.cpp | ||
+++ b/media/libmediatranscoding/TranscodingResourcePolicy.cpp | ||
@@ -24,6 +24,8 @@ | ||
#include <media/TranscodingResourcePolicy.h> | ||
#include <utils/Log.h> | ||
|
||
+#include <map> | ||
+ | ||
namespace android { | ||
|
||
using Status = ::ndk::ScopedAStatus; | ||
@@ -66,11 +68,31 @@ struct TranscodingResourcePolicy::ResourceObserver : public BnResourceObserver { | ||
TranscodingResourcePolicy* mOwner; | ||
}; | ||
|
||
+// cookie used for death recipients. The TranscodingResourcePolicy | ||
+// that this cookie is associated with must outlive this cookie. It is | ||
+// either deleted by binderDied, or in unregisterSelf which is also called | ||
+// in the destructor of TranscodingResourcePolicy | ||
+class TranscodingResourcePolicyCookie { | ||
+public: | ||
+ TranscodingResourcePolicyCookie(TranscodingResourcePolicy* policy) : mPolicy(policy) {} | ||
+ TranscodingResourcePolicyCookie() = delete; | ||
+ TranscodingResourcePolicy* mPolicy; | ||
+}; | ||
+ | ||
+static std::map<uintptr_t, std::unique_ptr<TranscodingResourcePolicyCookie>> sCookies; | ||
+static uintptr_t sCookieKeyCounter; | ||
+static std::mutex sCookiesMutex; | ||
+ | ||
// static | ||
void TranscodingResourcePolicy::BinderDiedCallback(void* cookie) { | ||
- TranscodingResourcePolicy* owner = reinterpret_cast<TranscodingResourcePolicy*>(cookie); | ||
- if (owner != nullptr) { | ||
- owner->unregisterSelf(); | ||
+ std::lock_guard<std::mutex> guard(sCookiesMutex); | ||
+ if (auto it = sCookies.find(reinterpret_cast<uintptr_t>(cookie)); it != sCookies.end()) { | ||
+ ALOGI("BinderDiedCallback unregistering TranscodingResourcePolicy"); | ||
+ auto policy = reinterpret_cast<TranscodingResourcePolicy*>(it->second->mPolicy); | ||
+ if (policy) { | ||
+ policy->unregisterSelf(); | ||
+ } | ||
+ sCookies.erase(it); | ||
} | ||
// TODO(chz): retry to connecting to IResourceObserverService after failure. | ||
// Also need to have back-up logic if IResourceObserverService is offline for | ||
@@ -88,6 +110,24 @@ TranscodingResourcePolicy::TranscodingResourcePolicy() | ||
} | ||
|
||
TranscodingResourcePolicy::~TranscodingResourcePolicy() { | ||
+ { | ||
+ std::lock_guard<std::mutex> guard(sCookiesMutex); | ||
+ | ||
+ // delete all of the cookies associated with this TranscodingResourcePolicy | ||
+ // instance since they are holding pointers to this object that will no | ||
+ // longer be valid. | ||
+ for (auto it = sCookies.begin(); it != sCookies.end();) { | ||
+ const uintptr_t key = it->first; | ||
+ std::lock_guard guard(mCookieKeysLock); | ||
+ if (mCookieKeys.find(key) != mCookieKeys.end()) { | ||
+ // No longer need to track this cookie | ||
+ mCookieKeys.erase(key); | ||
+ it = sCookies.erase(it); | ||
+ } else { | ||
+ it++; | ||
+ } | ||
+ } | ||
+ } | ||
unregisterSelf(); | ||
} | ||
|
||
@@ -123,7 +163,16 @@ void TranscodingResourcePolicy::registerSelf() { | ||
return; | ||
} | ||
|
||
- AIBinder_linkToDeath(binder.get(), mDeathRecipient.get(), reinterpret_cast<void*>(this)); | ||
+ std::unique_ptr<TranscodingResourcePolicyCookie> cookie = | ||
+ std::make_unique<TranscodingResourcePolicyCookie>(this); | ||
+ uintptr_t cookieKey = sCookieKeyCounter++; | ||
+ sCookies.emplace(cookieKey, std::move(cookie)); | ||
+ { | ||
+ std::lock_guard guard(mCookieKeysLock); | ||
+ mCookieKeys.insert(cookieKey); | ||
+ } | ||
+ | ||
+ AIBinder_linkToDeath(binder.get(), mDeathRecipient.get(), reinterpret_cast<void*>(cookieKey)); | ||
|
||
ALOGD("@@@ registered observer"); | ||
mRegistered = true; | ||
@@ -141,7 +190,6 @@ void TranscodingResourcePolicy::unregisterSelf() { | ||
::ndk::SpAIBinder binder = mService->asBinder(); | ||
if (binder.get() != nullptr) { | ||
Status status = mService->unregisterObserver(mObserver); | ||
- AIBinder_unlinkToDeath(binder.get(), mDeathRecipient.get(), reinterpret_cast<void*>(this)); | ||
} | ||
|
||
mService = nullptr; | ||
diff --git a/media/libmediatranscoding/include/media/TranscodingResourcePolicy.h b/media/libmediatranscoding/include/media/TranscodingResourcePolicy.h | ||
index ee232e7551..4d762b5832 100644 | ||
--- a/media/libmediatranscoding/include/media/TranscodingResourcePolicy.h | ||
+++ b/media/libmediatranscoding/include/media/TranscodingResourcePolicy.h | ||
@@ -22,6 +22,7 @@ | ||
#include <utils/Condition.h> | ||
|
||
#include <mutex> | ||
+#include <set> | ||
namespace aidl { | ||
namespace android { | ||
namespace media { | ||
@@ -48,6 +49,8 @@ private: | ||
bool mRegistered GUARDED_BY(mRegisteredLock); | ||
std::shared_ptr<IResourceObserverService> mService GUARDED_BY(mRegisteredLock); | ||
std::shared_ptr<ResourceObserver> mObserver; | ||
+ mutable std::mutex mCookieKeysLock; | ||
+ std::set<uintptr_t> mCookieKeys; | ||
|
||
mutable std::mutex mCallbackLock; | ||
std::weak_ptr<ResourcePolicyCallbackInterface> mResourcePolicyCallback | ||
@@ -59,6 +62,7 @@ private: | ||
static void BinderDiedCallback(void* cookie); | ||
|
||
void registerSelf(); | ||
+ // must delete the associated TranscodingResourcePolicyCookie any time this is called | ||
void unregisterSelf(); | ||
void onResourceAvailable(pid_t pid); | ||
}; // class TranscodingUidPolicy | ||
-- | ||
2.45.2.505.gda0bf45e8d-goog | ||
|
41 changes: 41 additions & 0 deletions
41
...reliminary/frameworks/av/0032-StagefrightRecoder-Disabling-B-frame-support.bulletin.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
From 999f881dd3fb1c6396299cb65b9817fffe23e2dd Mon Sep 17 00:00:00 2001 | ||
From: Rakesh Kumar <[email protected]> | ||
Date: Thu, 30 May 2024 11:17:48 +0000 | ||
Subject: [PATCH] StagefrightRecoder: Disabling B-frame support | ||
|
||
Disabling b-frame support from stagefright recorder in case of | ||
audio source as mic and video source is surface use case only | ||
because screen recorder with microphone doesn't play in sync | ||
if b-frame is enabled. | ||
If the audio source selected is INTERNAL (i.e. device) or | ||
MIC_AND_INTERNAL with screen recorder then b frame is supported. | ||
|
||
Bug: 288549440 | ||
Test: manually check screen recording with audio from mic has audio/video in synch | ||
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:af685c66bab17b71fe1624f76b5d55628f79e6fa) | ||
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:da3407f7688f35eb2dce79f1405feeb182241a3c) | ||
Merged-In: I4098655eb9687fb633085333bc140634441566e6 | ||
Change-Id: I4098655eb9687fb633085333bc140634441566e6 | ||
--- | ||
media/libmediaplayerservice/StagefrightRecorder.cpp | 5 +++++ | ||
1 file changed, 5 insertions(+) | ||
|
||
diff --git a/media/libmediaplayerservice/StagefrightRecorder.cpp b/media/libmediaplayerservice/StagefrightRecorder.cpp | ||
index ea1fdf4bf8..e2e77ee367 100644 | ||
--- a/media/libmediaplayerservice/StagefrightRecorder.cpp | ||
+++ b/media/libmediaplayerservice/StagefrightRecorder.cpp | ||
@@ -2075,6 +2075,11 @@ status_t StagefrightRecorder::setupVideoEncoder( | ||
|
||
if (tsLayers > 1) { | ||
uint32_t bLayers = std::min(2u, tsLayers - 1); // use up-to 2 B-layers | ||
+ // TODO(b/341121900): Remove this once B frames are handled correctly in screen recorder | ||
+ // use case in case of mic only | ||
+ if (mAudioSource == AUDIO_SOURCE_MIC && mVideoSource == VIDEO_SOURCE_SURFACE) { | ||
+ bLayers = 0; | ||
+ } | ||
uint32_t pLayers = tsLayers - bLayers; | ||
format->setString( | ||
"ts-schema", AStringPrintf("android.generic.%u+%u", pLayers, bLayers)); | ||
-- | ||
2.45.2.505.gda0bf45e8d-goog | ||
|
34 changes: 34 additions & 0 deletions
34
...ary/frameworks/base/99_0200-Fix-error-handling-for-non-dynamic-permissions.bulletin.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
From a7245613009e19d83a43c2bad50beba58145c920 Mon Sep 17 00:00:00 2001 | ||
From: Yi-an Chen <[email protected]> | ||
Date: Wed, 21 Feb 2024 01:56:22 +0000 | ||
Subject: [PATCH] Fix error handling for non-dynamic permissions | ||
|
||
We only allow removing dynamic permissions. When removePermission() is | ||
called for a non-dynamic permission, in addition to logging it, we | ||
should also return early to avoid the removePermission() call. | ||
|
||
Test: manual | ||
Bug: 321555066 | ||
Fixes: 321711213 | ||
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:528a87e90ff9354581d54fd37fbe9f95cccbcdb1) | ||
Merged-In: Ie2f43663bc71a06ffadb868d2d0eea5ee78f76e5 | ||
Change-Id: Ie2f43663bc71a06ffadb868d2d0eea5ee78f76e5 | ||
--- | ||
.../server/pm/permission/PermissionManagerServiceImpl.java | 1 + | ||
1 file changed, 1 insertion(+) | ||
|
||
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java | ||
index 5eff14f24f77..f2834fd8b997 100644 | ||
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java | ||
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java | ||
@@ -678,6 +678,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt | ||
// TODO: switch this back to SecurityException | ||
Slog.wtf(TAG, "Not allowed to modify non-dynamic permission " | ||
+ permName); | ||
+ return; | ||
} | ||
mRegistry.removePermission(permName); | ||
} | ||
-- | ||
2.45.2.505.gda0bf45e8d-goog | ||
|
40 changes: 40 additions & 0 deletions
40
.../base/99_0201-Fix-security-vulnerability-of-non-dynamic-permission-removal.bulletin.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
From 10aaf9ae5b6e30d20de62ecaa6df5b790f1b13bc Mon Sep 17 00:00:00 2001 | ||
From: Yi-an Chen <[email protected]> | ||
Date: Tue, 23 Apr 2024 21:17:44 +0000 | ||
Subject: [PATCH] Fix security vulnerability of non-dynamic permission removal | ||
|
||
The original removePermission() code in PermissionManagerServiceImpl | ||
missed a logical negation operator when handling non-dynamic | ||
permissions, causing both | ||
testPermissionPermission_nonDynamicPermission_permissionUnchanged and | ||
testRemovePermission_dynamicPermission_permissionRemoved tests in | ||
DynamicPermissionsTest to fail. | ||
|
||
The corresponding test DynamicPermissionsTest is also updated in the | ||
other CL: ag/27073864 | ||
|
||
Bug: 321711213 | ||
Test: DynamicPermissionsTest on sc-dev and tm-dev locally | ||
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0ead58f69f5de82b00406316b333366d556239f1) | ||
Merged-In: Ia146d4098643d9c473f8c83d33a8a125a53101fc | ||
Change-Id: Ia146d4098643d9c473f8c83d33a8a125a53101fc | ||
--- | ||
.../server/pm/permission/PermissionManagerServiceImpl.java | 2 +- | ||
1 file changed, 1 insertion(+), 1 deletion(-) | ||
|
||
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java | ||
index f2834fd8b997..f43f5f6b3935 100644 | ||
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java | ||
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java | ||
@@ -674,7 +674,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt | ||
if (bp == null) { | ||
return; | ||
} | ||
- if (bp.isDynamic()) { | ||
+ if (!bp.isDynamic()) { | ||
// TODO: switch this back to SecurityException | ||
Slog.wtf(TAG, "Not allowed to modify non-dynamic permission " | ||
+ permName); | ||
-- | ||
2.45.2.505.gda0bf45e8d-goog | ||
|
Oops, something went wrong.