Merge "Improve visibility of IMemory security risks"

cmds/stagefright/stagefright.cpp

@@ -989,7 +989,7 @@ int main(int argc, char **argv) {
                 failed = false;
                 printf("getFrameAtTime(%s) => OK\n", filename);
 
-                VideoFrame *frame = (VideoFrame *)mem->pointer();
+                VideoFrame *frame = (VideoFrame *)mem->unsecurePointer();
 
                 CHECK_EQ(writeJpegFile("/sdcard/out.jpg",
                             frame->getFlattenedData(),

cmds/stagefright/stream.cpp

@@ -116,7 +116,7 @@ void MyStreamSource::onBufferAvailable(size_t index) {
 
     sp<IMemory> mem = mBuffers.itemAt(index);
 
-    ssize_t n = read(mFd, mem->pointer(), mem->size());
+    ssize_t n = read(mFd, mem->unsecurePointer(), mem->size());
     if (n <= 0) {
         mListener->issueCommand(IStreamListener::EOS, false /* synchronous */);
     } else {
@@ -238,7 +238,7 @@ ssize_t MyConvertingStreamSource::writeData(const void *data, size_t size) {
             copy = mem->size() - mCurrentBufferOffset;
         }
 
-        memcpy((uint8_t *)mem->pointer() + mCurrentBufferOffset, data, copy);
+        memcpy((uint8_t *)mem->unsecurePointer() + mCurrentBufferOffset, data, copy);
         mCurrentBufferOffset += copy;
 
         if (mCurrentBufferOffset == mem->size()) {

drm/libmediadrm/CryptoHal.cpp

@@ -321,7 +321,11 @@ status_t CryptoHal::toSharedBuffer(const sp<IMemory>& memory, int32_t seqNum, ::
      }
 
     // memory must be within the address space of the heap
-    if (memory->pointer() != static_cast<uint8_t *>(heap->getBase()) + memory->offset()  ||
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    if (memory->unsecurePointer() != static_cast<uint8_t *>(heap->getBase()) + memory->offset()  ||
             heap->getSize() < memory->offset() + memory->size() ||
             SIZE_MAX - memory->offset() < memory->size()) {
         android_errorWriteLog(0x534e4554, "76221123");

media/codec2/sfplugin/Codec2Buffer.cpp

@@ -764,7 +764,11 @@ EncryptedLinearBlockBuffer::EncryptedLinearBlockBuffer(
         const std::shared_ptr<C2LinearBlock> &block,
         const sp<IMemory> &memory,
         int32_t heapSeqNum)
-    : Codec2Buffer(format, new ABuffer(memory->pointer(), memory->size())),
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    : Codec2Buffer(format, new ABuffer(memory->unsecurePointer(), memory->size())),
       mBlock(block),
       mMemory(memory),
       mHeapSeqNum(heapSeqNum) {
@@ -800,7 +804,7 @@ bool EncryptedLinearBlockBuffer::copyDecryptedContent(
     if (view.size() < length) {
         return false;
     }
-    memcpy(view.data(), decrypted->pointer(), length);
+    memcpy(view.data(), decrypted->unsecurePointer(), length);
     return true;
 }
 

media/libaudioclient/AudioEffect.cpp

@@ -159,7 +159,11 @@ status_t AudioEffect::set(const effect_uuid_t *type,
 
     mIEffect = iEffect;
     mCblkMemory = cblk;
-    mCblk = static_cast<effect_param_cblk_t*>(cblk->pointer());
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    mCblk = static_cast<effect_param_cblk_t*>(cblk->unsecurePointer());
     int bufOffset = ((sizeof(effect_param_cblk_t) - 1) / sizeof(int) + 1) * sizeof(int);
     mCblk->buffer = (uint8_t *)mCblk + bufOffset;
 

media/libaudioclient/AudioRecord.cpp

@@ -759,7 +759,11 @@ status_t AudioRecord::createRecord_l(const Modulo<uint32_t> &epoch, const String
         status = NO_INIT;
         goto exit;
     }
-    iMemPointer = output.cblk ->pointer();
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    iMemPointer = output.cblk ->unsecurePointer();
     if (iMemPointer == NULL) {
         ALOGE("%s(%d): Could not get control block pointer", __func__, mPortId);
         status = NO_INIT;
@@ -774,7 +778,11 @@ status_t AudioRecord::createRecord_l(const Modulo<uint32_t> &epoch, const String
     if (output.buffers == 0) {
         buffers = cblk + 1;
     } else {
-        buffers = output.buffers->pointer();
+        // TODO: Using unsecurePointer() has some associated security pitfalls
+        //       (see declaration for details).
+        //       Either document why it is safe in this case or address the
+        //       issue (e.g. by copying).
+        buffers = output.buffers->unsecurePointer();
         if (buffers == NULL) {
             ALOGE("%s(%d): Could not get buffer pointer", __func__, mPortId);
             status = NO_INIT;

media/libaudioclient/AudioTrack.cpp

@@ -406,7 +406,7 @@ status_t AudioTrack::set(
     mDoNotReconnect = doNotReconnect;
 
     ALOGV_IF(sharedBuffer != 0, "%s(): sharedBuffer: %p, size: %zu",
-            __func__, sharedBuffer->pointer(), sharedBuffer->size());
+            __func__, sharedBuffer->unsecurePointer(), sharedBuffer->size());
 
     ALOGV("%s(): streamType %d frameCount %zu flags %04x",
             __func__, streamType, frameCount, flags);
@@ -1508,7 +1508,11 @@ status_t AudioTrack::createTrack_l()
         status = NO_INIT;
         goto exit;
     }
-    void *iMemPointer = iMem->pointer();
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    void *iMemPointer = iMem->unsecurePointer();
     if (iMemPointer == NULL) {
         ALOGE("%s(%d): Could not get control block pointer", __func__, mPortId);
         status = NO_INIT;
@@ -1563,7 +1567,11 @@ status_t AudioTrack::createTrack_l()
     if (mSharedBuffer == 0) {
         buffers = cblk + 1;
     } else {
-        buffers = mSharedBuffer->pointer();
+        // TODO: Using unsecurePointer() has some associated security pitfalls
+        //       (see declaration for details).
+        //       Either document why it is safe in this case or address the
+        //       issue (e.g. by copying).
+        buffers = mSharedBuffer->unsecurePointer();
         if (buffers == NULL) {
             ALOGE("%s(%d): Could not get buffer pointer", __func__, mPortId);
             status = NO_INIT;

media/libaudioclient/IAudioTrack.cpp

@@ -62,7 +62,7 @@ public:
         status_t status = remote()->transact(GET_CBLK, data, &reply);
         if (status == NO_ERROR) {
             cblk = interface_cast<IMemory>(reply.readStrongBinder());
-            if (cblk != 0 && cblk->pointer() == NULL) {
+            if (cblk != 0 && cblk->unsecurePointer() == NULL) {
                 cblk.clear();
             }
         }

media/libaudioclient/IEffect.cpp

@@ -122,7 +122,7 @@ public:
         status_t status = remote()->transact(GET_CBLK, data, &reply);
         if (status == NO_ERROR) {
             cblk = interface_cast<IMemory>(reply.readStrongBinder());
-            if (cblk != 0 && cblk->pointer() == NULL) {
+            if (cblk != 0 && cblk->unsecurePointer() == NULL) {
                 cblk.clear();
             }
         }

media/libaudioclient/include/media/IAudioFlinger.h

@@ -70,8 +70,12 @@ public:
                 return DEAD_OBJECT;
             }
             if (parcel->readInt32() != 0) {
+                // TODO: Using unsecurePointer() has some associated security
+                //       pitfalls (see declaration for details).
+                //       Either document why it is safe in this case or address
+                //       the issue (e.g. by copying).
                 sharedBuffer = interface_cast<IMemory>(parcel->readStrongBinder());
-                if (sharedBuffer == 0 || sharedBuffer->pointer() == NULL) {
+                if (sharedBuffer == 0 || sharedBuffer->unsecurePointer() == NULL) {
                     return BAD_VALUE;
                 }
             }
@@ -269,13 +273,21 @@ public:
             (void)parcel->read(&inputId, sizeof(audio_io_handle_t));
             if (parcel->readInt32() != 0) {
                 cblk = interface_cast<IMemory>(parcel->readStrongBinder());
-                if (cblk == 0 || cblk->pointer() == NULL) {
+                // TODO: Using unsecurePointer() has some associated security
+                //       pitfalls (see declaration for details).
+                //       Either document why it is safe in this case or address
+                //       the issue (e.g. by copying).
+                if (cblk == 0 || cblk->unsecurePointer() == NULL) {
                     return BAD_VALUE;
                 }
             }
             if (parcel->readInt32() != 0) {
                 buffers = interface_cast<IMemory>(parcel->readStrongBinder());
-                if (buffers == 0 || buffers->pointer() == NULL) {
+                // TODO: Using unsecurePointer() has some associated security
+                //       pitfalls (see declaration for details).
+                //       Either document why it is safe in this case or address
+                //       the issue (e.g. by copying).
+                if (buffers == 0 || buffers->unsecurePointer() == NULL) {
                     return BAD_VALUE;
                 }
             }

media/libheif/HeifDecoderImpl.cpp

@@ -173,7 +173,7 @@ ssize_t HeifDataSource::readAt(off64_t offset, size_t size) {
 
     // copy from cache if the request falls entirely in cache
     if (offset + size <= mCachedOffset + mCachedSize) {
-        memcpy(mMemory->pointer(), mCache.get() + offset - mCachedOffset, size);
+        memcpy(mMemory->unsecurePointer(), mCache.get() + offset - mCachedOffset, size);
         return size;
     }
 
@@ -271,7 +271,7 @@ ssize_t HeifDataSource::readAt(off64_t offset, size_t size) {
     if (bytesAvailable < (int64_t)size) {
         size = bytesAvailable;
     }
-    memcpy(mMemory->pointer(), mCache.get() + offset - mCachedOffset, size);
+    memcpy(mMemory->unsecurePointer(), mCache.get() + offset - mCachedOffset, size);
     return size;
 }
 
@@ -360,12 +360,16 @@ bool HeifDecoderImpl::init(HeifStream* stream, HeifFrameInfo* frameInfo) {
         sp<IMemory> sharedMem = mRetriever->getImageAtIndex(
                 -1, mOutputColor, true /*metaOnly*/);
 
-        if (sharedMem == nullptr || sharedMem->pointer() == nullptr) {
+        if (sharedMem == nullptr || sharedMem->unsecurePointer() == nullptr) {
             ALOGE("init: videoFrame is a nullptr");
             return false;
         }
 
-        VideoFrame* videoFrame = static_cast<VideoFrame*>(sharedMem->pointer());
+        // TODO: Using unsecurePointer() has some associated security pitfalls
+        //       (see declaration for details).
+        //       Either document why it is safe in this case or address the
+        //       issue (e.g. by copying).
+        VideoFrame* videoFrame = static_cast<VideoFrame*>(sharedMem->unsecurePointer());
 
         ALOGV("Image dimension %dx%d, display %dx%d, angle %d, iccSize %d",
                 videoFrame->mWidth,
@@ -391,12 +395,17 @@ bool HeifDecoderImpl::init(HeifStream* stream, HeifFrameInfo* frameInfo) {
                 MediaSource::ReadOptions::SEEK_PREVIOUS_SYNC,
                 mOutputColor, true /*metaOnly*/);
 
-        if (sharedMem == nullptr || sharedMem->pointer() == nullptr) {
+        if (sharedMem == nullptr || sharedMem->unsecurePointer() == nullptr) {
             ALOGE("init: videoFrame is a nullptr");
             return false;
         }
 
-        VideoFrame* videoFrame = static_cast<VideoFrame*>(sharedMem->pointer());
+        // TODO: Using unsecurePointer() has some associated security pitfalls
+        //       (see declaration for details).
+        //       Either document why it is safe in this case or address the
+        //       issue (e.g. by copying).
+        VideoFrame* videoFrame = static_cast<VideoFrame*>(
+            sharedMem->unsecurePointer());
 
         ALOGV("Sequence dimension %dx%d, display %dx%d, angle %d, iccSize %d",
                 videoFrame->mWidth,
@@ -487,7 +496,7 @@ bool HeifDecoderImpl::decodeAsync() {
         {
             Mutex::Autolock autolock(mLock);
 
-            if (frameMemory == nullptr || frameMemory->pointer() == nullptr) {
+            if (frameMemory == nullptr || frameMemory->unsecurePointer() == nullptr) {
                 mAsyncDecodeDone = true;
                 mScanlineReady.signal();
                 break;
@@ -529,12 +538,16 @@ bool HeifDecoderImpl::decode(HeifFrameInfo* frameInfo) {
             sp<IMemory> frameMemory = mRetriever->getImageRectAtIndex(
                     -1, mOutputColor, 0, 0, mImageInfo.mWidth, mSliceHeight);
 
-            if (frameMemory == nullptr || frameMemory->pointer() == nullptr) {
+            if (frameMemory == nullptr || frameMemory->unsecurePointer() == nullptr) {
                 ALOGE("decode: metadata is a nullptr");
                 return false;
             }
 
-            VideoFrame* videoFrame = static_cast<VideoFrame*>(frameMemory->pointer());
+            // TODO: Using unsecurePointer() has some associated security pitfalls
+            //       (see declaration for details).
+            //       Either document why it is safe in this case or address the
+            //       issue (e.g. by copying).
+            VideoFrame* videoFrame = static_cast<VideoFrame*>(frameMemory->unsecurePointer());
 
             if (frameInfo != nullptr) {
                 initFrameInfo(frameInfo, videoFrame);
@@ -563,12 +576,16 @@ bool HeifDecoderImpl::decode(HeifFrameInfo* frameInfo) {
                 MediaSource::ReadOptions::SEEK_PREVIOUS_SYNC, mOutputColor);
     }
 
-    if (mFrameMemory == nullptr || mFrameMemory->pointer() == nullptr) {
+    if (mFrameMemory == nullptr || mFrameMemory->unsecurePointer() == nullptr) {
         ALOGE("decode: videoFrame is a nullptr");
         return false;
     }
 
-    VideoFrame* videoFrame = static_cast<VideoFrame*>(mFrameMemory->pointer());
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    VideoFrame* videoFrame = static_cast<VideoFrame*>(mFrameMemory->unsecurePointer());
     if (videoFrame->mSize == 0 ||
             mFrameMemory->size() < videoFrame->getFlattenedSize()) {
         ALOGE("decode: videoFrame size is invalid");
@@ -613,12 +630,16 @@ bool HeifDecoderImpl::decodeSequence(int frameIndex, HeifFrameInfo* frameInfo) {
     mTotalScanline = mSequenceInfo.mHeight;
 
     mFrameMemory = mRetriever->getFrameAtIndex(frameIndex, mOutputColor);
-    if (mFrameMemory == nullptr || mFrameMemory->pointer() == nullptr) {
+    if (mFrameMemory == nullptr || mFrameMemory->unsecurePointer() == nullptr) {
         ALOGE("decode: videoFrame is a nullptr");
         return false;
     }
 
-    VideoFrame* videoFrame = static_cast<VideoFrame*>(mFrameMemory->pointer());
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    VideoFrame* videoFrame = static_cast<VideoFrame*>(mFrameMemory->unsecurePointer());
     if (videoFrame->mSize == 0 ||
             mFrameMemory->size() < videoFrame->getFlattenedSize()) {
         ALOGE("decode: videoFrame size is invalid");
@@ -641,10 +662,14 @@ bool HeifDecoderImpl::decodeSequence(int frameIndex, HeifFrameInfo* frameInfo) {
 }
 
 bool HeifDecoderImpl::getScanlineInner(uint8_t* dst) {
-    if (mFrameMemory == nullptr || mFrameMemory->pointer() == nullptr) {
+    if (mFrameMemory == nullptr || mFrameMemory->unsecurePointer() == nullptr) {
         return false;
     }
-    VideoFrame* videoFrame = static_cast<VideoFrame*>(mFrameMemory->pointer());
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    VideoFrame* videoFrame = static_cast<VideoFrame*>(mFrameMemory->unsecurePointer());
     uint8_t* src = videoFrame->getFlattenedData() + videoFrame->mRowBytes * mCurScanline++;
     memcpy(dst, src, videoFrame->mBytesPerPixel * videoFrame->mWidth);
     return true;

media/libmedia/IMediaHTTPConnection.cpp

@@ -128,12 +128,12 @@ struct BpMediaHTTPConnection : public BpInterface<IMediaHTTPConnection> {
            ALOGE("readAt got a NULL buffer");
            return UNKNOWN_ERROR;
         }
-        if (mMemory->pointer() == NULL) {
+        if (mMemory->unsecurePointer() == NULL) {
-           ALOGE("readAt got a NULL mMemory->pointer()");
+           ALOGE("readAt got a NULL mMemory->unsecurePointer()");
            return UNKNOWN_ERROR;
         }
 
-        memcpy(buffer, mMemory->pointer(), len);
+        memcpy(buffer, mMemory->unsecurePointer(), len);
 
         return len;
     }

media/libmediaplayerservice/MetadataRetrieverClient.cpp

@@ -292,7 +292,7 @@ sp<IMemory> MetadataRetrieverClient::extractAlbumArt()
         delete albumArt;
         return NULL;
     }
-    MediaAlbumArt::init((MediaAlbumArt *) mAlbumArt->pointer(),
+    MediaAlbumArt::init((MediaAlbumArt *) mAlbumArt->unsecurePointer(),
                         albumArt->size(), albumArt->data());
     delete albumArt;  // We've taken our copy.
     return mAlbumArt;

media/libmediaplayerservice/nuplayer/NuPlayerStreamListener.cpp

@@ -154,7 +154,7 @@ ssize_t NuPlayer::NuPlayerStreamListener::read(
     }
 
     memcpy(data,
-           (const uint8_t *)mem->pointer()
+           (const uint8_t *)mem->unsecurePointer()
             + entry->mOffset,
            copy);
 

media/libnblog/Reader.cpp

@@ -45,7 +45,12 @@ Reader::Reader(const void *shared, size_t size, const std::string &name)
 }
 
 Reader::Reader(const sp<IMemory>& iMemory, size_t size, const std::string &name)
-    : Reader(iMemory != 0 ? (Shared *) iMemory->pointer() : NULL, size, name)
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    : Reader(iMemory != 0 ? (Shared *) iMemory->unsecurePointer() : NULL, size,
+             name)
 {
     mIMemory = iMemory;
 }
@@ -156,7 +161,8 @@ std::unique_ptr<Snapshot> Reader::getSnapshot(bool flush)
 
 bool Reader::isIMemory(const sp<IMemory>& iMemory) const
 {
-    return iMemory != 0 && mIMemory != 0 && iMemory->pointer() == mIMemory->pointer();
+    return iMemory != 0 && mIMemory != 0 &&
+           iMemory->unsecurePointer() == mIMemory->unsecurePointer();
 }
 
 // We make a set of the invalid types rather than the valid types when aligning

media/libnblog/Writer.cpp

@@ -56,7 +56,11 @@ Writer::Writer(void *shared, size_t size)
 }
 
 Writer::Writer(const sp<IMemory>& iMemory, size_t size)
-    : Writer(iMemory != 0 ? (Shared *) iMemory->pointer() : NULL, size)
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    : Writer(iMemory != 0 ? (Shared *) iMemory->unsecurePointer() : NULL, size)
 {
     mIMemory = iMemory;
 }

media/libstagefright/ACodecBufferChannel.cpp

@@ -153,7 +153,8 @@ status_t ACodecBufferChannel::queueSecureInputBuffer(
         }
 
         if (destination.mType == ICrypto::kDestinationTypeSharedMemory) {
-            memcpy(it->mCodecBuffer->base(), destination.mSharedMemory->pointer(), result);
+            memcpy(it->mCodecBuffer->base(),
+                destination.mSharedMemory->unsecurePointer(), result);
         }
     } else {
         // Here we cast CryptoPlugin::SubSample to hardware::cas::native::V1_0::SubSample
@@ -219,7 +220,8 @@ status_t ACodecBufferChannel::queueSecureInputBuffer(
 
         if (dstBuffer.type == BufferType::SHARED_MEMORY) {
             memcpy(it->mCodecBuffer->base(),
-                    (uint8_t*)it->mSharedEncryptedBuffer->pointer(), result);
+                    (uint8_t*)it->mSharedEncryptedBuffer->unsecurePointer(),
+                    result);
         }
     }
 

media/libstagefright/BufferImpl.cpp

@@ -32,7 +32,11 @@ namespace android {
 // SharedMemoryBuffer
 
 SharedMemoryBuffer::SharedMemoryBuffer(const sp<AMessage> &format, const sp<IMemory> &mem)
-    : MediaCodecBuffer(format, new ABuffer(mem->pointer(), mem->size())),
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
+    : MediaCodecBuffer(format, new ABuffer(mem->unsecurePointer(), mem->size())),
       mMemory(mem) {
 }
 

media/libstagefright/CallbackDataSource.cpp

@@ -81,7 +81,8 @@ ssize_t CallbackDataSource::readAt(off64_t offset, void* data, size_t size) {
             return ERROR_OUT_OF_RANGE;
         }
         CHECK(numRead >= 0 && (size_t)numRead <= bufferSize);
-        memcpy(((uint8_t*)data) + totalNumRead, mMemory->pointer(), numRead);
+        memcpy(((uint8_t*)data) + totalNumRead, mMemory->unsecurePointer(),
+            numRead);
         numLeft -= numRead;
         totalNumRead += numRead;
     }

media/libstagefright/CameraSource.cpp

@@ -89,7 +89,7 @@ void CameraSourceListener::notify(int32_t msgType, int32_t ext1, int32_t ext2) {
 void CameraSourceListener::postData(int32_t msgType, const sp<IMemory> &dataPtr,
                                     camera_frame_metadata_t * /* metadata */) {
     ALOGV("postData(%d, ptr:%p, size:%zu)",
-         msgType, dataPtr->pointer(), dataPtr->size());
+         msgType, dataPtr->unsecurePointer(), dataPtr->size());
 
     sp<CameraSource> source = mSource.promote();
     if (source.get() != NULL) {
@@ -966,8 +966,12 @@ void CameraSource::releaseRecordingFrame(const sp<IMemory>& frame) {
 
         // Check if frame contains a VideoNativeHandleMetadata.
         if (frame->size() == sizeof(VideoNativeHandleMetadata)) {
-            VideoNativeHandleMetadata *metadata =
+          // TODO: Using unsecurePointer() has some associated security pitfalls
-                (VideoNativeHandleMetadata*)(frame->pointer());
+          //       (see declaration for details).
+          //       Either document why it is safe in this case or address the
+          //       issue (e.g. by copying).
+           VideoNativeHandleMetadata *metadata =
+                (VideoNativeHandleMetadata*)(frame->unsecurePointer());
             if (metadata->eType == kMetadataBufferTypeNativeHandleSource) {
                 handle = metadata->pHandle;
             }
@@ -1047,7 +1051,7 @@ void CameraSource::signalBufferReturned(MediaBufferBase *buffer) {
     Mutex::Autolock autoLock(mLock);
     for (List<sp<IMemory> >::iterator it = mFramesBeingEncoded.begin();
          it != mFramesBeingEncoded.end(); ++it) {
-        if ((*it)->pointer() ==  buffer->data()) {
+        if ((*it)->unsecurePointer() ==  buffer->data()) {
             releaseOneRecordingFrame((*it));
             mFramesBeingEncoded.erase(it);
             ++mNumFramesEncoded;
@@ -1102,7 +1106,11 @@ status_t CameraSource::read(
         frameTime = *mFrameTimes.begin();
         mFrameTimes.erase(mFrameTimes.begin());
         mFramesBeingEncoded.push_back(frame);
-        *buffer = new MediaBuffer(frame->pointer(), frame->size());
+        // TODO: Using unsecurePointer() has some associated security pitfalls
+        //       (see declaration for details).
+        //       Either document why it is safe in this case or address the
+        //       issue (e.g. by copying).
+        *buffer = new MediaBuffer(frame->unsecurePointer(), frame->size());
         (*buffer)->setObserver(this);
         (*buffer)->add_ref();
         (*buffer)->meta_data().setInt64(kKeyTime, frameTime);
@@ -1248,7 +1256,7 @@ void CameraSource::recordingFrameHandleCallbackTimestamp(int64_t timestampUs,
     mMemoryBases.erase(mMemoryBases.begin());
 
     // Wrap native handle in sp<IMemory> so it can be pushed to mFramesReceived.
-    VideoNativeHandleMetadata *metadata = (VideoNativeHandleMetadata*)(data->pointer());
+    VideoNativeHandleMetadata *metadata = (VideoNativeHandleMetadata*)(data->unsecurePointer());
     metadata->eType = kMetadataBufferTypeNativeHandleSource;
     metadata->pHandle = handle;
 
@@ -1296,7 +1304,11 @@ void CameraSource::recordingFrameHandleCallbackTimestampBatch(
         mMemoryBases.erase(mMemoryBases.begin());
 
         // Wrap native handle in sp<IMemory> so it can be pushed to mFramesReceived.
-        VideoNativeHandleMetadata *metadata = (VideoNativeHandleMetadata*)(data->pointer());
+        // TODO: Using unsecurePointer() has some associated security pitfalls
+        //       (see declaration for details).
+        //       Either document why it is safe in this case or address the
+        //       issue (e.g. by copying).
+        VideoNativeHandleMetadata *metadata = (VideoNativeHandleMetadata*)(data->unsecurePointer());
         metadata->eType = kMetadataBufferTypeNativeHandleSource;
         metadata->pHandle = handle;
 

media/libstagefright/CameraSourceTimeLapse.cpp

@@ -245,11 +245,11 @@ sp<IMemory> CameraSourceTimeLapse::createIMemoryCopy(
 
     ALOGV("createIMemoryCopy");
     size_t source_size = source_data->size();
-    void* source_pointer = source_data->pointer();
+    void* source_pointer = source_data->unsecurePointer();
 
     sp<MemoryHeapBase> newMemoryHeap = new MemoryHeapBase(source_size);
     sp<MemoryBase> newMemory = new MemoryBase(newMemoryHeap, 0, source_size);
-    memcpy(newMemory->pointer(), source_pointer, source_size);
+    memcpy(newMemory->unsecurePointer(), source_pointer, source_size);
     return newMemory;
 }
 

media/libstagefright/FrameDecoder.cpp

@@ -99,7 +99,7 @@ sp<IMemory> allocVideoFrame(const sp<MetaData>& trackMeta,
         ALOGE("not enough memory for VideoFrame size=%zu", size);
         return NULL;
     }
-    VideoFrame* frameCopy = static_cast<VideoFrame*>(frameMem->pointer());
+    VideoFrame* frameCopy = static_cast<VideoFrame*>(frameMem->unsecurePointer());
     frameCopy->init(frame, iccData, iccSize);
 
     return frameMem;
@@ -206,7 +206,7 @@ sp<IMemory> FrameDecoder::getMetadataOnly(
     // try to fill sequence meta's duration based on average frame rate,
     // default to 33ms if frame rate is unavailable.
     int32_t frameRate;
-    VideoFrame* meta = static_cast<VideoFrame*>(metaMem->pointer());
+    VideoFrame* meta = static_cast<VideoFrame*>(metaMem->unsecurePointer());
     if (trackMeta->findInt32(kKeyFrameRate, &frameRate) && frameRate > 0) {
         meta->mDurationUs = 1000000LL / frameRate;
     } else {
@@ -614,7 +614,7 @@ status_t VideoFrameDecoder::onOutputReceived(
                 0,
                 dstBpp(),
                 mSurfaceControl != nullptr /*allocRotated*/);
-        mFrame = static_cast<VideoFrame*>(frameMem->pointer());
+        mFrame = static_cast<VideoFrame*>(frameMem->unsecurePointer());
 
         setFrame(frameMem);
     }
@@ -895,7 +895,7 @@ status_t ImageDecoder::onOutputReceived(
     if (mFrame == NULL) {
         sp<IMemory> frameMem = allocVideoFrame(
                 trackMeta(), mWidth, mHeight, mTileWidth, mTileHeight, dstBpp());
-        mFrame = static_cast<VideoFrame*>(frameMem->pointer());
+        mFrame = static_cast<VideoFrame*>(frameMem->unsecurePointer());
 
         setFrame(frameMem);
     }

media/libstagefright/StagefrightMediaScanner.cpp

@@ -158,7 +158,11 @@ MediaAlbumArt *StagefrightMediaScanner::extractAlbumArt(int fd) {
     if (mRetriever->setDataSource(fd, 0, size) == OK) {
         sp<IMemory> mem = mRetriever->extractAlbumArt();
         if (mem != NULL) {
-            MediaAlbumArt *art = static_cast<MediaAlbumArt *>(mem->pointer());
+            // TODO: Using unsecurePointer() has some associated security pitfalls
+            //       (see declaration for details).
+            //       Either document why it is safe in this case or address the
+            //       issue (e.g. by copying).
+            MediaAlbumArt *art = static_cast<MediaAlbumArt *>(mem->unsecurePointer());
             return art->clone();
         }
     }

media/libstagefright/foundation/MediaBuffer.cpp

@@ -72,7 +72,7 @@ MediaBuffer::MediaBuffer(size_t size)
             }
         } else {
             getSharedControl()->clear();
-            mData = (uint8_t *)mMemory->pointer() + sizeof(SharedControl);
+            mData = (uint8_t *)mMemory->unsecurePointer() + sizeof(SharedControl);
             ALOGV("Allocated shared mem buffer of size %zu @ %p", size, mData);
         }
     }

media/libstagefright/foundation/MediaBufferGroup.cpp

@@ -75,7 +75,7 @@ void MediaBufferGroup::init(size_t buffers, size_t buffer_size, size_t growthLim
 
         for (size_t i = 0; i < buffers; ++i) {
             sp<IMemory> mem = memoryDealer->allocate(augmented_size);
-            if (mem.get() == nullptr || mem->pointer() == nullptr) {
+            if (mem.get() == nullptr || mem->unsecurePointer() == nullptr) {
                 ALOGW("Only allocated %zu shared buffers of size %zu", i, buffer_size);
                 break;
             }

media/libstagefright/include/media/stagefright/MediaBuffer.h

@@ -48,7 +48,11 @@ public:
     explicit MediaBuffer(const sp<ABuffer> &buffer);
 #ifndef NO_IMEMORY
     MediaBuffer(const sp<IMemory> &mem) :
-        MediaBuffer((uint8_t *)mem->pointer() + sizeof(SharedControl), mem->size()) {
+         // TODO: Using unsecurePointer() has some associated security pitfalls
+         //       (see declaration for details).
+         //       Either document why it is safe in this case or address the
+         //       issue (e.g. by copying).
+        MediaBuffer((uint8_t *)mem->unsecurePointer() + sizeof(SharedControl), mem->size()) {
         // delegate and override mMemory
         mMemory = mem;
     }
@@ -94,9 +98,13 @@ public:
 
     virtual int remoteRefcount() const {
 #ifndef NO_IMEMORY
-        if (mMemory.get() == nullptr || mMemory->pointer() == nullptr) return 0;
+         // TODO: Using unsecurePointer() has some associated security pitfalls
+         //       (see declaration for details).
+         //       Either document why it is safe in this case or address the
+         //       issue (e.g. by copying).
+        if (mMemory.get() == nullptr || mMemory->unsecurePointer() == nullptr) return 0;
         int32_t remoteRefcount =
-                reinterpret_cast<SharedControl *>(mMemory->pointer())->getRemoteRefcount();
+                reinterpret_cast<SharedControl *>(mMemory->unsecurePointer())->getRemoteRefcount();
         // Sanity check so that remoteRefCount() is non-negative.
         return remoteRefcount >= 0 ? remoteRefcount : 0; // do not allow corrupted data.
 #else
@@ -107,8 +115,12 @@ public:
     // returns old value
     int addRemoteRefcount(int32_t value) {
 #ifndef NO_IMEMORY
-        if (mMemory.get() == nullptr || mMemory->pointer() == nullptr) return 0;
+          // TODO: Using unsecurePointer() has some associated security pitfalls
-        return reinterpret_cast<SharedControl *>(mMemory->pointer())->addRemoteRefcount(value);
+         //       (see declaration for details).
+         //       Either document why it is safe in this case or address the
+         //       issue (e.g. by copying).
+       if (mMemory.get() == nullptr || mMemory->unsecurePointer() == nullptr) return 0;
+        return reinterpret_cast<SharedControl *>(mMemory->unsecurePointer())->addRemoteRefcount(value);
 #else
         (void) value;
         return 0;
@@ -121,8 +133,12 @@ public:
 
     static bool isDeadObject(const sp<IMemory> &memory) {
 #ifndef NO_IMEMORY
-        if (memory.get() == nullptr || memory->pointer() == nullptr) return false;
+         // TODO: Using unsecurePointer() has some associated security pitfalls
-        return reinterpret_cast<SharedControl *>(memory->pointer())->isDeadObject();
+         //       (see declaration for details).
+         //       Either document why it is safe in this case or address the
+         //       issue (e.g. by copying).
+        if (memory.get() == nullptr || memory->unsecurePointer() == nullptr) return false;
+        return reinterpret_cast<SharedControl *>(memory->unsecurePointer())->isDeadObject();
 #else
         (void) memory;
         return false;
@@ -220,7 +236,11 @@ private:
 
     inline SharedControl *getSharedControl() const {
 #ifndef NO_IMEMORY
-         return reinterpret_cast<SharedControl *>(mMemory->pointer());
+         // TODO: Using unsecurePointer() has some associated security pitfalls
+         //       (see declaration for details).
+         //       Either document why it is safe in this case or address the
+         //       issue (e.g. by copying).
+         return reinterpret_cast<SharedControl *>(mMemory->unsecurePointer());
 #else
          return nullptr;
 #endif

media/libstagefright/include/media/stagefright/RemoteDataSource.h

@@ -48,7 +48,7 @@ public:
         if (size > kBufferSize) {
             size = kBufferSize;
         }
-        return mSource->readAt(offset, mMemory->pointer(), size);
+        return mSource->readAt(offset, mMemory->unsecurePointer(), size);
     }
     virtual status_t getSize(off64_t *size) {
         return mSource->getSize(size);

media/libstagefright/omx/OMXNodeInstance.cpp

@@ -128,7 +128,7 @@ struct BufferMeta {
     }
 
     OMX_U8 *getPointer() {
-        return mMem.get() ? static_cast<OMX_U8*>(mMem->pointer()) :
+        return mMem.get() ? static_cast<OMX_U8*>(mMem->unsecurePointer()) :
                 mHidlMemory.get() ? static_cast<OMX_U8*>(
                 static_cast<void*>(mHidlMemory->getPointer())) : nullptr;
     }
@@ -1173,7 +1173,11 @@ status_t OMXNodeInstance::useBuffer_l(
         return BAD_VALUE;
     }
     if (params != NULL) {
-        paramsPointer = params->pointer();
+        // TODO: Using unsecurePointer() has some associated security pitfalls
+        //       (see declaration for details).
+        //       Either document why it is safe in this case or address the
+        //       issue (e.g. by copying).
+        paramsPointer = params->unsecurePointer();
         paramsSize = params->size();
     } else if (hParams != NULL) {
         paramsPointer = hParams->getPointer();

media/utils/ServiceUtilities.cpp

@@ -222,9 +222,9 @@ status_t checkIMemory(const sp<IMemory>& iMemory)
     off_t size = lseek(heap->getHeapID(), 0, SEEK_END);
     lseek(heap->getHeapID(), 0, SEEK_SET);
 
-    if (iMemory->pointer() == NULL || size < (off_t)iMemory->size()) {
+    if (iMemory->unsecurePointer() == NULL || size < (off_t)iMemory->size()) {
         ALOGE("%s check failed: pointer %p size %zu fd size %u",
-              __FUNCTION__, iMemory->pointer(), iMemory->size(), (uint32_t)size);
+              __FUNCTION__, iMemory->unsecurePointer(), iMemory->size(), (uint32_t)size);
         return BAD_VALUE;
     }
 

services/audioflinger/AudioFlinger.cpp

@@ -652,7 +652,7 @@ sp<NBLog::Writer> AudioFlinger::newWriter_l(size_t size, const char *name)
         return new NBLog::Writer();
     }
 success:
-    NBLog::Shared *sharedRawPtr = (NBLog::Shared *) shared->pointer();
+    NBLog::Shared *sharedRawPtr = (NBLog::Shared *) shared->unsecurePointer();
     new((void *) sharedRawPtr) NBLog::Shared(); // placement new here, but the corresponding
                                                 // explicit destructor not needed since it is POD
     sMediaLogService->registerWriter(shared, size, name);

services/audioflinger/Effects.cpp

@@ -1588,7 +1588,7 @@ AudioFlinger::EffectHandle::EffectHandle(const sp<EffectModule>& effect,
     int bufOffset = ((sizeof(effect_param_cblk_t) - 1) / sizeof(int) + 1) * sizeof(int);
     mCblkMemory = client->heap()->allocate(EFFECT_PARAM_BUFFER_SIZE + bufOffset);
     if (mCblkMemory == 0 ||
-            (mCblk = static_cast<effect_param_cblk_t *>(mCblkMemory->pointer())) == NULL) {
+            (mCblk = static_cast<effect_param_cblk_t *>(mCblkMemory->unsecurePointer())) == NULL) {
         ALOGE("not enough memory for Effect size=%zu", EFFECT_PARAM_BUFFER_SIZE +
                 sizeof(effect_param_cblk_t));
         mCblkMemory.clear();

services/audioflinger/Threads.cpp

@@ -2079,9 +2079,9 @@ sp<AudioFlinger::PlaybackThread::Track> AudioFlinger::PlaybackThread::createTrac
             // More than 2 channels does not require stronger alignment than stereo
             alignment <<= 1;
         }
-        if (((uintptr_t)sharedBuffer->pointer() & (alignment - 1)) != 0) {
+        if (((uintptr_t)sharedBuffer->unsecurePointer() & (alignment - 1)) != 0) {
             ALOGE("Invalid buffer alignment: address %p, channel count %u",
-                  sharedBuffer->pointer(), channelCount);
+                  sharedBuffer->unsecurePointer(), channelCount);
             lStatus = BAD_VALUE;
             goto Exit;
         }
@@ -6756,7 +6756,7 @@ AudioFlinger::RecordThread::RecordThread(const sp<AudioFlinger>& audioFlinger,
         sp<IMemory> pipeMemory;
         if ((roHeap == 0) ||
                 (pipeMemory = roHeap->allocate(pipeSize)) == 0 ||
-                (pipeBuffer = pipeMemory->pointer()) == nullptr) {
+                (pipeBuffer = pipeMemory->unsecurePointer()) == nullptr) {
             ALOGE("not enough memory for pipe buffer size=%zu; "
                     "roHeap=%p, pipeMemory=%p, pipeBuffer=%p; roHeapSize: %lld",
                     pipeSize, roHeap.get(), pipeMemory.get(), pipeBuffer,

services/audioflinger/Tracks.cpp

@@ -150,7 +150,7 @@ AudioFlinger::ThreadBase::TrackBase::TrackBase(
     if (client != 0) {
         mCblkMemory = client->heap()->allocate(size);
         if (mCblkMemory == 0 ||
-                (mCblk = static_cast<audio_track_cblk_t *>(mCblkMemory->pointer())) == NULL) {
+                (mCblk = static_cast<audio_track_cblk_t *>(mCblkMemory->unsecurePointer())) == NULL) {
             ALOGE("%s(%d): not enough memory for AudioTrack size=%zu", __func__, mId, size);
             client->heap()->dump("AudioTrack");
             mCblkMemory.clear();
@@ -172,7 +172,7 @@ AudioFlinger::ThreadBase::TrackBase::TrackBase(
             const sp<MemoryDealer> roHeap(thread->readOnlyHeap());
             if (roHeap == 0 ||
                     (mBufferMemory = roHeap->allocate(bufferSize)) == 0 ||
-                    (mBuffer = mBufferMemory->pointer()) == NULL) {
+                    (mBuffer = mBufferMemory->unsecurePointer()) == NULL) {
                 ALOGE("%s(%d): not enough memory for read-only buffer size=%zu",
                         __func__, mId, bufferSize);
                 if (roHeap != 0) {
@@ -187,7 +187,7 @@ AudioFlinger::ThreadBase::TrackBase::TrackBase(
         case ALLOC_PIPE:
             mBufferMemory = thread->pipeMemory();
             // mBuffer is the virtual address as seen from current process (mediaserver),
-            // and should normally be coming from mBufferMemory->pointer().
+            // and should normally be coming from mBufferMemory->unsecurePointer().
             // However in this case the TrackBase does not reference the buffer directly.
             // It should references the buffer via the pipe.
             // Therefore, to detect incorrect usage of the buffer, we set mBuffer to NULL.
@@ -510,7 +510,11 @@ AudioFlinger::PlaybackThread::Track::Track(
             track_type type,
             audio_port_handle_t portId)
     :   TrackBase(thread, client, attr, sampleRate, format, channelMask, frameCount,
-                  (sharedBuffer != 0) ? sharedBuffer->pointer() : buffer,
+                  // TODO: Using unsecurePointer() has some associated security pitfalls
+                  //       (see declaration for details).
+                  //       Either document why it is safe in this case or address the
+                  //       issue (e.g. by copying).
+                  (sharedBuffer != 0) ? sharedBuffer->unsecurePointer() : buffer,
                   (sharedBuffer != 0) ? sharedBuffer->size() : bufferSize,
                   sessionId, creatorPid, uid, true /*isOut*/,
                   (type == TYPE_PATCH) ? ( buffer == NULL ? ALLOC_LOCAL : ALLOC_NONE) : ALLOC_CBLK,
@@ -540,7 +544,7 @@ AudioFlinger::PlaybackThread::Track::Track(
     ALOG_ASSERT(!(client == 0 && sharedBuffer != 0));
 
     ALOGV_IF(sharedBuffer != 0, "%s(%d): sharedBuffer: %p, size: %zu",
-            __func__, mId, sharedBuffer->pointer(), sharedBuffer->size());
+            __func__, mId, sharedBuffer->unsecurePointer(), sharedBuffer->size());
 
     if (mCblk == NULL) {
         return;

services/camera/libcameraservice/api1/CameraClient.cpp

@@ -534,7 +534,7 @@ void CameraClient::releaseRecordingFrameHandle(native_handle_t *handle) {
     }
 
     if (mHardware != nullptr) {
-        VideoNativeHandleMetadata *metadata = (VideoNativeHandleMetadata*)(dataPtr->pointer());
+        VideoNativeHandleMetadata *metadata = (VideoNativeHandleMetadata*)(dataPtr->unsecurePointer());
         metadata->eType = kMetadataBufferTypeNativeHandleSource;
         metadata->pHandle = handle;
         mHardware->releaseRecordingFrame(dataPtr);
@@ -573,7 +573,7 @@ void CameraClient::releaseRecordingFrameHandleBatch(const std::vector<native_han
         }
 
         if (!disconnected) {
-            VideoNativeHandleMetadata *metadata = (VideoNativeHandleMetadata*)(dataPtr->pointer());
+            VideoNativeHandleMetadata *metadata = (VideoNativeHandleMetadata*)(dataPtr->unsecurePointer());
             metadata->eType = kMetadataBufferTypeNativeHandleSource;
             metadata->pHandle = handle;
             frames.push_back(dataPtr);
@@ -916,8 +916,12 @@ void CameraClient::handleCallbackTimestampBatch(
                 ALOGE("%s: dataPtr does not contain VideoNativeHandleMetadata!", __FUNCTION__);
                 return;
             }
+            // TODO: Using unsecurePointer() has some associated security pitfalls
+            //       (see declaration for details).
+            //       Either document why it is safe in this case or address the
+            //       issue (e.g. by copying).
             VideoNativeHandleMetadata *metadata =
-                (VideoNativeHandleMetadata*)(msg.dataPtr->pointer());
+                (VideoNativeHandleMetadata*)(msg.dataPtr->unsecurePointer());
             if (metadata->eType == kMetadataBufferTypeNativeHandleSource) {
                 handle = metadata->pHandle;
             }
@@ -1073,8 +1077,12 @@ void CameraClient::handleGenericDataTimestamp(nsecs_t timestamp,
 
         // Check if dataPtr contains a VideoNativeHandleMetadata.
         if (dataPtr->size() == sizeof(VideoNativeHandleMetadata)) {
+            // TODO: Using unsecurePointer() has some associated security pitfalls
+            //       (see declaration for details).
+            //       Either document why it is safe in this case or address the
+            //       issue (e.g. by copying).
             VideoNativeHandleMetadata *metadata =
-                (VideoNativeHandleMetadata*)(dataPtr->pointer());
+                (VideoNativeHandleMetadata*)(dataPtr->unsecurePointer());
             if (metadata->eType == kMetadataBufferTypeNativeHandleSource) {
                 handle = metadata->pHandle;
             }

services/camera/libcameraservice/device1/CameraHardwareInterface.cpp

@@ -165,8 +165,12 @@ hardware::Return<void> CameraHardwareInterface::handleCallbackTimestamp(
         mem = mHidlMemPoolMap.at(data);
     }
     sp<CameraHeapMemory> heapMem(static_cast<CameraHeapMemory *>(mem->handle));
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
     VideoNativeHandleMetadata* md = (VideoNativeHandleMetadata*)
-            heapMem->mBuffers[bufferIndex]->pointer();
+            heapMem->mBuffers[bufferIndex]->unsecurePointer();
     md->pHandle = const_cast<native_handle_t*>(frameData.getNativeHandle());
     sDataCbTimestamp(timestamp, (int32_t) msgType, mem, bufferIndex, this);
     return hardware::Void();
@@ -192,8 +196,12 @@ hardware::Return<void> CameraHardwareInterface::handleCallbackTimestampBatch(
                      hidl_msg.bufferIndex, mem->mNumBufs);
                 return hardware::Void();
             }
+            // TODO: Using unsecurePointer() has some associated security pitfalls
+            //       (see declaration for details).
+            //       Either document why it is safe in this case or address the
+            //       issue (e.g. by copying).
             VideoNativeHandleMetadata* md = (VideoNativeHandleMetadata*)
-                    mem->mBuffers[hidl_msg.bufferIndex]->pointer();
+                    mem->mBuffers[hidl_msg.bufferIndex]->unsecurePointer();
             md->pHandle = const_cast<native_handle_t*>(hidl_msg.frameData.getNativeHandle());
 
             msgs.push_back({hidl_msg.timestamp, mem->mBuffers[hidl_msg.bufferIndex]});
@@ -578,7 +586,11 @@ void CameraHardwareInterface::releaseRecordingFrame(const sp<IMemory>& mem)
     int bufferIndex = offset / size;
     if (CC_LIKELY(mHidlDevice != nullptr)) {
         if (size == sizeof(VideoNativeHandleMetadata)) {
-            VideoNativeHandleMetadata* md = (VideoNativeHandleMetadata*) mem->pointer();
+            // TODO: Using unsecurePointer() has some associated security pitfalls
+            //       (see declaration for details).
+            //       Either document why it is safe in this case or address the
+            //       issue (e.g. by copying).
+            VideoNativeHandleMetadata* md = (VideoNativeHandleMetadata*) mem->unsecurePointer();
             // Caching the handle here because md->pHandle will be subject to HAL's edit
             native_handle_t* nh = md->pHandle;
             hidl_handle frame = nh;
@@ -605,7 +617,11 @@ void CameraHardwareInterface::releaseRecordingFrameBatch(const std::vector<sp<IM
             if (size == sizeof(VideoNativeHandleMetadata)) {
                 uint32_t heapId = heap->getHeapID();
                 uint32_t bufferIndex = offset / size;
-                VideoNativeHandleMetadata* md = (VideoNativeHandleMetadata*) mem->pointer();
+                // TODO: Using unsecurePointer() has some associated security pitfalls
+                //       (see declaration for details).
+                //       Either document why it is safe in this case or address the
+                //       issue (e.g. by copying).
+                VideoNativeHandleMetadata* md = (VideoNativeHandleMetadata*) mem->unsecurePointer();
                 // Caching the handle here because md->pHandle will be subject to HAL's edit
                 native_handle_t* nh = md->pHandle;
                 VideoFrameMessage msg;

services/soundtrigger/SoundTriggerHwService.cpp

@@ -234,11 +234,11 @@ sp<IMemory> SoundTriggerHwService::prepareRecognitionEvent(
 
     size_t size = event->data_offset + event->data_size;
     eventMemory = mMemoryDealer->allocate(size);
-    if (eventMemory == 0 || eventMemory->pointer() == NULL) {
+    if (eventMemory == 0 || eventMemory->unsecurePointer() == NULL) {
         eventMemory.clear();
         return eventMemory;
     }
-    memcpy(eventMemory->pointer(), event, size);
+    memcpy(eventMemory->unsecurePointer(), event, size);
 
     return eventMemory;
 }
@@ -283,11 +283,11 @@ sp<IMemory> SoundTriggerHwService::prepareSoundModelEvent(struct sound_trigger_m
 
     size_t size = event->data_offset + event->data_size;
     eventMemory = mMemoryDealer->allocate(size);
-    if (eventMemory == 0 || eventMemory->pointer() == NULL) {
+    if (eventMemory == 0 || eventMemory->unsecurePointer() == NULL) {
         eventMemory.clear();
         return eventMemory;
     }
-    memcpy(eventMemory->pointer(), event, size);
+    memcpy(eventMemory->unsecurePointer(), event, size);
 
     return eventMemory;
 }
@@ -313,11 +313,11 @@ sp<IMemory> SoundTriggerHwService::prepareServiceStateEvent(sound_trigger_servic
 
     size_t size = sizeof(sound_trigger_service_state_t);
     eventMemory = mMemoryDealer->allocate(size);
-    if (eventMemory == 0 || eventMemory->pointer() == NULL) {
+    if (eventMemory == 0 || eventMemory->unsecurePointer() == NULL) {
         eventMemory.clear();
         return eventMemory;
     }
-    *((sound_trigger_service_state_t *)eventMemory->pointer()) = state;
+    *((sound_trigger_service_state_t *)eventMemory->unsecurePointer()) = state;
     return eventMemory;
 }
 
@@ -557,8 +557,12 @@ status_t SoundTriggerHwService::Module::loadSoundModel(const sp<IMemory>& modelM
         return NO_INIT;
     }
 
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
     struct sound_trigger_sound_model *sound_model =
-            (struct sound_trigger_sound_model *)modelMemory->pointer();
+            (struct sound_trigger_sound_model *)modelMemory->unsecurePointer();
 
     size_t structSize;
     if (sound_model->type == SOUND_MODEL_TYPE_KEYPHRASE) {
@@ -651,8 +655,12 @@ status_t SoundTriggerHwService::Module::startRecognition(sound_model_handle_t ha
         return NO_INIT;
     }
 
+    // TODO: Using unsecurePointer() has some associated security pitfalls
+    //       (see declaration for details).
+    //       Either document why it is safe in this case or address the
+    //       issue (e.g. by copying).
     struct sound_trigger_recognition_config *config =
-            (struct sound_trigger_recognition_config *)dataMemory->pointer();
+            (struct sound_trigger_recognition_config *)dataMemory->unsecurePointer();
 
     if (config->data_offset < sizeof(struct sound_trigger_recognition_config) ||
             config->data_size > (UINT_MAX - config->data_offset) ||
@@ -734,9 +742,10 @@ void SoundTriggerHwService::Module::onCallbackEvent(const sp<CallbackEvent>& eve
 {
     ALOGV("onCallbackEvent type %d", event->mType);
 
+    // Memory is coming from a trusted process.
     sp<IMemory> eventMemory = event->mMemory;
 
-    if (eventMemory == 0 || eventMemory->pointer() == NULL) {
+    if (eventMemory == 0 || eventMemory->unsecurePointer() == NULL) {
         return;
     }
     if (mModuleClients.isEmpty()) {
@@ -749,7 +758,7 @@ void SoundTriggerHwService::Module::onCallbackEvent(const sp<CallbackEvent>& eve
     switch (event->mType) {
     case CallbackEvent::TYPE_RECOGNITION: {
         struct sound_trigger_recognition_event *recognitionEvent =
-                (struct sound_trigger_recognition_event *)eventMemory->pointer();
+                (struct sound_trigger_recognition_event *)eventMemory->unsecurePointer();
         {
             AutoMutex lock(mLock);
             sp<Model> model = getModel(recognitionEvent->model);
@@ -769,7 +778,7 @@ void SoundTriggerHwService::Module::onCallbackEvent(const sp<CallbackEvent>& eve
     } break;
     case CallbackEvent::TYPE_SOUNDMODEL: {
         struct sound_trigger_model_event *soundmodelEvent =
-                (struct sound_trigger_model_event *)eventMemory->pointer();
+                (struct sound_trigger_model_event *)eventMemory->unsecurePointer();
         {
             AutoMutex lock(mLock);
             sp<Model> model = getModel(soundmodelEvent->model);
@@ -1082,7 +1091,8 @@ void SoundTriggerHwService::ModuleClient::onCallbackEvent(const sp<CallbackEvent
 
     sp<IMemory> eventMemory = event->mMemory;
 
-    if (eventMemory == 0 || eventMemory->pointer() == NULL) {
+    // Memory is coming from a trusted process.
+    if (eventMemory == 0 || eventMemory->unsecurePointer() == NULL) {
         return;
     }
 

soundtrigger/SoundTrigger.cpp

@@ -204,39 +204,42 @@ status_t SoundTrigger::getModelState(sound_model_handle_t handle)
 void SoundTrigger::onRecognitionEvent(const sp<IMemory>& eventMemory)
 {
     Mutex::Autolock _l(mLock);
-    if (eventMemory == 0 || eventMemory->pointer() == NULL) {
+    if (eventMemory == 0 || eventMemory->unsecurePointer() == NULL) {
         return;
     }
 
     if (mCallback != 0) {
+        // Memory is coming from a trusted process.
         mCallback->onRecognitionEvent(
-                (struct sound_trigger_recognition_event *)eventMemory->pointer());
+                (struct sound_trigger_recognition_event *)eventMemory->unsecurePointer());
     }
 }
 
 void SoundTrigger::onSoundModelEvent(const sp<IMemory>& eventMemory)
 {
     Mutex::Autolock _l(mLock);
-    if (eventMemory == 0 || eventMemory->pointer() == NULL) {
+    if (eventMemory == 0 || eventMemory->unsecurePointer() == NULL) {
         return;
     }
 
     if (mCallback != 0) {
+        // Memory is coming from a trusted process.
         mCallback->onSoundModelEvent(
-                (struct sound_trigger_model_event *)eventMemory->pointer());
+                (struct sound_trigger_model_event *)eventMemory->unsecurePointer());
     }
 }
 
 void SoundTrigger::onServiceStateChange(const sp<IMemory>& eventMemory)
 {
     Mutex::Autolock _l(mLock);
-    if (eventMemory == 0 || eventMemory->pointer() == NULL) {
+    if (eventMemory == 0 || eventMemory->unsecurePointer() == NULL) {
         return;
     }
 
     if (mCallback != 0) {
+        // Memory is coming from a trusted process.
         mCallback->onServiceStateChange(
-                *((sound_trigger_service_state_t *)eventMemory->pointer()));
+                *((sound_trigger_service_state_t *)eventMemory->unsecurePointer()));
     }
 }