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 +- media/libstagefright/OMXClient.cpp | 13 +- media/libstagefright/include/OMX.h | 4 +- media/libstagefright/include/OMXNodeInstance.h | 10 +- media/libstagefright/omx/OMX.cpp | 8 +- media/libstagefright/omx/OMXNodeInstance.cpp | 184 ++++++++++++++++++++++--- 6 files changed, 194 insertions(+), 39 deletions(-) (limited to 'media') 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); diff --git a/media/libstagefright/OMXClient.cpp b/media/libstagefright/OMXClient.cpp index e69890d..d252cb6 100644 --- a/media/libstagefright/OMXClient.cpp +++ b/media/libstagefright/OMXClient.cpp @@ -90,7 +90,7 @@ struct MuxOMX : public IOMX { 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); virtual status_t useGraphicBuffer( node_id node, OMX_U32 port_index, @@ -120,7 +120,7 @@ struct MuxOMX : public IOMX { 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); virtual status_t freeBuffer( node_id node, OMX_U32 port_index, buffer_id buffer); @@ -322,8 +322,9 @@ status_t MuxOMX::getGraphicBufferUsage( status_t MuxOMX::useBuffer( node_id node, OMX_U32 port_index, const sp ¶ms, - buffer_id *buffer, OMX_U32 allottedSize) { - return getOMX(node)->useBuffer(node, port_index, params, buffer, allottedSize); + buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL /* crossProcess */) { + return getOMX(node)->useBuffer( + node, port_index, params, buffer, allottedSize, OMX_FALSE /* crossProcess */); } status_t MuxOMX::useGraphicBuffer( @@ -375,9 +376,9 @@ status_t MuxOMX::allocateBuffer( status_t MuxOMX::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 */) { return getOMX(node)->allocateBufferWithBackup( - node, port_index, params, buffer, allottedSize); + node, port_index, params, buffer, allottedSize, OMX_FALSE /* crossProcess */); } status_t MuxOMX::freeBuffer( diff --git a/media/libstagefright/include/OMX.h b/media/libstagefright/include/OMX.h index e7c4f6d..b20b2ea 100644 --- a/media/libstagefright/include/OMX.h +++ b/media/libstagefright/include/OMX.h @@ -81,7 +81,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); virtual status_t useGraphicBuffer( node_id node, OMX_U32 port_index, @@ -113,7 +113,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); virtual status_t freeBuffer( node_id node, OMX_U32 port_index, buffer_id buffer); diff --git a/media/libstagefright/include/OMXNodeInstance.h b/media/libstagefright/include/OMXNodeInstance.h index babf5b7..bd33ab7 100644 --- a/media/libstagefright/include/OMXNodeInstance.h +++ b/media/libstagefright/include/OMXNodeInstance.h @@ -21,6 +21,7 @@ #include "OMX.h" #include +#include #include namespace android { @@ -71,7 +72,7 @@ struct OMXNodeInstance { status_t useBuffer( OMX_U32 portIndex, const sp ¶ms, - OMX::buffer_id *buffer, OMX_U32 allottedSize); + OMX::buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess); status_t useGraphicBuffer( OMX_U32 portIndex, const sp &graphicBuffer, @@ -101,7 +102,7 @@ struct OMXNodeInstance { status_t allocateBufferWithBackup( OMX_U32 portIndex, const sp ¶ms, - OMX::buffer_id *buffer, OMX_U32 allottedSize); + OMX::buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess); status_t freeBuffer(OMX_U32 portIndex, OMX::buffer_id buffer); @@ -146,6 +147,9 @@ private: OMX_HANDLETYPE mHandle; sp mObserver; bool mDying; + bool mSailed; // configuration is set (no more meta-mode changes) + bool mQueriedProhibitedExtensions; + SortedVector mProhibitedExtensions; bool mIsSecure; // Lock only covers mGraphicBufferSource. We can't always use mLock @@ -191,6 +195,8 @@ private: OMX::buffer_id findBufferID(OMX_BUFFERHEADERTYPE *bufferHeader); void invalidateBufferID(OMX::buffer_id buffer); + bool isProhibitedIndex_l(OMX_INDEXTYPE index); + status_t useGraphicBuffer2_l( OMX_U32 portIndex, const sp &graphicBuffer, OMX::buffer_id *buffer); diff --git a/media/libstagefright/omx/OMX.cpp b/media/libstagefright/omx/OMX.cpp index 56b6055..5898b4e 100644 --- a/media/libstagefright/omx/OMX.cpp +++ b/media/libstagefright/omx/OMX.cpp @@ -368,9 +368,9 @@ status_t OMX::configureVideoTunnelMode( status_t OMX::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) { return findInstance(node)->useBuffer( - port_index, params, buffer, allottedSize); + port_index, params, buffer, allottedSize, crossProcess); } status_t OMX::useGraphicBuffer( @@ -421,9 +421,9 @@ status_t OMX::allocateBuffer( status_t OMX::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) { return findInstance(node)->allocateBufferWithBackup( - port_index, params, buffer, allottedSize); + port_index, params, buffer, allottedSize, crossProcess); } status_t OMX::freeBuffer(node_id node, OMX_U32 port_index, buffer_id buffer) { diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp index 7f534b5..a0dc2ec 100644 --- a/media/libstagefright/omx/OMXNodeInstance.cpp +++ b/media/libstagefright/omx/OMXNodeInstance.cpp @@ -101,26 +101,34 @@ static void InitOMXParams(T *params) { namespace android { struct BufferMeta { - BufferMeta(const sp &mem, OMX_U32 portIndex, bool is_backup = false) + BufferMeta( + const sp &mem, OMX_U32 portIndex, bool copyToOmx, + bool copyFromOmx, OMX_U8 *backup) : mMem(mem), - mIsBackup(is_backup), - mPortIndex(portIndex) { + mCopyFromOmx(copyFromOmx), + mCopyToOmx(copyToOmx), + mPortIndex(portIndex), + mBackup(backup) { } BufferMeta(size_t size, OMX_U32 portIndex) : mSize(size), - mIsBackup(false), - mPortIndex(portIndex) { + mCopyFromOmx(false), + mCopyToOmx(false), + mPortIndex(portIndex), + mBackup(NULL) { } BufferMeta(const sp &graphicBuffer, OMX_U32 portIndex) : mGraphicBuffer(graphicBuffer), - mIsBackup(false), - mPortIndex(portIndex) { + mCopyFromOmx(false), + mCopyToOmx(false), + mPortIndex(portIndex), + mBackup(NULL) { } void CopyFromOMX(const OMX_BUFFERHEADERTYPE *header) { - if (!mIsBackup) { + if (!mCopyFromOmx) { return; } @@ -131,7 +139,7 @@ struct BufferMeta { } void CopyToOMX(const OMX_BUFFERHEADERTYPE *header) { - if (!mIsBackup) { + if (!mCopyToOmx) { return; } @@ -167,12 +175,18 @@ struct BufferMeta { return mPortIndex; } + ~BufferMeta() { + delete[] mBackup; + } + private: sp mGraphicBuffer; sp mMem; size_t mSize; - bool mIsBackup; + bool mCopyFromOmx; + bool mCopyToOmx; OMX_U32 mPortIndex; + OMX_U8 *mBackup; BufferMeta(const BufferMeta &); BufferMeta &operator=(const BufferMeta &); @@ -199,6 +213,8 @@ OMXNodeInstance::OMXNodeInstance( mHandle(NULL), mObserver(observer), mDying(false), + mSailed(false), + mQueriedProhibitedExtensions(false), mBufferIDCount(0) { mName = ADebug::GetDebugName(name); @@ -370,7 +386,12 @@ status_t OMXNodeInstance::freeNode(OMXMaster *master) { status_t OMXNodeInstance::sendCommand( OMX_COMMANDTYPE cmd, OMX_S32 param) { - const sp& bufferSource(getGraphicBufferSource()); + if (cmd == OMX_CommandStateSet) { + // We do not support returning from unloaded state, so there are no configurations past + // first StateSet command. + mSailed = true; + } + const sp bufferSource(getGraphicBufferSource()); if (bufferSource != NULL && cmd == OMX_CommandStateSet) { if (param == OMX_StateIdle) { // Initiating transition from Executing -> Idle @@ -403,10 +424,57 @@ status_t OMXNodeInstance::sendCommand( return StatusFromOMXError(err); } +bool OMXNodeInstance::isProhibitedIndex_l(OMX_INDEXTYPE index) { + // these extensions can only be used from OMXNodeInstance, not by clients directly. + static const char *restricted_extensions[] = { + "OMX.google.android.index.storeMetaDataInBuffers", + "OMX.google.android.index.storeANWBufferInMetadata", + "OMX.google.android.index.prepareForAdaptivePlayback", + "OMX.google.android.index.configureVideoTunnelMode", + "OMX.google.android.index.useAndroidNativeBuffer2", + "OMX.google.android.index.useAndroidNativeBuffer", + "OMX.google.android.index.enableAndroidNativeBuffers", + "OMX.google.android.index.allocateNativeHandle", + "OMX.google.android.index.getAndroidNativeBufferUsage", + }; + + if ((index > OMX_IndexComponentStartUnused && index <= OMX_IndexParamStandardComponentRole) + || (index > OMX_IndexPortStartUnused && index <= OMX_IndexParamCompBufferSupplier) + || (index > OMX_IndexAudioStartUnused && index <= OMX_IndexConfigAudioChannelVolume) + || (index > OMX_IndexVideoStartUnused && index <= OMX_IndexConfigVideoNalSize) + || (index > OMX_IndexCommonStartUnused + && index <= OMX_IndexConfigCommonTransitionEffect) + || (index > (OMX_INDEXTYPE)OMX_IndexExtAudioStartUnused + && index <= (OMX_INDEXTYPE)OMX_IndexParamAudioAndroidEac3) + || (index > (OMX_INDEXTYPE)OMX_IndexExtVideoStartUnused + && index <= (OMX_INDEXTYPE)OMX_IndexParamSliceSegments) + || (index > (OMX_INDEXTYPE)OMX_IndexExtOtherStartUnused + && index <= (OMX_INDEXTYPE)OMX_IndexParamConsumerUsageBits)) { + return false; + } + + if (!mQueriedProhibitedExtensions) { + for (size_t i = 0; i < NELEM(restricted_extensions); ++i) { + OMX_INDEXTYPE ext; + if (OMX_GetExtensionIndex(mHandle, (OMX_STRING)restricted_extensions[i], &ext) == OMX_ErrorNone) { + mProhibitedExtensions.add(ext); + } + } + mQueriedProhibitedExtensions = true; + } + + return mProhibitedExtensions.indexOf(index) >= 0; +} + status_t OMXNodeInstance::getParameter( OMX_INDEXTYPE index, void *params, size_t /* size */) { Mutex::Autolock autoLock(mLock); + if (isProhibitedIndex_l(index)) { + android_errorWriteLog(0x534e4554, "29422020"); + return BAD_INDEX; + } + OMX_ERRORTYPE err = OMX_GetParameter(mHandle, index, params); OMX_INDEXEXTTYPE extIndex = (OMX_INDEXEXTTYPE)index; // some errors are expected for getParameter @@ -422,6 +490,11 @@ status_t OMXNodeInstance::setParameter( OMX_INDEXEXTTYPE extIndex = (OMX_INDEXEXTTYPE)index; CLOG_CONFIG(setParameter, "%s(%#x), %zu@%p)", asString(extIndex), index, size, params); + if (isProhibitedIndex_l(index)) { + android_errorWriteLog(0x534e4554, "29422020"); + return BAD_INDEX; + } + OMX_ERRORTYPE err = OMX_SetParameter( mHandle, index, const_cast(params)); CLOG_IF_ERROR(setParameter, err, "%s(%#x)", asString(extIndex), index); @@ -432,6 +505,11 @@ status_t OMXNodeInstance::getConfig( OMX_INDEXTYPE index, void *params, size_t /* size */) { Mutex::Autolock autoLock(mLock); + if (isProhibitedIndex_l(index)) { + android_errorWriteLog(0x534e4554, "29422020"); + return BAD_INDEX; + } + OMX_ERRORTYPE err = OMX_GetConfig(mHandle, index, params); OMX_INDEXEXTTYPE extIndex = (OMX_INDEXEXTTYPE)index; // some errors are expected for getConfig @@ -447,6 +525,11 @@ status_t OMXNodeInstance::setConfig( OMX_INDEXEXTTYPE extIndex = (OMX_INDEXEXTTYPE)index; CLOG_CONFIG(setConfig, "%s(%#x), %zu@%p)", asString(extIndex), index, size, params); + if (isProhibitedIndex_l(index)) { + android_errorWriteLog(0x534e4554, "29422020"); + return BAD_INDEX; + } + OMX_ERRORTYPE err = OMX_SetConfig( mHandle, index, const_cast(params)); CLOG_IF_ERROR(setConfig, err, "%s(%#x)", asString(extIndex), index); @@ -526,6 +609,10 @@ status_t OMXNodeInstance::storeMetaDataInBuffers( status_t OMXNodeInstance::storeMetaDataInBuffers_l( OMX_U32 portIndex, OMX_BOOL enable, MetadataBufferType *type) { + if (mSailed) { + android_errorWriteLog(0x534e4554, "29422020"); + return INVALID_OPERATION; + } if (portIndex != kPortIndexInput && portIndex != kPortIndexOutput) { android_errorWriteLog(0x534e4554, "26324358"); return BAD_VALUE; @@ -593,6 +680,10 @@ status_t OMXNodeInstance::prepareForAdaptivePlayback( OMX_U32 portIndex, OMX_BOOL enable, OMX_U32 maxFrameWidth, OMX_U32 maxFrameHeight) { Mutex::Autolock autolock(mLock); + if (mSailed) { + android_errorWriteLog(0x534e4554, "29422020"); + return INVALID_OPERATION; + } CLOG_CONFIG(prepareForAdaptivePlayback, "%s:%u en=%d max=%ux%u", portString(portIndex), portIndex, enable, maxFrameWidth, maxFrameHeight); @@ -623,6 +714,10 @@ status_t OMXNodeInstance::configureVideoTunnelMode( OMX_U32 portIndex, OMX_BOOL tunneled, OMX_U32 audioHwSync, native_handle_t **sidebandHandle) { Mutex::Autolock autolock(mLock); + if (mSailed) { + android_errorWriteLog(0x534e4554, "29422020"); + return INVALID_OPERATION; + } CLOG_CONFIG(configureVideoTunnelMode, "%s:%u tun=%d sync=%u", portString(portIndex), portIndex, tunneled, audioHwSync); @@ -663,23 +758,48 @@ status_t OMXNodeInstance::configureVideoTunnelMode( status_t OMXNodeInstance::useBuffer( OMX_U32 portIndex, const sp ¶ms, - OMX::buffer_id *buffer, OMX_U32 allottedSize) { + OMX::buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess) { Mutex::Autolock autoLock(mLock); - if (allottedSize > params->size()) { + if (allottedSize > params->size() || portIndex >= NELEM(mNumPortBuffers)) { return BAD_VALUE; } - BufferMeta *buffer_meta = new BufferMeta(params, portIndex); + // metadata buffers are not connected cross process + BufferMeta *buffer_meta; + bool isMeta = mMetadataType[portIndex] != kMetadataBufferTypeInvalid; + bool useBackup = crossProcess && isMeta; // use a backup buffer instead of the actual buffer + OMX_U8 *data = static_cast(params->pointer()); + // allocate backup buffer + if (useBackup) { + data = new (std::nothrow) OMX_U8[allottedSize]; + if (data == NULL) { + return NO_MEMORY; + } + memset(data, 0, allottedSize); + + // if we are not connecting the buffers, the sizes must match + if (allottedSize != params->size()) { + CLOG_ERROR(useBuffer, BAD_VALUE, SIMPLE_BUFFER(portIndex, (size_t)allottedSize, data)); + delete[] data; + return BAD_VALUE; + } + + buffer_meta = new BufferMeta( + params, portIndex, false /* copyToOmx */, false /* copyFromOmx */, data); + } else { + buffer_meta = new BufferMeta( + params, portIndex, false /* copyFromOmx */, false /* copyToOmx */, NULL); + } OMX_BUFFERHEADERTYPE *header; OMX_ERRORTYPE err = OMX_UseBuffer( mHandle, &header, portIndex, buffer_meta, - allottedSize, static_cast(params->pointer())); + allottedSize, data); if (err != OMX_ErrorNone) { CLOG_ERROR(useBuffer, err, SIMPLE_BUFFER( - portIndex, (size_t)allottedSize, params->pointer())); + portIndex, (size_t)allottedSize, data)); delete buffer_meta; buffer_meta = NULL; @@ -864,7 +984,16 @@ status_t OMXNodeInstance::createGraphicBufferSource( OMX_U32 portIndex, sp bufferConsumer, MetadataBufferType *type) { status_t err; - const sp& surfaceCheck = getGraphicBufferSource(); + // only allow graphic source on input port, when there are no allocated buffers yet + if (portIndex != kPortIndexInput) { + android_errorWriteLog(0x534e4554, "29422020"); + return BAD_VALUE; + } else if (mNumPortBuffers[portIndex] > 0) { + android_errorWriteLog(0x534e4554, "29422020"); + return INVALID_OPERATION; + } + + const sp surfaceCheck = getGraphicBufferSource(); if (surfaceCheck != NULL) { if (portIndex < NELEM(mMetadataType) && type != NULL) { *type = mMetadataType[portIndex]; @@ -1020,13 +1149,21 @@ status_t OMXNodeInstance::allocateBuffer( status_t OMXNodeInstance::allocateBufferWithBackup( OMX_U32 portIndex, const sp ¶ms, - OMX::buffer_id *buffer, OMX_U32 allottedSize) { + OMX::buffer_id *buffer, OMX_U32 allottedSize, OMX_BOOL crossProcess) { Mutex::Autolock autoLock(mLock); - if (allottedSize > params->size()) { + if (allottedSize > params->size() || portIndex >= NELEM(mNumPortBuffers)) { return BAD_VALUE; } - BufferMeta *buffer_meta = new BufferMeta(params, portIndex, true); + // metadata buffers are not connected cross process + bool isMeta = mMetadataType[portIndex] != kMetadataBufferTypeInvalid; + bool copy = !(crossProcess && isMeta); + + BufferMeta *buffer_meta = new BufferMeta( + params, portIndex, + (portIndex == kPortIndexInput) && copy /* copyToOmx */, + (portIndex == kPortIndexOutput) && copy /* copyFromOmx */, + NULL /* data */); OMX_BUFFERHEADERTYPE *header; @@ -1044,6 +1181,7 @@ status_t OMXNodeInstance::allocateBufferWithBackup( } CHECK_EQ(header->pAppPrivate, buffer_meta); + memset(header->pBuffer, 0, header->nAllocLen); *buffer = makeBufferID(header); @@ -1122,6 +1260,12 @@ status_t OMXNodeInstance::emptyBuffer( OMX_U32 flags, OMX_TICKS timestamp, int fenceFd) { Mutex::Autolock autoLock(mLock); + // no emptybuffer if using input surface + if (getGraphicBufferSource() != NULL) { + android_errorWriteLog(0x534e4554, "29422020"); + return INVALID_OPERATION; + } + OMX_BUFFERHEADERTYPE *header = findBufferHeader(buffer, kPortIndexInput); if (header == NULL) { return BAD_VALUE; -- cgit v1.1