From bc8a45f506a8be33250c523d71fab637a5fdaf81 Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Tue, 2 Aug 2016 07:07:05 -0700 Subject: IOMX: work against metadata buffer spoofing - Prohibit direct set/getParam/Settings for extensions meant for OMXNodeInstance alone. This disallows enabling metadata mode without the knowledge of OMXNodeInstance. - Do not share metadata mode buffers cross process. - Disallow setting up metadata mode/tunneling/input surface after first sendCommand. - Disallow store-meta for input cross process. - Disallow emptyBuffer for surface input (via IOMX). - Fix checking for input surface. Bug: 29422020 Change-Id: I801c77b80e703903f62e42d76fd2e76a34e4bc8e (cherry picked from commit f8a4cb410115045278f534e54b41ac78d6bf6c07) --- media/libmedia/IOMX.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index b082fe4..c28eac8 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -248,7 +248,7 @@ public: virtual status_t useBuffer( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL /* crossProcess */) { Parcel data, reply; data.writeInterfaceToken(IOMX::getInterfaceDescriptor()); data.writeInt32((int32_t)node); @@ -481,7 +481,7 @@ public: virtual status_t allocateBufferWithBackup( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL /* crossProcess */) { Parcel data, reply; data.writeInterfaceToken(IOMX::getInterfaceDescriptor()); data.writeInt32((int32_t)node); @@ -834,7 +834,8 @@ status_t BnOMX::onTransact( OMX_U32 allottedSize = data.readInt32(); buffer_id buffer; - status_t err = useBuffer(node, port_index, params, &buffer, allottedSize); + status_t err = useBuffer( + node, port_index, params, &buffer, allottedSize, OMX_TRUE /* crossProcess */); reply->writeInt32(err); if (err == OK) { @@ -969,7 +970,10 @@ status_t BnOMX::onTransact( OMX_BOOL enable = (OMX_BOOL)data.readInt32(); MetadataBufferType type = kMetadataBufferTypeInvalid; - status_t err = storeMetaDataInBuffers(node, port_index, enable, &type); + status_t err = + // only control output metadata via Binder + port_index != 1 /* kOutputPortIndex */ ? BAD_VALUE : + storeMetaDataInBuffers(node, port_index, enable, &type); reply->writeInt32(type); reply->writeInt32(err); @@ -1054,7 +1058,7 @@ status_t BnOMX::onTransact( buffer_id buffer; status_t err = allocateBufferWithBackup( - node, port_index, params, &buffer, allottedSize); + node, port_index, params, &buffer, allottedSize, OMX_TRUE /* crossProcess */); reply->writeInt32(err); -- cgit v1.1 From ace612c5f838944d1b88be18e317709e640becc2 Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Fri, 2 Sep 2016 11:36:46 -0700 Subject: Fix stack content leak vulnerability in mediaserver bug: 30875060 Change-Id: I03f4d08b7b31ac5b507cfc9e65e5607c73972d95 (cherry picked from commit 9a6861cbd3bb0e1b8fe4c105795256ee032f9664) --- media/libmedia/IDrm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IDrm.cpp b/media/libmedia/IDrm.cpp index 7c709cd..6f6530b 100644 --- a/media/libmedia/IDrm.cpp +++ b/media/libmedia/IDrm.cpp @@ -912,7 +912,7 @@ status_t BnDrm::onTransact( readVector(data, keyId); readVector(data, message); readVector(data, signature); - bool match; + bool match = false; uint32_t result = verify(sessionId, keyId, message, signature, match); reply->writeInt32(match); reply->writeInt32(result); -- cgit v1.1 From c2dd82bfd6d7aea5b6760efc0712ae11d7a52e6b Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 20 Sep 2016 13:36:40 -0700 Subject: Check mprotect result mprotect can theoretically fail, which could then let one exploit a vulnerable codec if one exists on the device. Bug: 31350239 Change-Id: I7b99c190619f0fb2eb93119596e6da0d2deb8ba5 (cherry picked from commit 866c800c0624bb13eee44973cc8a2ecd0012de6e) --- media/libmedia/IOMX.cpp | 52 ++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 24 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index c28eac8..365d9ac 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -733,31 +733,35 @@ status_t BnOMX::onTransact( // mark the last page as inaccessible, to avoid exploitation // of codecs that access past the end of the allocation because // they didn't check the size - mprotect((char*)params + allocSize - pageSize, pageSize, PROT_NONE); - switch (code) { - case GET_PARAMETER: - err = getParameter(node, index, params, size); - break; - case SET_PARAMETER: - err = setParameter(node, index, params, size); - break; - case GET_CONFIG: - err = getConfig(node, index, params, size); - break; - case SET_CONFIG: - err = setConfig(node, index, params, size); - break; - case SET_INTERNAL_OPTION: - { - InternalOptionType type = - (InternalOptionType)data.readInt32(); - - err = setInternalOption(node, index, type, params, size); - break; + if (mprotect((char*)params + allocSize - pageSize, pageSize, + PROT_NONE) != 0) { + ALOGE("mprotect failed: %s", strerror(errno)); + } else { + switch (code) { + case GET_PARAMETER: + err = getParameter(node, index, params, size); + break; + case SET_PARAMETER: + err = setParameter(node, index, params, size); + break; + case GET_CONFIG: + err = getConfig(node, index, params, size); + break; + case SET_CONFIG: + err = setConfig(node, index, params, size); + break; + case SET_INTERNAL_OPTION: + { + InternalOptionType type = + (InternalOptionType)data.readInt32(); + + err = setInternalOption(node, index, type, params, size); + break; + } + + default: + TRESPASS(); } - - default: - TRESPASS(); } } } -- cgit v1.1