From 53243faf690a49e00952b3d3956d2fff0b8d4a3c Mon Sep 17 00:00:00 2001 From: Shruti Bihani Date: Mon, 10 Jul 2023 08:53:42 +0000 Subject: [PATCH 01/12] Fix for heap buffer overflow issue flagged by fuzzer test. OOB write occurs when a value is assigned to a buffer index which is greater than the buffer size. Adding a check on buffer bounds fixes the issue. Similar checks have been added wherever applicable on other such methods of the class. Bug: 243463593 Test: Build mtp_packet_fuzzer and run on the target device (cherry picked from commit a669e34bb8e6f0f7b5d7a35144bd342271a24712) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0c26c2f4595b58bd7a11512227eacd480c4ddcd9) Merged-In: Icd0f2307803a1a35e655bc08d9d4cca5e2b58a9b Change-Id: Icd0f2307803a1a35e655bc08d9d4cca5e2b58a9b --- media/mtp/MtpPacket.cpp | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/media/mtp/MtpPacket.cpp b/media/mtp/MtpPacket.cpp index f069a83b5f..5faaac2026 100644 --- a/media/mtp/MtpPacket.cpp +++ b/media/mtp/MtpPacket.cpp @@ -92,24 +92,46 @@ void MtpPacket::copyFrom(const MtpPacket& src) { } uint16_t MtpPacket::getUInt16(int offset) const { - return ((uint16_t)mBuffer[offset + 1] << 8) | (uint16_t)mBuffer[offset]; + if ((unsigned long)(offset+2) <= mBufferSize) { + return ((uint16_t)mBuffer[offset + 1] << 8) | (uint16_t)mBuffer[offset]; + } + else { + ALOGE("offset for buffer read is greater than buffer size!"); + abort(); + } } uint32_t MtpPacket::getUInt32(int offset) const { - return ((uint32_t)mBuffer[offset + 3] << 24) | ((uint32_t)mBuffer[offset + 2] << 16) | - ((uint32_t)mBuffer[offset + 1] << 8) | (uint32_t)mBuffer[offset]; + if ((unsigned long)(offset+4) <= mBufferSize) { + return ((uint32_t)mBuffer[offset + 3] << 24) | ((uint32_t)mBuffer[offset + 2] << 16) | + ((uint32_t)mBuffer[offset + 1] << 8) | (uint32_t)mBuffer[offset]; + } + else { + ALOGE("offset for buffer read is greater than buffer size!"); + abort(); + } } void MtpPacket::putUInt16(int offset, uint16_t value) { - mBuffer[offset++] = (uint8_t)(value & 0xFF); - mBuffer[offset++] = (uint8_t)((value >> 8) & 0xFF); + if ((unsigned long)(offset+2) <= mBufferSize) { + mBuffer[offset++] = (uint8_t)(value & 0xFF); + mBuffer[offset++] = (uint8_t)((value >> 8) & 0xFF); + } + else { + ALOGE("offset for buffer write is greater than buffer size!"); + } } void MtpPacket::putUInt32(int offset, uint32_t value) { - mBuffer[offset++] = (uint8_t)(value & 0xFF); - mBuffer[offset++] = (uint8_t)((value >> 8) & 0xFF); - mBuffer[offset++] = (uint8_t)((value >> 16) & 0xFF); - mBuffer[offset++] = (uint8_t)((value >> 24) & 0xFF); + if ((unsigned long)(offset+4) <= mBufferSize) { + mBuffer[offset++] = (uint8_t)(value & 0xFF); + mBuffer[offset++] = (uint8_t)((value >> 8) & 0xFF); + mBuffer[offset++] = (uint8_t)((value >> 16) & 0xFF); + mBuffer[offset++] = (uint8_t)((value >> 24) & 0xFF); + } + else { + ALOGE("offset for buffer write is greater than buffer size!"); + } } uint16_t MtpPacket::getContainerCode() const { -- GitLab From 2fdf54b050f728fd965c9afdd03116e9b9dafbae Mon Sep 17 00:00:00 2001 From: Shruti Bihani Date: Thu, 13 Jul 2023 09:19:08 +0000 Subject: [PATCH 02/12] Fix heap-use-after-free issue flagged by fuzzer test. A data member of class MtpFfsHandle is being accessed after the class object has been freed in the fuzzer. The method accessing the data member is running in a separate thread that gets detached from its parent. Using a conditional variable with an atomic int predicate in the close() function to ensure the detached thread's execution has completed before freeing the object fixes the issue without blocking the processing mid-way. Bug: 243381410 Test: Build mtp_handle_fuzzer and run on the target device (cherry picked from commit 50bf46a3f62136386548a9187a749936bda3ee8f) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:05dc1c083095ebee0faa20498153eb466082ace0) Merged-In: I41dde165a5eba151c958b81417d9e1065af1b411 Change-Id: I41dde165a5eba151c958b81417d9e1065af1b411 --- media/mtp/MtpFfsHandle.cpp | 14 ++++++++++++++ media/mtp/MtpFfsHandle.h | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/media/mtp/MtpFfsHandle.cpp b/media/mtp/MtpFfsHandle.cpp index 2ffd7759e0..ef8c9aa789 100644 --- a/media/mtp/MtpFfsHandle.cpp +++ b/media/mtp/MtpFfsHandle.cpp @@ -297,6 +297,10 @@ int MtpFfsHandle::start(bool ptp) { } void MtpFfsHandle::close() { + auto timeout = std::chrono::seconds(2); + std::unique_lock lk(m); + cv.wait_for(lk, timeout ,[this]{return child_threads==0;}); + io_destroy(mCtx); closeEndpoints(); closeConfig(); @@ -669,6 +673,11 @@ int MtpFfsHandle::sendEvent(mtp_event me) { char *temp = new char[me.length]; memcpy(temp, me.data, me.length); me.data = temp; + + std::unique_lock lk(m); + child_threads++; + lk.unlock(); + std::thread t([this, me]() { return this->doSendEvent(me); }); t.detach(); return 0; @@ -680,6 +689,11 @@ void MtpFfsHandle::doSendEvent(mtp_event me) { if (static_cast(ret) != length) PLOG(ERROR) << "Mtp error sending event thread!"; delete[] reinterpret_cast(me.data); + + std::unique_lock lk(m); + child_threads--; + lk.unlock(); + cv.notify_one(); } } // namespace android diff --git a/media/mtp/MtpFfsHandle.h b/media/mtp/MtpFfsHandle.h index e552e03bec..51cdef0953 100644 --- a/media/mtp/MtpFfsHandle.h +++ b/media/mtp/MtpFfsHandle.h @@ -60,6 +60,10 @@ protected: bool mCanceled; bool mBatchCancel; + std::mutex m; + std::condition_variable cv; + std::atomic child_threads{0}; + android::base::unique_fd mControl; // "in" from the host's perspective => sink for mtp server android::base::unique_fd mBulkIn; -- GitLab From acb81624b4f50fed52cb1b3829809ee2f7377093 Mon Sep 17 00:00:00 2001 From: Venkatarama Avadhani Date: Fri, 11 Aug 2023 15:19:24 +0000 Subject: [PATCH 03/12] Initialise VPS buffer to NULL in constructor Missing initialisation of this pointer could lead to an incorrect free if the ARTWriter object is cleared immeddiately after the constructor call. Bug: 287298721 Test: rtp_writer_fuzzer (cherry picked from https://partner-android-review.googlesource.com/q/commit:2710696b001f2e95586151c1ee337a4e3c4da48a) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:900195c1d3589c7cbf9e116f61bebaefc0519101) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0efe2b4d6b739650039c2cab176ef11d5f5ac49c) Merged-In: I08eacd7a0201bc9a41b821e20cae916d8870147a Change-Id: I08eacd7a0201bc9a41b821e20cae916d8870147a --- media/libstagefright/rtsp/ARTPWriter.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/media/libstagefright/rtsp/ARTPWriter.cpp b/media/libstagefright/rtsp/ARTPWriter.cpp index 8990f0cf7c..e20d70235b 100644 --- a/media/libstagefright/rtsp/ARTPWriter.cpp +++ b/media/libstagefright/rtsp/ARTPWriter.cpp @@ -105,6 +105,7 @@ ARTPWriter::ARTPWriter(int fd) mRTCPAddr = mRTPAddr; mRTCPAddr.sin_port = htons(ntohs(mRTPAddr.sin_port) | 1); + mVPSBuf = NULL; mSPSBuf = NULL; mPPSBuf = NULL; -- GitLab From 58fd993a89a3a22fa5a4a1a4548125c6783ec80c Mon Sep 17 00:00:00 2001 From: Toni Heidenreich Date: Wed, 6 Sep 2023 12:49:33 +0000 Subject: [PATCH 04/12] httplive: fix use-after-free Implement a mutex to ensure secure multi-threaded access to the KeyedVector in MetaDataBase. Concurrent access by different threads can lead to accessing the wrong memory location due to potential changes in the vector Bug: 298057702 Test: HTTP Live Streaming test (cherry picked from https://partner-android-review.googlesource.com/q/commit:a2dfb31957a9d5358d0219a0eda7dcb5b0fff5fe) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:90fb4ca425444429ada6ce0de1c13d35829bc196) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3c1d9613ef64e01d2e81c4aa44c90dcd8ca958b9) Merged-In: I46b05c85d9c39f4ce549efc160c08a0646c9fd0a Change-Id: I46b05c85d9c39f4ce549efc160c08a0646c9fd0a --- media/libstagefright/foundation/MetaDataBase.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/media/libstagefright/foundation/MetaDataBase.cpp b/media/libstagefright/foundation/MetaDataBase.cpp index 33707482c9..46a600a9d9 100644 --- a/media/libstagefright/foundation/MetaDataBase.cpp +++ b/media/libstagefright/foundation/MetaDataBase.cpp @@ -23,6 +23,8 @@ #include #include +#include + #include #include #include @@ -78,6 +80,7 @@ struct MetaDataBase::Rect { struct MetaDataBase::MetaDataInternal { + std::mutex mLock; KeyedVector mItems; }; @@ -102,10 +105,12 @@ MetaDataBase::~MetaDataBase() { } void MetaDataBase::clear() { + std::lock_guard guard(mInternalData->mLock); mInternalData->mItems.clear(); } bool MetaDataBase::remove(uint32_t key) { + std::lock_guard guard(mInternalData->mLock); ssize_t i = mInternalData->mItems.indexOfKey(key); if (i < 0) { @@ -252,6 +257,7 @@ bool MetaDataBase::setData( uint32_t key, uint32_t type, const void *data, size_t size) { bool overwrote_existing = true; + std::lock_guard guard(mInternalData->mLock); ssize_t i = mInternalData->mItems.indexOfKey(key); if (i < 0) { typed_data item; @@ -269,6 +275,7 @@ bool MetaDataBase::setData( bool MetaDataBase::findData(uint32_t key, uint32_t *type, const void **data, size_t *size) const { + std::lock_guard guard(mInternalData->mLock); ssize_t i = mInternalData->mItems.indexOfKey(key); if (i < 0) { @@ -283,6 +290,7 @@ bool MetaDataBase::findData(uint32_t key, uint32_t *type, } bool MetaDataBase::hasData(uint32_t key) const { + std::lock_guard guard(mInternalData->mLock); ssize_t i = mInternalData->mItems.indexOfKey(key); if (i < 0) { @@ -429,6 +437,7 @@ static void MakeFourCCString(uint32_t x, char *s) { String8 MetaDataBase::toString() const { String8 s; + std::lock_guard guard(mInternalData->mLock); for (int i = mInternalData->mItems.size(); --i >= 0;) { int32_t key = mInternalData->mItems.keyAt(i); char cc[5]; @@ -443,6 +452,7 @@ String8 MetaDataBase::toString() const { } void MetaDataBase::dumpToLog() const { + std::lock_guard guard(mInternalData->mLock); for (int i = mInternalData->mItems.size(); --i >= 0;) { int32_t key = mInternalData->mItems.keyAt(i); char cc[5]; @@ -455,6 +465,7 @@ void MetaDataBase::dumpToLog() const { #if defined(__ANDROID__) && !defined(__ANDROID_VNDK__) && !defined(__ANDROID_APEX__) status_t MetaDataBase::writeToParcel(Parcel &parcel) { status_t ret; + std::lock_guard guard(mInternalData->mLock); size_t numItems = mInternalData->mItems.size(); ret = parcel.writeUint32(uint32_t(numItems)); if (ret) { -- GitLab From 148aeea373febc959c429f2cabd8323508c38ad8 Mon Sep 17 00:00:00 2001 From: Atneya Nair Date: Wed, 10 May 2023 21:37:41 -0700 Subject: [PATCH 05/12] Correct attribution source for MMAP thread Ensure that the package name, which is used for listening for appops below getInputForAttr, is corrected for MMAP threads. Bug: 268724205 Test: AudioRecordTest Test: Oboetester MMAP record silenced when backgrounded - 6s (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f59db5cb1be38abce4c3c4f553090e527a6d4513) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0230540dbcefd8c9d0e73a423ad95f3ad379c3a0) Merged-In: Ia6fc1bff815bbbb2fee8bc1a60569a663a713e4b Change-Id: Ia6fc1bff815bbbb2fee8bc1a60569a663a713e4b --- services/audioflinger/Threads.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index 683e320073..964f4d0036 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -9509,6 +9509,9 @@ status_t AudioFlinger::MmapThread::start(const AudioClient& client, audio_port_handle_t portId = AUDIO_PORT_HANDLE_NONE; audio_io_handle_t io = mId; + AttributionSourceState adjAttributionSource = AudioFlinger::checkAttributionSourcePackage( + client.attributionSource); + if (isOutput()) { audio_config_t config = AUDIO_CONFIG_INITIALIZER; config.sample_rate = mSampleRate; @@ -9523,7 +9526,7 @@ status_t AudioFlinger::MmapThread::start(const AudioClient& client, ret = AudioSystem::getOutputForAttr(&mAttr, &io, mSessionId, &stream, - client.attributionSource, + adjAttributionSource, &config, flags, &deviceId, @@ -9541,7 +9544,7 @@ status_t AudioFlinger::MmapThread::start(const AudioClient& client, ret = AudioSystem::getInputForAttr(&mAttr, &io, RECORD_RIID_INVALID, mSessionId, - client.attributionSource, + adjAttributionSource, &config, AUDIO_INPUT_FLAG_MMAP_NOIRQ, &deviceId, -- GitLab From 5f401fc9f214789d691798620fea60015962370a Mon Sep 17 00:00:00 2001 From: Atneya Nair Date: Wed, 9 Aug 2023 16:21:48 -0700 Subject: [PATCH 06/12] Condition background record restriction on Sdk To prevent breaking existing apps, modify the checks around when an app should have its recording silenced to retain prior behavior unless an app has targetSdk U or greater. Test: oboetester conditionally restricted based on targetSdk level Bug: 268724205 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:dc4f375d570965775634d90856719b812aee9865) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3de585db623429900c684c12ad4ac17fb78979b0) Merged-In: I42b6cbca60db6ce1a073254239b48e9104c4ebfb Change-Id: I42b6cbca60db6ce1a073254239b48e9104c4ebfb --- .../service/AudioPolicyService.cpp | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/services/audiopolicy/service/AudioPolicyService.cpp b/services/audiopolicy/service/AudioPolicyService.cpp index e7d945fad6..926d7f013d 100644 --- a/services/audiopolicy/service/AudioPolicyService.cpp +++ b/services/audiopolicy/service/AudioPolicyService.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -192,6 +193,27 @@ static void destroyAudioPolicyManager(AudioPolicyInterface *interface) { delete interface; } + +namespace { +int getTargetSdkForPackageName(std::string_view packageName) { + const auto binder = defaultServiceManager()->checkService(String16{"package_native"}); + int targetSdk = -1; + if (binder != nullptr) { + const auto pm = interface_cast(binder); + if (pm != nullptr) { + const auto status = pm->getTargetSdkVersionForPackage( + String16{packageName.data(), packageName.size()}, &targetSdk); + return status.isOk() ? targetSdk : -1; + } + } + return targetSdk; +} + +bool doesPackageTargetAtLeastU(std::string_view packageName) { + constexpr int ANDROID_API_U = 34; + return getTargetSdkForPackageName(packageName) >= ANDROID_API_U; +} +} // anonymous // ---------------------------------------------------------------------------- AudioPolicyService::AudioPolicyService() @@ -1873,10 +1895,14 @@ void AudioPolicyService::OpRecordAudioMonitor::onFirstRef() checkOp(); mOpCallback = new RecordAudioOpCallback(this); ALOGV("start watching op %d for %s", mAppOp, mAttributionSource.toString().c_str()); + int flags = doesPackageTargetAtLeastU( + mAttributionSource.packageName.value_or("")) ? + AppOpsManager::WATCH_FOREGROUND_CHANGES : 0; // TODO: We need to always watch AppOpsManager::OP_RECORD_AUDIO too // since it controls the mic permission for legacy apps. mAppOpsManager.startWatchingMode(mAppOp, VALUE_OR_FATAL(aidl2legacy_string_view_String16( mAttributionSource.packageName.value_or(""))), + flags, mOpCallback); } -- GitLab From 099906c5eb69f9e1e2ab2ebf1ad48ae4bcfb1a50 Mon Sep 17 00:00:00 2001 From: guochuang Date: Fri, 15 Sep 2023 17:45:44 +0800 Subject: [PATCH 07/12] NdkMedia: fix android.mediav2.cts.CodecEncoderSurfaceTest failed. use Abuffer.base instead of data. Bug: 300861053 Test: run cts -m CtsMediaV2TestCases -t android.mediav2.cts.CodecEncoderSurfaceTest Signed-off-by: guochuang (cherry picked from https://android-review.googlesource.com/q/commit:184293b550dec1bcb7b8ba5f2f5342c355f5b4dc) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:95f411fbda480f986811e6f8010f0a2fd91fd206) Merged-In: I76ef2c404f8264843973e1af9be1b23fc1e6c3af Change-Id: I76ef2c404f8264843973e1af9be1b23fc1e6c3af --- media/ndk/NdkMediaCodec.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/media/ndk/NdkMediaCodec.cpp b/media/ndk/NdkMediaCodec.cpp index 38e422df3c..1c655f9c73 100644 --- a/media/ndk/NdkMediaCodec.cpp +++ b/media/ndk/NdkMediaCodec.cpp @@ -626,7 +626,7 @@ uint8_t* AMediaCodec_getInputBuffer(AMediaCodec *mData, size_t idx, size_t *out_ if (out_size != NULL) { *out_size = abuf->capacity(); } - return abuf->data(); + return abuf->base(); } android::Vector > abufs; @@ -643,7 +643,7 @@ uint8_t* AMediaCodec_getInputBuffer(AMediaCodec *mData, size_t idx, size_t *out_ if (out_size != NULL) { *out_size = abufs[idx]->capacity(); } - return abufs[idx]->data(); + return abufs[idx]->base(); } ALOGE("couldn't get input buffers"); return NULL; @@ -661,7 +661,7 @@ uint8_t* AMediaCodec_getOutputBuffer(AMediaCodec *mData, size_t idx, size_t *out if (out_size != NULL) { *out_size = abuf->capacity(); } - return abuf->data(); + return abuf->base(); } android::Vector > abufs; @@ -674,7 +674,7 @@ uint8_t* AMediaCodec_getOutputBuffer(AMediaCodec *mData, size_t idx, size_t *out if (out_size != NULL) { *out_size = abufs[idx]->capacity(); } - return abufs[idx]->data(); + return abufs[idx]->base(); } ALOGE("couldn't get output buffers"); return NULL; -- GitLab From 3cc4024ac751c154ffb59f9fc86bfd10d728a20e Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Thu, 19 Oct 2023 18:39:26 +0000 Subject: [PATCH 08/12] Revert "NdkMedia: fix android.mediav2.cts.CodecEncoderSurfaceTest failed." This reverts commit 95f411fbda480f986811e6f8010f0a2fd91fd206. Reason for revert: This breaks app compatibility (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:883e38b694a6a2a51d34ea145607b62b0bdb316f) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a2118fa4988f1d8135894800de122296e146a157) Merged-In: I94d7034907d5befc0aabf7bf2b490d1fc86c69af Change-Id: I94d7034907d5befc0aabf7bf2b490d1fc86c69af --- media/ndk/NdkMediaCodec.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/media/ndk/NdkMediaCodec.cpp b/media/ndk/NdkMediaCodec.cpp index 1c655f9c73..38e422df3c 100644 --- a/media/ndk/NdkMediaCodec.cpp +++ b/media/ndk/NdkMediaCodec.cpp @@ -626,7 +626,7 @@ uint8_t* AMediaCodec_getInputBuffer(AMediaCodec *mData, size_t idx, size_t *out_ if (out_size != NULL) { *out_size = abuf->capacity(); } - return abuf->base(); + return abuf->data(); } android::Vector > abufs; @@ -643,7 +643,7 @@ uint8_t* AMediaCodec_getInputBuffer(AMediaCodec *mData, size_t idx, size_t *out_ if (out_size != NULL) { *out_size = abufs[idx]->capacity(); } - return abufs[idx]->base(); + return abufs[idx]->data(); } ALOGE("couldn't get input buffers"); return NULL; @@ -661,7 +661,7 @@ uint8_t* AMediaCodec_getOutputBuffer(AMediaCodec *mData, size_t idx, size_t *out if (out_size != NULL) { *out_size = abuf->capacity(); } - return abuf->base(); + return abuf->data(); } android::Vector > abufs; @@ -674,7 +674,7 @@ uint8_t* AMediaCodec_getOutputBuffer(AMediaCodec *mData, size_t idx, size_t *out if (out_size != NULL) { *out_size = abufs[idx]->capacity(); } - return abufs[idx]->base(); + return abufs[idx]->data(); } ALOGE("couldn't get output buffers"); return NULL; -- GitLab From 30b1b34cfd5abfcfee759e7d13167d368ac6c268 Mon Sep 17 00:00:00 2001 From: Harish Mahendrakar Date: Mon, 28 Aug 2023 17:35:56 +0000 Subject: [PATCH 09/12] Codec2BufferUtils: Use cropped dimensions in RGB to YUV conversion Bug: 283099444 Test: poc in the bug (cherry picked from https://partner-android-review.googlesource.com/q/commit:3875b858a347e25db94574e6362798a849bf9ebd) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:4eba80f6698cb2d7aa48ea4f7728dbdf11f29fd3) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:aaddf2a49d758a55319b88c8331d5b1209858c41) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3ee0378ac5b39fe57fb91f0a8113e0fd18ec1822) Merged-In: I42c71616c9d50f61c92f461f6a91f5addb1d724a Change-Id: I42c71616c9d50f61c92f461f6a91f5addb1d724a --- media/codec2/sfplugin/utils/Codec2BufferUtils.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/media/codec2/sfplugin/utils/Codec2BufferUtils.cpp b/media/codec2/sfplugin/utils/Codec2BufferUtils.cpp index 807841e4c6..4f21990ef0 100644 --- a/media/codec2/sfplugin/utils/Codec2BufferUtils.cpp +++ b/media/codec2/sfplugin/utils/Codec2BufferUtils.cpp @@ -581,8 +581,8 @@ status_t ConvertRGBToPlanarYUV( uint8_t maxLvlChroma = colorRange == C2Color::RANGE_FULL ? 255 : 240; #define CLIP3(min,v,max) (((v) < (min)) ? (min) : (((max) > (v)) ? (v) : (max))) - for (size_t y = 0; y < src.height(); ++y) { - for (size_t x = 0; x < src.width(); ++x) { + for (size_t y = 0; y < src.crop().height; ++y) { + for (size_t x = 0; x < src.crop().width; ++x) { uint8_t r = *pRed; uint8_t g = *pGreen; uint8_t b = *pBlue; -- GitLab From bf6406041919f67219fd1829438dda28845d4c23 Mon Sep 17 00:00:00 2001 From: Songyue Han Date: Tue, 3 Oct 2023 22:40:14 +0000 Subject: [PATCH 10/12] Fix convertYUV420Planar16ToY410 overflow issue for unsupported cropwidth. Bug: 300476626 Test: color_conversion_fuzzer (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:de2ad0fad97d6d97d1e01f0e8d8309536eb268b4) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:745ab99f7343bc236b88b9d63cd7b06ab192f9e9) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:aa8298ec8eb903e1e3dd915fa24f32e1aea1f76c) Merged-In: I8631426188af3c5f9b6c1ff6a0039254c252f733 Change-Id: I8631426188af3c5f9b6c1ff6a0039254c252f733 --- media/libstagefright/colorconversion/ColorConverter.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/media/libstagefright/colorconversion/ColorConverter.cpp b/media/libstagefright/colorconversion/ColorConverter.cpp index 4b4f65f13c..6eae6c71cf 100644 --- a/media/libstagefright/colorconversion/ColorConverter.cpp +++ b/media/libstagefright/colorconversion/ColorConverter.cpp @@ -949,7 +949,8 @@ status_t ColorConverter::convertYUV420Planar16ToY410( uint32_t u01, v01, y01, y23, y45, y67, uv0, uv1; size_t x = 0; - for (; x < src.cropWidth() - 3; x += 4) { + // x % 4 is always 0 so x + 3 will never overflow. + for (; x + 3 < src.cropWidth(); x += 4) { u01 = *((uint32_t*)ptr_u); ptr_u += 2; v01 = *((uint32_t*)ptr_v); ptr_v += 2; -- GitLab From 3f6e2f600ff8700161a283a7e4825628949f484a Mon Sep 17 00:00:00 2001 From: Ashish Kumar Gupta Date: Tue, 21 Nov 2023 08:48:43 +0530 Subject: [PATCH 11/12] Update mtp packet buffer Currently, the buffer size is not changed when the packet size is increased. Ideally, the buffer size should be larger than the packet size. In our case, when the packet size is increased, we must reallocate the buffer of MTP packet. Bug: 300007708 Test: build and flash the device. Check MTP works Test: run fuzzer locally (cherry picked from commit e1494a2d8e7eee25d7ea5469be43740e97294c99) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5c0f99beb6fa5ff920caf5b0d06aaebc8e9eab24) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:960d83c60805bd0991e02cd72224a4063097af89) Merged-In: I98398a9e15962e6d5f08445ee7b17f5d61a3a528 Change-Id: I98398a9e15962e6d5f08445ee7b17f5d61a3a528 --- media/mtp/MtpPacket.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/media/mtp/MtpPacket.cpp b/media/mtp/MtpPacket.cpp index 5faaac2026..634aa46dd9 100644 --- a/media/mtp/MtpPacket.cpp +++ b/media/mtp/MtpPacket.cpp @@ -168,8 +168,10 @@ void MtpPacket::setParameter(int index, uint32_t value) { return; } int offset = MTP_CONTAINER_PARAMETER_OFFSET + (index - 1) * sizeof(uint32_t); - if (mPacketSize < offset + sizeof(uint32_t)) + if (mPacketSize < offset + sizeof(uint32_t)) { mPacketSize = offset + sizeof(uint32_t); + allocate(mPacketSize); + } putUInt32(offset, value); } -- GitLab From c74dad300cea00693e7f55a61c44b2938ecf2879 Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Fri, 13 Oct 2023 18:13:43 +0200 Subject: [PATCH 12/12] Audio policy: anonymize Bluetooth MAC addresses Make sure APIs returning audio device descriptors from the native audioserver anonymize the Bluetooth MAC addresses because those are considered privacy sensitive. Only expose the full MAC address to system and apps with BLUETOOTH_CONNECT permission. APIs modified: listAudioPorts, listAudioPatches, getAudioPort APIs that can only be called from system server or only convey port IDs are not modified. Bug: 285588444 Test: atest AudioManagerTest Test: atest RoutingTest Test: atest AudioCommunicationDeviceTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:8682ced79a56ed01874b09881b41359606815dfa) Merged-In: Ia6bac184f5f39ed9d538f762ebb89bcceb44ae50 Change-Id: Ia6bac184f5f39ed9d538f762ebb89bcceb44ae50 --- .../audio_aidl_legacy_conversion_tests.cpp | 1 + media/utils/ServiceUtilities.cpp | 43 +++++++++++++++++ .../include/mediautils/ServiceUtilities.h | 4 ++ .../service/AudioPolicyInterfaceImpl.cpp | 46 +++++++++++++++++++ 4 files changed, 94 insertions(+) diff --git a/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp b/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp index 997f62a19a..307802b525 100644 --- a/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp +++ b/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp @@ -274,6 +274,7 @@ INSTANTIATE_TEST_SUITE_P(AudioDeviceDescriptionRoundTrip, class AudioFormatDescriptionRoundTripTest : public testing::TestWithParam {}; + TEST_P(AudioFormatDescriptionRoundTripTest, Aidl2Legacy2Aidl) { const auto initial = GetParam(); auto conv = aidl2legacy_AudioFormatDescription_audio_format_t(initial); diff --git a/media/utils/ServiceUtilities.cpp b/media/utils/ServiceUtilities.cpp index 83b84e392c..4ee45c795e 100644 --- a/media/utils/ServiceUtilities.cpp +++ b/media/utils/ServiceUtilities.cpp @@ -46,6 +46,7 @@ static const String16 sAndroidPermissionRecordAudio("android.permission.RECORD_A static const String16 sModifyPhoneState("android.permission.MODIFY_PHONE_STATE"); static const String16 sModifyAudioRouting("android.permission.MODIFY_AUDIO_ROUTING"); static const String16 sCallAudioInterception("android.permission.CALL_AUDIO_INTERCEPTION"); +static const String16 sAndroidPermissionBluetoothConnect("android.permission.BLUETOOTH_CONNECT"); static String16 resolveCallingPackage(PermissionController& permissionController, const std::optional opPackageName, uid_t uid) { @@ -374,6 +375,48 @@ status_t checkIMemory(const sp& iMemory) return NO_ERROR; } +/** + * Determines if the MAC address in Bluetooth device descriptors returned by APIs of + * a native audio service (audio flinger, audio policy) must be anonymized. + * MAC addresses returned to system server or apps with BLUETOOTH_CONNECT permission + * are not anonymized. + * + * @param attributionSource The attribution source of the calling app. + * @param caller string identifying the caller for logging. + * @return true if the MAC addresses must be anonymized, false otherwise. + */ +bool mustAnonymizeBluetoothAddress( + const AttributionSourceState& attributionSource, const String16& caller) { + uid_t uid = VALUE_OR_FATAL(aidl2legacy_int32_t_uid_t(attributionSource.uid)); + if (isAudioServerOrSystemServerUid(uid)) { + return false; + } + const std::optional resolvedAttributionSource = + resolveAttributionSource(attributionSource); + if (!resolvedAttributionSource.has_value()) { + return true; + } + permission::PermissionChecker permissionChecker; + return permissionChecker.checkPermissionForPreflightFromDatasource( + sAndroidPermissionBluetoothConnect, resolvedAttributionSource.value(), caller, + AppOpsManager::OP_BLUETOOTH_CONNECT) + != permission::PermissionChecker::PERMISSION_GRANTED; +} + +/** + * Modifies the passed MAC address string in place for consumption by unprivileged clients. + * the string is assumed to have a valid MAC address format. + * the anonymzation must be kept in sync with toAnonymizedAddress() in BluetoothUtils.java + * + * @param address input/output the char string contining the MAC address to anonymize. + */ +void anonymizeBluetoothAddress(char *address) { + if (address == nullptr || strlen(address) != strlen("AA:BB:CC:DD:EE:FF")) { + return; + } + memcpy(address, "XX:XX:XX:XX", strlen("XX:XX:XX:XX")); +} + sp MediaPackageManager::retrievePackageManager() { const sp sm = defaultServiceManager(); if (sm == nullptr) { diff --git a/media/utils/include/mediautils/ServiceUtilities.h b/media/utils/include/mediautils/ServiceUtilities.h index de20d559ef..46a70ddad5 100644 --- a/media/utils/include/mediautils/ServiceUtilities.h +++ b/media/utils/include/mediautils/ServiceUtilities.h @@ -108,6 +108,10 @@ bool modifyPhoneStateAllowed(const AttributionSourceState& attributionSource); bool bypassInterruptionPolicyAllowed(const AttributionSourceState& attributionSource); bool callAudioInterceptionAllowed(const AttributionSourceState& attributionSource); void purgePermissionCache(); +bool mustAnonymizeBluetoothAddress( + const AttributionSourceState& attributionSource, const String16& caller); +void anonymizeBluetoothAddress(char *address); + int32_t getOpForSource(audio_source_t source); AttributionSourceState getCallingAttributionSource(); diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp index 5dd9b8cb2f..5155a2764a 100644 --- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp +++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp @@ -1503,6 +1503,19 @@ Status AudioPolicyService::isDirectOutputSupported( return Status::ok(); } +template +void anonymizePortBluetoothAddress(Port *port) { + if (port->type != AUDIO_PORT_TYPE_DEVICE) { + return; + } + if (!(audio_is_a2dp_device(port->ext.device.type) + || audio_is_ble_device(port->ext.device.type) + || audio_is_bluetooth_sco_device(port->ext.device.type) + || audio_is_hearing_aid_out_device(port->ext.device.type))) { + return; + } + anonymizeBluetoothAddress(port->ext.device.address); +} Status AudioPolicyService::listAudioPorts(media::AudioPortRole roleAidl, media::AudioPortType typeAidl, Int* count, @@ -1525,10 +1538,20 @@ Status AudioPolicyService::listAudioPorts(media::AudioPortRole roleAidl, if (mAudioPolicyManager == NULL) { return binderStatusFromStatusT(NO_INIT); } + + const AttributionSourceState attributionSource = getCallingAttributionSource(); + AutoCallerClear acc; RETURN_IF_BINDER_ERROR(binderStatusFromStatusT( mAudioPolicyManager->listAudioPorts(role, type, &num_ports, ports.get(), &generation))); numPortsReq = std::min(numPortsReq, num_ports); + + if (mustAnonymizeBluetoothAddress(attributionSource, String16(__func__))) { + for (size_t i = 0; i < numPortsReq; ++i) { + anonymizePortBluetoothAddress(&ports[i]); + } + } + RETURN_IF_BINDER_ERROR(binderStatusFromStatusT( convertRange(ports.get(), ports.get() + numPortsReq, std::back_inserter(*portsAidl), legacy2aidl_audio_port_v7_AudioPort))); @@ -1544,8 +1567,16 @@ Status AudioPolicyService::getAudioPort(int portId, if (mAudioPolicyManager == NULL) { return binderStatusFromStatusT(NO_INIT); } + + const AttributionSourceState attributionSource = getCallingAttributionSource(); + AutoCallerClear acc; RETURN_IF_BINDER_ERROR(binderStatusFromStatusT(mAudioPolicyManager->getAudioPort(&port))); + + if (mustAnonymizeBluetoothAddress(attributionSource, String16(__func__))) { + anonymizePortBluetoothAddress(&port); + } + *_aidl_return = VALUE_OR_RETURN_BINDER_STATUS(legacy2aidl_audio_port_v7_AudioPort(port)); return Status::ok(); } @@ -1606,10 +1637,25 @@ Status AudioPolicyService::listAudioPatches(Int* count, if (mAudioPolicyManager == NULL) { return binderStatusFromStatusT(NO_INIT); } + + const AttributionSourceState attributionSource = getCallingAttributionSource(); + AutoCallerClear acc; RETURN_IF_BINDER_ERROR(binderStatusFromStatusT( mAudioPolicyManager->listAudioPatches(&num_patches, patches.get(), &generation))); numPatchesReq = std::min(numPatchesReq, num_patches); + + if (mustAnonymizeBluetoothAddress(attributionSource, String16(__func__))) { + for (size_t i = 0; i < numPatchesReq; ++i) { + for (size_t j = 0; j < patches[i].num_sources; ++j) { + anonymizePortBluetoothAddress(&patches[i].sources[j]); + } + for (size_t j = 0; j < patches[i].num_sinks; ++j) { + anonymizePortBluetoothAddress(&patches[i].sinks[j]); + } + } + } + RETURN_IF_BINDER_ERROR(binderStatusFromStatusT( convertRange(patches.get(), patches.get() + numPatchesReq, std::back_inserter(*patchesAidl), legacy2aidl_audio_patch_AudioPatch))); -- GitLab