Skip to content

Commit ec6b7bf

Browse files
medialajosandi34
medialajos
authored and
andi34
committed
DO NOT MERGE: 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/input surface after first sendCommand (except to Idle for OMXCodec quirk). - Disallow store-meta for input cross process. - Disallow emptyBuffer for surface input (via IOMX). - Fix checking for input surface. [backported from L] Bug: 29422020 Change-Id: I801c77b80e703903f62e42d76fd2e76a34e4bc8e (cherry picked from commit 807e827)
1 parent beb5720 commit ec6b7bf

File tree

8 files changed

+211
-41
lines changed

8 files changed

+211
-41
lines changed

‎include/media/IOMX.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class IOMX : public IInterface {
9595

9696
virtual status_t useBuffer(
9797
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
98-
buffer_id *buffer) = 0;
98+
buffer_id *buffer, OMX_BOOL crossProcess = OMX_FALSE) = 0;
9999

100100
virtual status_t useGraphicBuffer(
101101
node_id node, OMX_U32 port_index,
@@ -121,7 +121,7 @@ class IOMX : public IInterface {
121121

122122
virtual status_t allocateBufferWithBackup(
123123
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
124-
buffer_id *buffer) = 0;
124+
buffer_id *buffer, OMX_BOOL crossProcess = OMX_FALSE) = 0;
125125

126126
virtual status_t freeBuffer(
127127
node_id node, OMX_U32 port_index, buffer_id buffer) = 0;

‎media/libmedia/IOMX.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ class BpOMX : public BpInterface<IOMX> {
244244

245245
virtual status_t useBuffer(
246246
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
247-
buffer_id *buffer) {
247+
buffer_id *buffer, OMX_BOOL /* crossProcess */) {
248248
Parcel data, reply;
249249
data.writeInterfaceToken(IOMX::getInterfaceDescriptor());
250250
data.writeIntPtr((intptr_t)node);
@@ -395,7 +395,7 @@ class BpOMX : public BpInterface<IOMX> {
395395

396396
virtual status_t allocateBufferWithBackup(
397397
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
398-
buffer_id *buffer) {
398+
buffer_id *buffer, OMX_BOOL /* crossProcess */) {
399399
Parcel data, reply;
400400
data.writeInterfaceToken(IOMX::getInterfaceDescriptor());
401401
data.writeIntPtr((intptr_t)node);
@@ -741,7 +741,8 @@ status_t BnOMX::onTransact(
741741
interface_cast<IMemory>(data.readStrongBinder());
742742

743743
buffer_id buffer;
744-
status_t err = useBuffer(node, port_index, params, &buffer);
744+
status_t err = useBuffer(
745+
node, port_index, params, &buffer, OMX_TRUE /* crossProcess */);
745746
reply->writeInt32(err);
746747

747748
if (err == OK) {
@@ -829,7 +830,10 @@ status_t BnOMX::onTransact(
829830
OMX_U32 port_index = data.readInt32();
830831
OMX_BOOL enable = (OMX_BOOL)data.readInt32();
831832

832-
status_t err = storeMetaDataInBuffers(node, port_index, enable);
833+
status_t err =
834+
// only control output metadata via Binder
835+
port_index != 1 /* kOutputPortIndex */ ? BAD_VALUE :
836+
storeMetaDataInBuffers(node, port_index, enable);
833837
reply->writeInt32(err);
834838

835839
return NO_ERROR;
@@ -891,7 +895,7 @@ status_t BnOMX::onTransact(
891895

892896
buffer_id buffer;
893897
status_t err = allocateBufferWithBackup(
894-
node, port_index, params, &buffer);
898+
node, port_index, params, &buffer, OMX_TRUE /* crossProcess */);
895899

896900
reply->writeInt32(err);
897901

‎media/libstagefright/ACodec.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -866,7 +866,8 @@ ACodec::BufferInfo *ACodec::dequeueBufferFromNativeWindow() {
866866
VideoDecoderOutputMetaData *metaData =
867867
reinterpret_cast<VideoDecoderOutputMetaData *>(
868868
oldest->mData->base());
869-
CHECK_EQ(metaData->eType, kMetadataBufferTypeGrallocSource);
869+
// metaData is only readable if codec is in the same process
870+
//CHECK_EQ(metaData->eType, kMetadataBufferTypeGrallocSource);
870871

871872
ALOGV("replaced oldest buffer #%u with age %u (%p/%p stored in %p)",
872873
oldest - &mBuffers[kPortIndexOutput][0],

‎media/libstagefright/OMXClient.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ struct MuxOMX : public IOMX {
8181

8282
virtual status_t useBuffer(
8383
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
84-
buffer_id *buffer);
84+
buffer_id *buffer, OMX_BOOL crossProcess);
8585

8686
virtual status_t useGraphicBuffer(
8787
node_id node, OMX_U32 port_index,
@@ -103,7 +103,7 @@ struct MuxOMX : public IOMX {
103103

104104
virtual status_t allocateBufferWithBackup(
105105
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
106-
buffer_id *buffer);
106+
buffer_id *buffer, OMX_BOOL crossProcess);
107107

108108
virtual status_t freeBuffer(
109109
node_id node, OMX_U32 port_index, buffer_id buffer);
@@ -291,8 +291,9 @@ status_t MuxOMX::getGraphicBufferUsage(
291291

292292
status_t MuxOMX::useBuffer(
293293
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
294-
buffer_id *buffer) {
295-
return getOMX(node)->useBuffer(node, port_index, params, buffer);
294+
buffer_id *buffer, OMX_BOOL /* crossProcess */) {
295+
return getOMX(node)->useBuffer(
296+
node, port_index, params, buffer, OMX_FALSE /* crossProcess */);
296297
}
297298

298299
status_t MuxOMX::useGraphicBuffer(
@@ -330,9 +331,9 @@ status_t MuxOMX::allocateBuffer(
330331

331332
status_t MuxOMX::allocateBufferWithBackup(
332333
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
333-
buffer_id *buffer) {
334+
buffer_id *buffer, OMX_BOOL /* crossProcess */) {
334335
return getOMX(node)->allocateBufferWithBackup(
335-
node, port_index, params, buffer);
336+
node, port_index, params, buffer, OMX_FALSE /* crossProcess */);
336337
}
337338

338339
status_t MuxOMX::freeBuffer(

‎media/libstagefright/include/OMX.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ class OMX : public BnOMX,
7777

7878
virtual status_t useBuffer(
7979
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
80-
buffer_id *buffer);
80+
buffer_id *buffer, OMX_BOOL crossProcess);
8181

8282
virtual status_t useGraphicBuffer(
8383
node_id node, OMX_U32 port_index,
@@ -99,7 +99,7 @@ class OMX : public BnOMX,
9999

100100
virtual status_t allocateBufferWithBackup(
101101
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
102-
buffer_id *buffer);
102+
buffer_id *buffer, OMX_BOOL crossProcess);
103103

104104
virtual status_t freeBuffer(
105105
node_id node, OMX_U32 port_index, buffer_id buffer);

‎media/libstagefright/include/OMXNodeInstance.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "OMX.h"
2222

2323
#include <utils/RefBase.h>
24+
#include <utils/SortedVector.h>
2425
#include <utils/threads.h>
2526

2627
namespace android {
@@ -64,7 +65,7 @@ struct OMXNodeInstance {
6465

6566
status_t useBuffer(
6667
OMX_U32 portIndex, const sp<IMemory> &params,
67-
OMX::buffer_id *buffer);
68+
OMX::buffer_id *buffer, OMX_BOOL crossProcess);
6869

6970
status_t useGraphicBuffer(
7071
OMX_U32 portIndex, const sp<GraphicBuffer> &graphicBuffer,
@@ -85,7 +86,7 @@ struct OMXNodeInstance {
8586

8687
status_t allocateBufferWithBackup(
8788
OMX_U32 portIndex, const sp<IMemory> &params,
88-
OMX::buffer_id *buffer);
89+
OMX::buffer_id *buffer, OMX_BOOL crossProcess);
8990

9091
status_t freeBuffer(OMX_U32 portIndex, OMX::buffer_id buffer);
9192

@@ -129,6 +130,9 @@ struct OMXNodeInstance {
129130
OMX_HANDLETYPE mHandle;
130131
sp<IOMXObserver> mObserver;
131132
bool mDying;
133+
bool mSailed; // configuration is set (no more meta-mode changes)
134+
bool mQueriedProhibitedExtensions;
135+
SortedVector<OMX_INDEXTYPE> mProhibitedExtensions;
132136
bool mIsSecure;
133137

134138
// Lock only covers mGraphicBufferSource. We can't always use mLock
@@ -144,11 +148,17 @@ struct OMXNodeInstance {
144148
};
145149
Vector<ActiveBuffer> mActiveBuffers;
146150

151+
// metadata mode tracking
152+
bool mUsingMetadata[2];
153+
147154
~OMXNodeInstance();
148155

149156
void addActiveBuffer(OMX_U32 portIndex, OMX::buffer_id id);
150157
void removeActiveBuffer(OMX_U32 portIndex, OMX::buffer_id id);
151158
void freeActiveBuffers();
159+
160+
bool isProhibitedIndex_l(OMX_INDEXTYPE index);
161+
152162
status_t useGraphicBuffer2_l(
153163
OMX_U32 portIndex, const sp<GraphicBuffer> &graphicBuffer,
154164
OMX::buffer_id *buffer);

‎media/libstagefright/omx/OMX.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -350,9 +350,9 @@ status_t OMX::prepareForAdaptivePlayback(
350350

351351
status_t OMX::useBuffer(
352352
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
353-
buffer_id *buffer) {
353+
buffer_id *buffer, OMX_BOOL crossProcess) {
354354
return findInstance(node)->useBuffer(
355-
port_index, params, buffer);
355+
port_index, params, buffer, crossProcess);
356356
}
357357

358358
status_t OMX::useGraphicBuffer(
@@ -389,9 +389,9 @@ status_t OMX::allocateBuffer(
389389

390390
status_t OMX::allocateBufferWithBackup(
391391
node_id node, OMX_U32 port_index, const sp<IMemory> &params,
392-
buffer_id *buffer) {
392+
buffer_id *buffer, OMX_BOOL crossProcess) {
393393
return findInstance(node)->allocateBufferWithBackup(
394-
port_index, params, buffer);
394+
port_index, params, buffer, crossProcess);
395395
}
396396

397397
status_t OMX::freeBuffer(node_id node, OMX_U32 port_index, buffer_id buffer) {

0 commit comments

Comments
 (0)