-
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-Aug 2024 Security Patches integration
Integrating Google Android Security Bulletin Patches Test done: STS r29 TCs Passed Tracked-On: OAM-122533 Signed-off-by: Alam, Sahibex <[email protected]>
- Loading branch information
Showing
12 changed files
with
1,027 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
...av/41_0041-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 07df1d8dd95fb6af803f6f44905f2d473bdf7efa 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
...se_aaos/frameworks/av/42_0042-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 73d11ac20f91aff263e45555a85643884d656714 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 bffd7b3243..209b1f8036 100644 | ||
--- a/media/libmediaplayerservice/StagefrightRecorder.cpp | ||
+++ b/media/libmediaplayerservice/StagefrightRecorder.cpp | ||
@@ -2070,6 +2070,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
...aos/frameworks/base/99_0286-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 57ba37a9fc7f118d97d2e4ac7efc5a41d04f2501 Mon Sep 17 00:00:00 2001 | ||
From: Yi-an Chen <[email protected]> | ||
Date: Tue, 20 Feb 2024 04:34:57 +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:2b5d63b64b2b8208ccc4f62eac3d8962f981dbf8) | ||
Merged-In: I7336f2fc78804f26e4b2a329870ecdea776595d8 | ||
Change-Id: I7336f2fc78804f26e4b2a329870ecdea776595d8 | ||
--- | ||
.../android/server/pm/permission/PermissionManagerService.java | 1 + | ||
1 file changed, 1 insertion(+) | ||
|
||
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java | ||
index dd8b96eab3d7..31babe0418b8 100644 | ||
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java | ||
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java | ||
@@ -691,6 +691,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { | ||
// 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_0287-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 58a0ab5bc7abaa174187a1548982fd74ff18f00d Mon Sep 17 00:00:00 2001 | ||
From: Yi-an Chen <[email protected]> | ||
Date: Tue, 23 Apr 2024 21:53:02 +0000 | ||
Subject: [PATCH] Fix security vulnerability of non-dynamic permission removal | ||
|
||
The original removePermission() code in PermissionManagerService | ||
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:35d77a77feef62dc108f6478cb9228cc6044f70d) | ||
Merged-In: Id573b75cdcfce3a1df5731ffb00c4228c513e686 | ||
Change-Id: Id573b75cdcfce3a1df5731ffb00c4228c513e686 | ||
--- | ||
.../android/server/pm/permission/PermissionManagerService.java | 2 +- | ||
1 file changed, 1 insertion(+), 1 deletion(-) | ||
|
||
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java | ||
index 31babe0418b8..93f9e1c2295c 100644 | ||
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java | ||
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java | ||
@@ -687,7 +687,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { | ||
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.