From 8d75246dcb76d2427a3931802b94bc8c538100c9 Mon Sep 17 00:00:00 2001 From: Sungtak Lee Date: Sat, 2 Mar 2019 16:40:47 -0800 Subject: RESTRICT AUTOMERGE BQ: retain buffer drop from BufferQueueProducer Retain buffer drop from BufferQueueProducer::queueBuffer unless dequeue timeout is set positive. Bug: 122433957 Change-Id: I8432a7ad386836498e632c67953ad49c6be008bb (cherry picked from commit fe8713cbb23b1a3f543ed18ce85ddfaecf513831) --- libs/gui/BufferQueueCore.cpp | 1 + libs/gui/BufferQueueProducer.cpp | 16 +++++++++++----- libs/gui/include/gui/BufferQueueCore.h | 5 +++++ libs/gui/include/gui/IGraphicBufferProducer.h | 4 ++++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp index bb703da3dd..7dddd8b2c4 100644 --- a/libs/gui/BufferQueueCore.cpp +++ b/libs/gui/BufferQueueCore.cpp @@ -73,6 +73,7 @@ BufferQueueCore::BufferQueueCore() : mActiveBuffers(), mDequeueCondition(), mDequeueBufferCannotBlock(false), + mQueueBufferCanDrop(false), mDefaultBufferFormat(PIXEL_FORMAT_RGBA_8888), mDefaultWidth(1), mDefaultHeight(1), diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index c96a2dd6a3..c01b2b6598 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -872,7 +872,7 @@ status_t BufferQueueProducer::queueBuffer(int slot, item.mFence = acquireFence; item.mFenceTime = acquireFenceTime; item.mIsDroppable = mCore->mAsyncMode || - mCore->mDequeueBufferCannotBlock || + mCore->mQueueBufferCanDrop || (mCore->mSharedBufferMode && mCore->mSharedBufferSlot == slot); item.mSurfaceDamage = surfaceDamage; item.mQueuedBuffer = true; @@ -1213,9 +1213,10 @@ status_t BufferQueueProducer::connect(const sp& listener, mCore->mConnectedPid = IPCThreadState::self()->getCallingPid(); mCore->mBufferHasBeenQueued = false; mCore->mDequeueBufferCannotBlock = false; - if (mDequeueTimeout < 0) { - mCore->mDequeueBufferCannotBlock = - mCore->mConsumerControlledByApp && producerControlledByApp; + mCore->mQueueBufferCanDrop = false; + if (mCore->mConsumerControlledByApp && producerControlledByApp) { + mCore->mDequeueBufferCannotBlock = mDequeueTimeout < 0; + mCore->mQueueBufferCanDrop = mDequeueTimeout <= 0; } mCore->mAllowAllocation = true; @@ -1486,7 +1487,12 @@ status_t BufferQueueProducer::setDequeueTimeout(nsecs_t timeout) { } mDequeueTimeout = timeout; - mCore->mDequeueBufferCannotBlock = false; + if (timeout >= 0) { + mCore->mDequeueBufferCannotBlock = false; + if (timeout != 0) { + mCore->mQueueBufferCanDrop = false; + } + } VALIDATE_CONSISTENCY(); return NO_ERROR; diff --git a/libs/gui/include/gui/BufferQueueCore.h b/libs/gui/include/gui/BufferQueueCore.h index 537c957746..f91babca11 100644 --- a/libs/gui/include/gui/BufferQueueCore.h +++ b/libs/gui/include/gui/BufferQueueCore.h @@ -225,6 +225,11 @@ private: // consumer are controlled by the application. bool mDequeueBufferCannotBlock; + // mQueueBufferCanDrop indicates whether queueBuffer is allowd to drop + // buffers in non-async mode. This flag is set during connect when both the + // producer and consumer are controlled by application. + bool mQueueBufferCanDrop; + // mDefaultBufferFormat can be set so it will override the buffer format // when it isn't specified in dequeueBuffer. PixelFormat mDefaultBufferFormat; diff --git a/libs/gui/include/gui/IGraphicBufferProducer.h b/libs/gui/include/gui/IGraphicBufferProducer.h index 887654e05b..2f8a154faa 100644 --- a/libs/gui/include/gui/IGraphicBufferProducer.h +++ b/libs/gui/include/gui/IGraphicBufferProducer.h @@ -584,6 +584,10 @@ public: // non-blocking mode and its corresponding spare buffer (which is used to // ensure a buffer is always available). // + // N.B. queueBuffer will stop buffer dropping behavior if timeout is + // strictly positive. If timeout is zero or negative, previous buffer + // dropping behavior will not be changed. + // // Return of a value other than NO_ERROR means an error has occurred: // * BAD_VALUE - Failure to adjust the number of available slots. This can // happen because of trying to allocate/deallocate the async -- cgit v1.2.3-59-g8ed1b From cf4ab6166d76f2cb015d0fc651e189f216fa8ab6 Mon Sep 17 00:00:00 2001 From: Sungtak Lee Date: Sat, 2 Mar 2019 16:40:47 -0800 Subject: RESTRICT AUTOMERGE BQ: retain buffer drop from BufferQueueProducer Retain buffer drop from BufferQueueProducer::queueBuffer unless dequeue timeout is set positive. Bug: 122433957 Change-Id: I8432a7ad386836498e632c67953ad49c6be008bb (cherry picked from commit fe8713cbb23b1a3f543ed18ce85ddfaecf513831) --- libs/gui/BufferQueueCore.cpp | 1 + libs/gui/BufferQueueProducer.cpp | 16 +++++++++++----- libs/gui/include/gui/BufferQueueCore.h | 5 +++++ libs/gui/include/gui/IGraphicBufferProducer.h | 4 ++++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp index bb703da3dd..7dddd8b2c4 100644 --- a/libs/gui/BufferQueueCore.cpp +++ b/libs/gui/BufferQueueCore.cpp @@ -73,6 +73,7 @@ BufferQueueCore::BufferQueueCore() : mActiveBuffers(), mDequeueCondition(), mDequeueBufferCannotBlock(false), + mQueueBufferCanDrop(false), mDefaultBufferFormat(PIXEL_FORMAT_RGBA_8888), mDefaultWidth(1), mDefaultHeight(1), diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index c96a2dd6a3..c01b2b6598 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -872,7 +872,7 @@ status_t BufferQueueProducer::queueBuffer(int slot, item.mFence = acquireFence; item.mFenceTime = acquireFenceTime; item.mIsDroppable = mCore->mAsyncMode || - mCore->mDequeueBufferCannotBlock || + mCore->mQueueBufferCanDrop || (mCore->mSharedBufferMode && mCore->mSharedBufferSlot == slot); item.mSurfaceDamage = surfaceDamage; item.mQueuedBuffer = true; @@ -1213,9 +1213,10 @@ status_t BufferQueueProducer::connect(const sp& listener, mCore->mConnectedPid = IPCThreadState::self()->getCallingPid(); mCore->mBufferHasBeenQueued = false; mCore->mDequeueBufferCannotBlock = false; - if (mDequeueTimeout < 0) { - mCore->mDequeueBufferCannotBlock = - mCore->mConsumerControlledByApp && producerControlledByApp; + mCore->mQueueBufferCanDrop = false; + if (mCore->mConsumerControlledByApp && producerControlledByApp) { + mCore->mDequeueBufferCannotBlock = mDequeueTimeout < 0; + mCore->mQueueBufferCanDrop = mDequeueTimeout <= 0; } mCore->mAllowAllocation = true; @@ -1486,7 +1487,12 @@ status_t BufferQueueProducer::setDequeueTimeout(nsecs_t timeout) { } mDequeueTimeout = timeout; - mCore->mDequeueBufferCannotBlock = false; + if (timeout >= 0) { + mCore->mDequeueBufferCannotBlock = false; + if (timeout != 0) { + mCore->mQueueBufferCanDrop = false; + } + } VALIDATE_CONSISTENCY(); return NO_ERROR; diff --git a/libs/gui/include/gui/BufferQueueCore.h b/libs/gui/include/gui/BufferQueueCore.h index 537c957746..f91babca11 100644 --- a/libs/gui/include/gui/BufferQueueCore.h +++ b/libs/gui/include/gui/BufferQueueCore.h @@ -225,6 +225,11 @@ private: // consumer are controlled by the application. bool mDequeueBufferCannotBlock; + // mQueueBufferCanDrop indicates whether queueBuffer is allowd to drop + // buffers in non-async mode. This flag is set during connect when both the + // producer and consumer are controlled by application. + bool mQueueBufferCanDrop; + // mDefaultBufferFormat can be set so it will override the buffer format // when it isn't specified in dequeueBuffer. PixelFormat mDefaultBufferFormat; diff --git a/libs/gui/include/gui/IGraphicBufferProducer.h b/libs/gui/include/gui/IGraphicBufferProducer.h index 887654e05b..2f8a154faa 100644 --- a/libs/gui/include/gui/IGraphicBufferProducer.h +++ b/libs/gui/include/gui/IGraphicBufferProducer.h @@ -584,6 +584,10 @@ public: // non-blocking mode and its corresponding spare buffer (which is used to // ensure a buffer is always available). // + // N.B. queueBuffer will stop buffer dropping behavior if timeout is + // strictly positive. If timeout is zero or negative, previous buffer + // dropping behavior will not be changed. + // // Return of a value other than NO_ERROR means an error has occurred: // * BAD_VALUE - Failure to adjust the number of available slots. This can // happen because of trying to allocate/deallocate the async -- cgit v1.2.3-59-g8ed1b From 8f7b62748e4a0569e4f2d4d8101b377595e1ccf0 Mon Sep 17 00:00:00 2001 From: Sungtak Lee Date: Sat, 2 Mar 2019 16:40:47 -0800 Subject: RESTRICT AUTOMERGE BQ: retain buffer drop from BufferQueueProducer Retain buffer drop from BufferQueueProducer::queueBuffer unless dequeue timeout is set positive. Bug: 122433957 Change-Id: I8432a7ad386836498e632c67953ad49c6be008bb (cherry picked from commit fe8713cbb23b1a3f543ed18ce85ddfaecf513831) --- libs/gui/BufferQueueCore.cpp | 1 + libs/gui/BufferQueueProducer.cpp | 16 +++++++++++----- libs/gui/include/gui/BufferQueueCore.h | 5 +++++ libs/gui/include/gui/IGraphicBufferProducer.h | 4 ++++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp index bb703da3dd..7dddd8b2c4 100644 --- a/libs/gui/BufferQueueCore.cpp +++ b/libs/gui/BufferQueueCore.cpp @@ -73,6 +73,7 @@ BufferQueueCore::BufferQueueCore() : mActiveBuffers(), mDequeueCondition(), mDequeueBufferCannotBlock(false), + mQueueBufferCanDrop(false), mDefaultBufferFormat(PIXEL_FORMAT_RGBA_8888), mDefaultWidth(1), mDefaultHeight(1), diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index c96a2dd6a3..c01b2b6598 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -872,7 +872,7 @@ status_t BufferQueueProducer::queueBuffer(int slot, item.mFence = acquireFence; item.mFenceTime = acquireFenceTime; item.mIsDroppable = mCore->mAsyncMode || - mCore->mDequeueBufferCannotBlock || + mCore->mQueueBufferCanDrop || (mCore->mSharedBufferMode && mCore->mSharedBufferSlot == slot); item.mSurfaceDamage = surfaceDamage; item.mQueuedBuffer = true; @@ -1213,9 +1213,10 @@ status_t BufferQueueProducer::connect(const sp& listener, mCore->mConnectedPid = IPCThreadState::self()->getCallingPid(); mCore->mBufferHasBeenQueued = false; mCore->mDequeueBufferCannotBlock = false; - if (mDequeueTimeout < 0) { - mCore->mDequeueBufferCannotBlock = - mCore->mConsumerControlledByApp && producerControlledByApp; + mCore->mQueueBufferCanDrop = false; + if (mCore->mConsumerControlledByApp && producerControlledByApp) { + mCore->mDequeueBufferCannotBlock = mDequeueTimeout < 0; + mCore->mQueueBufferCanDrop = mDequeueTimeout <= 0; } mCore->mAllowAllocation = true; @@ -1486,7 +1487,12 @@ status_t BufferQueueProducer::setDequeueTimeout(nsecs_t timeout) { } mDequeueTimeout = timeout; - mCore->mDequeueBufferCannotBlock = false; + if (timeout >= 0) { + mCore->mDequeueBufferCannotBlock = false; + if (timeout != 0) { + mCore->mQueueBufferCanDrop = false; + } + } VALIDATE_CONSISTENCY(); return NO_ERROR; diff --git a/libs/gui/include/gui/BufferQueueCore.h b/libs/gui/include/gui/BufferQueueCore.h index 537c957746..f91babca11 100644 --- a/libs/gui/include/gui/BufferQueueCore.h +++ b/libs/gui/include/gui/BufferQueueCore.h @@ -225,6 +225,11 @@ private: // consumer are controlled by the application. bool mDequeueBufferCannotBlock; + // mQueueBufferCanDrop indicates whether queueBuffer is allowd to drop + // buffers in non-async mode. This flag is set during connect when both the + // producer and consumer are controlled by application. + bool mQueueBufferCanDrop; + // mDefaultBufferFormat can be set so it will override the buffer format // when it isn't specified in dequeueBuffer. PixelFormat mDefaultBufferFormat; diff --git a/libs/gui/include/gui/IGraphicBufferProducer.h b/libs/gui/include/gui/IGraphicBufferProducer.h index 887654e05b..2f8a154faa 100644 --- a/libs/gui/include/gui/IGraphicBufferProducer.h +++ b/libs/gui/include/gui/IGraphicBufferProducer.h @@ -584,6 +584,10 @@ public: // non-blocking mode and its corresponding spare buffer (which is used to // ensure a buffer is always available). // + // N.B. queueBuffer will stop buffer dropping behavior if timeout is + // strictly positive. If timeout is zero or negative, previous buffer + // dropping behavior will not be changed. + // // Return of a value other than NO_ERROR means an error has occurred: // * BAD_VALUE - Failure to adjust the number of available slots. This can // happen because of trying to allocate/deallocate the async -- cgit v1.2.3-59-g8ed1b From 6a2127f162788add4da891fcd22882b9e2b65bad Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Thu, 25 Apr 2019 03:44:52 -0700 Subject: Zero-initialize HIDL structs before passing Test: cts-tradefed run cts-dev --module CtsMediaTestCases --compatibility:module-arg CtsMediaTestCases:include-annotation:android.platform.test.annotations.RequiresDevice (only existing failures/flakes) Bug: 131267328 Bug: 131356202 Change-Id: I91e6e0c692d470f4d3a713068545cedf5ae925a7 Merged-In: I91e6e0c692d470f4d3a713068545cedf5ae925a7 (cherry picked from commit c4a5f5ec811b149428e0835b7aabc6c4c802e720) --- libs/gui/bufferqueue/1.0/H2BGraphicBufferProducer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/gui/bufferqueue/1.0/H2BGraphicBufferProducer.cpp b/libs/gui/bufferqueue/1.0/H2BGraphicBufferProducer.cpp index 3b89291dc8..fe99620f99 100644 --- a/libs/gui/bufferqueue/1.0/H2BGraphicBufferProducer.cpp +++ b/libs/gui/bufferqueue/1.0/H2BGraphicBufferProducer.cpp @@ -1056,7 +1056,7 @@ status_t H2BGraphicBufferProducer::detachNextBuffer( status_t H2BGraphicBufferProducer::attachBuffer( int* outSlot, const sp& buffer) { - AnwBuffer tBuffer; + AnwBuffer tBuffer{}; wrapAs(&tBuffer, *buffer); status_t fnStatus; status_t transStatus = toStatusT(mBase->attachBuffer(tBuffer, @@ -1071,7 +1071,7 @@ status_t H2BGraphicBufferProducer::queueBuffer( int slot, const QueueBufferInput& input, QueueBufferOutput* output) { - HGraphicBufferProducer::QueueBufferInput tInput; + HGraphicBufferProducer::QueueBufferInput tInput{}; native_handle_t* nh; if (!wrapAs(&tInput, &nh, input)) { ALOGE("H2BGraphicBufferProducer::queueBuffer - " -- cgit v1.2.3-59-g8ed1b From 9085ced4a58bb7a27c51411ab18d3e4a5612649a Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Tue, 19 Feb 2019 10:05:00 -0800 Subject: [RESTRICT AUTOMERGE]: Exclude secure layers from most screenshots taken by the system server. In pre-P versions of Android, it was allowed to screenshot secure layers if the buffer queue producer which was the target of the screenshot was owned by the system (in this case SurfaceFlinger). This really was a synonym for: The screen rotation animation was allowed to capture secure layers, but the other code paths weren't. In O we mistakenly changed this check to always allow the system server to capture secure layers via the captureScreen path (the captureLayers path used for TaskSnapshots was unaffected). This can result in data leakage in cases where the system server takes screenshots on behalf of other parts of the system (e.g. for the assistant). To mitigate this we provide an explicit switch for the system server to specify whether it wishes to capture Secure layers. While this is dangerous, I think it is less dangerous than the previous implicit switch of capturing secure layers based on which type of BufferQueue was passed in. The flag defaults to not capturing secure layers and we set it to true in the one place we need it (for the screen rotation animation). Non privileged clients can still not capture secure layers at all directly. Test: SetFlagsSecureEUidSystem Bug: 120610669 Change-Id: I288ad3bbb0444306e90fe3bb15e51b447539dea5 (cherry picked from commit 5c99901e77437e403ba643c9dc839ee9659acada) --- libs/gui/ISurfaceComposer.cpp | 6 +- libs/gui/SurfaceComposerClient.cpp | 13 ++- libs/gui/include/gui/ISurfaceComposer.h | 3 +- libs/gui/include/gui/SurfaceComposerClient.h | 4 + libs/gui/tests/Surface_test.cpp | 3 +- services/surfaceflinger/DisplayDevice.h | 9 +- services/surfaceflinger/SurfaceFlinger.cpp | 6 +- services/surfaceflinger/SurfaceFlinger.h | 2 +- services/surfaceflinger/tests/Transaction_test.cpp | 101 +++++++++++++++++++++ 9 files changed, 135 insertions(+), 12 deletions(-) diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index d2d27e8239..e230b3affb 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -105,7 +105,7 @@ public: virtual status_t captureScreen(const sp& display, sp* outBuffer, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, - ISurfaceComposer::Rotation rotation) { + ISurfaceComposer::Rotation rotation, bool captureSecureLayers) { Parcel data, reply; data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor()); data.writeStrongBinder(display); @@ -116,6 +116,7 @@ public: data.writeInt32(maxLayerZ); data.writeInt32(static_cast(useIdentityTransform)); data.writeInt32(static_cast(rotation)); + data.writeInt32(static_cast(captureSecureLayers)); status_t err = remote()->transact(BnSurfaceComposer::CAPTURE_SCREEN, data, &reply); if (err != NO_ERROR) { @@ -643,10 +644,11 @@ status_t BnSurfaceComposer::onTransact( int32_t maxLayerZ = data.readInt32(); bool useIdentityTransform = static_cast(data.readInt32()); int32_t rotation = data.readInt32(); + bool captureSecureLayers = static_cast(data.readInt32()); status_t res = captureScreen(display, &outBuffer, sourceCrop, reqWidth, reqHeight, minLayerZ, maxLayerZ, useIdentityTransform, - static_cast(rotation)); + static_cast(rotation), captureSecureLayers); reply->writeInt32(res); if (res == NO_ERROR) { reply->write(*outBuffer); diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index f3c6fd2f87..263c7ef9e0 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -768,18 +768,27 @@ status_t SurfaceComposerClient::getHdrCapabilities(const sp& display, status_t ScreenshotClient::capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, - sp* outBuffer) { + bool captureSecureLayers, sp* outBuffer) { sp s(ComposerService::getComposerService()); if (s == NULL) return NO_INIT; status_t ret = s->captureScreen(display, outBuffer, sourceCrop, reqWidth, reqHeight, minLayerZ, maxLayerZ, useIdentityTransform, - static_cast(rotation)); + static_cast(rotation), + captureSecureLayers); if (ret != NO_ERROR) { return ret; } return ret; } +status_t ScreenshotClient::capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, + uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, + bool useIdentityTransform, uint32_t rotation, + sp* outBuffer) { + return capture(display, sourceCrop, reqWidth, reqHeight, + minLayerZ, maxLayerZ, useIdentityTransform, rotation, false, outBuffer); +} + status_t ScreenshotClient::captureLayers(const sp& layerHandle, Rect sourceCrop, float frameScale, sp* outBuffer) { sp s(ComposerService::getComposerService()); diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index 99a3a75502..fc7579ff11 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -180,7 +180,8 @@ public: virtual status_t captureScreen(const sp& display, sp* outBuffer, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, - Rotation rotation = eRotateNone) = 0; + Rotation rotation = eRotateNone, + bool captureSecureLayers = false) = 0; /** * Capture a subtree of the layer hierarchy, potentially ignoring the root node. diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index ad8a8b09d0..7d72661717 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -312,6 +312,10 @@ class ScreenshotClient { public: // if cropping isn't required, callers may pass in a default Rect, e.g.: // capture(display, producer, Rect(), reqWidth, ...); + static status_t capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, + uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, + bool useIdentityTransform, uint32_t rotation, + bool captureSecureLayers, sp* outBuffer); static status_t capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 6e196bfac8..959d782506 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -604,7 +604,8 @@ public: Rect /*sourceCrop*/, uint32_t /*reqWidth*/, uint32_t /*reqHeight*/, int32_t /*minLayerZ*/, int32_t /*maxLayerZ*/, bool /*useIdentityTransform*/, - Rotation /*rotation*/) override { return NO_ERROR; } + Rotation /*rotation*/, + bool /*captureSecureLayers*/) override { return NO_ERROR; } virtual status_t captureLayers(const sp& /*parentHandle*/, sp* /*outBuffer*/, const Rect& /*sourceCrop*/, float /*frameScale*/, bool /*childrenOnly*/) override { diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 6c3bd91793..1262209773 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -341,21 +341,24 @@ public: : DisplayRenderArea(device, device->getBounds(), device->getHeight(), device->getWidth(), rotation) {} DisplayRenderArea(const sp device, Rect sourceCrop, uint32_t reqHeight, - uint32_t reqWidth, ISurfaceComposer::Rotation rotation) + uint32_t reqWidth, ISurfaceComposer::Rotation rotation, + bool allowSecureLayers = true) : RenderArea(reqHeight, reqWidth, CaptureFill::OPAQUE, rotation), mDevice(device), - mSourceCrop(sourceCrop) {} + mSourceCrop(sourceCrop), + mAllowSecureLayers(allowSecureLayers) {} const Transform& getTransform() const override { return mDevice->getTransform(); } Rect getBounds() const override { return mDevice->getBounds(); } int getHeight() const override { return mDevice->getHeight(); } int getWidth() const override { return mDevice->getWidth(); } - bool isSecure() const override { return mDevice->isSecure(); } + bool isSecure() const override { return mAllowSecureLayers && mDevice->isSecure(); } bool needsFiltering() const override { return mDevice->needsFiltering(); } Rect getSourceCrop() const override { return mSourceCrop; } private: const sp mDevice; const Rect mSourceCrop; + const bool mAllowSecureLayers; }; }; // namespace android diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 11e7ff0f68..c1dc65b11a 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4836,7 +4836,8 @@ status_t SurfaceFlinger::captureScreen(const sp& display, sp& display, sp& display, sp* outBuffer, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, - ISurfaceComposer::Rotation rotation); + ISurfaceComposer::Rotation rotation, bool captureSecureLayers); virtual status_t captureLayers(const sp& parentHandle, sp* outBuffer, const Rect& sourceCrop, float frameScale, bool childrenOnly); virtual status_t getDisplayStats(const sp& display, diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp index 5108279043..deca17799c 100644 --- a/services/surfaceflinger/tests/Transaction_test.cpp +++ b/services/surfaceflinger/tests/Transaction_test.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -36,6 +37,8 @@ #include #include +#include +#include namespace android { @@ -67,6 +70,30 @@ std::ostream& operator<<(std::ostream& os, const Color& color) { return os; } +// Fill a region with the specified color. +void fillANativeWindowBufferColor(const ANativeWindow_Buffer& buffer, const Rect& rect, + const Color& color) { + Rect r(0, 0, buffer.width, buffer.height); + if (!r.intersect(rect, &r)) { + return; + } + + int32_t width = r.right - r.left; + int32_t height = r.bottom - r.top; + + for (int32_t row = 0; row < height; row++) { + uint8_t* dst = + static_cast(buffer.bits) + (buffer.stride * (r.top + row) + r.left) * 4; + for (int32_t column = 0; column < width; column++) { + dst[0] = color.r; + dst[1] = color.g; + dst[2] = color.b; + dst[3] = color.a; + dst += 4; + } + } +} + // Fill a region with the specified color. void fillBufferColor(const ANativeWindow_Buffer& buffer, const Rect& rect, const Color& color) { int32_t x = rect.left; @@ -319,6 +346,16 @@ protected: return layer; } + ANativeWindow_Buffer getBufferQueueLayerBuffer(const sp& layer) { + // wait for previous transactions (such as setSize) to complete + Transaction().apply(true); + + ANativeWindow_Buffer buffer = {}; + EXPECT_EQ(NO_ERROR, layer->getSurface()->lock(&buffer, nullptr)); + + return buffer; + } + ANativeWindow_Buffer getLayerBuffer(const sp& layer) { // wait for previous transactions (such as setSize) to complete Transaction().apply(true); @@ -336,6 +373,21 @@ protected: waitForLayerBuffers(); } + void postBufferQueueLayerBuffer(const sp& layer) { + ASSERT_EQ(NO_ERROR, layer->getSurface()->unlockAndPost()); + + // wait for the newly posted buffer to be latched + waitForLayerBuffers(); + } + + virtual void fillBufferQueueLayerColor(const sp& layer, const Color& color, + int32_t bufferWidth, int32_t bufferHeight) { + ANativeWindow_Buffer buffer; + ASSERT_NO_FATAL_FAILURE(buffer = getBufferQueueLayerBuffer(layer)); + fillANativeWindowBufferColor(buffer, Rect(0, 0, bufferWidth, bufferHeight), color); + postBufferQueueLayerBuffer(layer); + } + void fillLayerColor(const sp& layer, const Color& color) { ANativeWindow_Buffer buffer; ASSERT_NO_FATAL_FAILURE(buffer = getLayerBuffer(layer)); @@ -847,6 +899,55 @@ TEST_F(LayerTransactionTest, SetFlagsSecure) { false)); } +/** RAII Wrapper around get/seteuid */ +class UIDFaker { + uid_t oldId; +public: + UIDFaker(uid_t uid) { + oldId = geteuid(); + seteuid(uid); + } + ~UIDFaker() { + seteuid(oldId); + } +}; + +TEST_F(LayerTransactionTest, SetFlagsSecureEUidSystem) { + sp layer; + ASSERT_NO_FATAL_FAILURE(layer = createLayer("test", 32, 32)); + ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(layer, Color::RED, 32, 32)); + + sp composer = ComposerService::getComposerService(); + sp outBuffer; + Transaction() + .setFlags(layer, layer_state_t::eLayerSecure, layer_state_t::eLayerSecure) + .apply(true); + + ASSERT_EQ(PERMISSION_DENIED, + composer->captureScreen(mDisplay, &outBuffer, Rect(), 0, 0, 0, INT_MAX, false)); + + UIDFaker f(AID_SYSTEM); + + // By default the system can capture screenshots with secure layers but they + // will be blacked out + ASSERT_EQ(NO_ERROR, + composer->captureScreen(mDisplay, &outBuffer, Rect(), 0, 0, 0, INT_MAX, false)); + + { + SCOPED_TRACE("as system"); + auto shot = screenshot(); + shot->expectColor(Rect(0, 0, 32, 32), Color::BLACK); + } + + // Here we pass captureSecureLayers = true and since we are AID_SYSTEM we should be able + // to receive them...we are expected to take care with the results. + ASSERT_EQ(NO_ERROR, + composer->captureScreen(mDisplay, &outBuffer, + Rect(), 0, 0, 0, INT_MAX, false, ISurfaceComposer::eRotateNone, true)); + ScreenCapture sc(outBuffer); + sc.expectColor(Rect(0, 0, 32, 32), Color::RED); +} + TEST_F(LayerTransactionTest, SetTransparentRegionHintBasic) { const Rect top(0, 0, 32, 16); const Rect bottom(0, 16, 32, 32); -- cgit v1.2.3-59-g8ed1b From 0283c73ccf4c54d0ed8e8b479ea76cb1e1f815d8 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Tue, 2 Apr 2019 16:32:58 -0700 Subject: [RESTRICT AUTOMERGE] SurfaceFlinger: Indicate whether we have captured secure layers. For purposes of the screen rotation animation the system server is allowed to capture secure (not protected) layers and trusted not to persist screenshots which may contain secure layers. However when displaying the screen rotation animation, the layer the screenshot is placed on will itself not be secure, so if we record the animation the recording will contain persisted versions of the secure content. Here we forward whether the screenshot contains secure content so that system server can do the right thing. Bug: b/69703445 Test: Transaction_test#SetFlagsSecureEUidSystem Change-Id: If493a39257b5e15410360a3df23f3e0fc8cf295c (cherry picked from commit a1586de21f6c9191b99d2f3c815fcd15c48114e1) --- libs/gui/ISurfaceComposer.cpp | 10 ++++++-- libs/gui/SurfaceComposerClient.cpp | 10 ++++---- libs/gui/include/gui/ISurfaceComposer.h | 15 ++++++++++-- libs/gui/include/gui/SurfaceComposerClient.h | 3 ++- libs/gui/tests/Surface_test.cpp | 8 ++++--- services/surfaceflinger/SurfaceFlinger.cpp | 28 ++++++++++++---------- services/surfaceflinger/SurfaceFlinger.h | 5 ++-- services/surfaceflinger/tests/Transaction_test.cpp | 4 +++- 8 files changed, 56 insertions(+), 27 deletions(-) diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index e230b3affb..cec86e2a04 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -103,6 +103,7 @@ public: } virtual status_t captureScreen(const sp& display, sp* outBuffer, + bool& outCapturedSecureLayers, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, ISurfaceComposer::Rotation rotation, bool captureSecureLayers) { @@ -130,6 +131,8 @@ public: *outBuffer = new GraphicBuffer(); reply.read(**outBuffer); + outCapturedSecureLayers = reply.readBool(); + return err; } @@ -646,12 +649,15 @@ status_t BnSurfaceComposer::onTransact( int32_t rotation = data.readInt32(); bool captureSecureLayers = static_cast(data.readInt32()); - status_t res = captureScreen(display, &outBuffer, sourceCrop, reqWidth, reqHeight, - minLayerZ, maxLayerZ, useIdentityTransform, + bool capturedSecureLayers = false; + status_t res = captureScreen(display, &outBuffer, capturedSecureLayers, sourceCrop, reqWidth, + reqHeight, minLayerZ, maxLayerZ, useIdentityTransform, static_cast(rotation), captureSecureLayers); + reply->writeInt32(res); if (res == NO_ERROR) { reply->write(*outBuffer); + reply->writeBool(capturedSecureLayers); } return NO_ERROR; } diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 263c7ef9e0..100257629e 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -768,11 +768,12 @@ status_t SurfaceComposerClient::getHdrCapabilities(const sp& display, status_t ScreenshotClient::capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, - bool captureSecureLayers, sp* outBuffer) { + bool captureSecureLayers, sp* outBuffer, + bool& outCapturedSecureLayers) { sp s(ComposerService::getComposerService()); if (s == NULL) return NO_INIT; - status_t ret = s->captureScreen(display, outBuffer, sourceCrop, reqWidth, reqHeight, minLayerZ, - maxLayerZ, useIdentityTransform, + status_t ret = s->captureScreen(display, outBuffer, outCapturedSecureLayers, sourceCrop, + reqWidth, reqHeight, minLayerZ, maxLayerZ, useIdentityTransform, static_cast(rotation), captureSecureLayers); if (ret != NO_ERROR) { @@ -785,8 +786,9 @@ status_t ScreenshotClient::capture(const sp& display, Rect sourceCrop, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, sp* outBuffer) { + bool ignored; return capture(display, sourceCrop, reqWidth, reqHeight, - minLayerZ, maxLayerZ, useIdentityTransform, rotation, false, outBuffer); + minLayerZ, maxLayerZ, useIdentityTransform, rotation, false, outBuffer, ignored); } status_t ScreenshotClient::captureLayers(const sp& layerHandle, Rect sourceCrop, diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index fc7579ff11..0db21a56e3 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -178,11 +178,22 @@ public: * This function will fail if there is a secure window on screen. */ virtual status_t captureScreen(const sp& display, sp* outBuffer, - Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, - int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, + bool& outCapturedSecureLayers, Rect sourceCrop, + uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, + int32_t maxLayerZ, bool useIdentityTransform, Rotation rotation = eRotateNone, bool captureSecureLayers = false) = 0; + virtual status_t captureScreen(const sp& display, sp* outBuffer, + Rect sourceCrop, + uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, + int32_t maxLayerZ, bool useIdentityTransform, + Rotation rotation = eRotateNone, + bool captureSecureLayers = false) { + bool ignored; + return captureScreen(display, outBuffer, ignored, sourceCrop, reqWidth, reqHeight, minLayerZ, + maxLayerZ, useIdentityTransform, rotation, captureSecureLayers); + } /** * Capture a subtree of the layer hierarchy, potentially ignoring the root node. */ diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 7d72661717..49bb687561 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -315,7 +315,8 @@ public: static status_t capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, - bool captureSecureLayers, sp* outBuffer); + bool captureSecureLayers, sp* outBuffer, + bool& outCapturedSecureLayers); static status_t capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 959d782506..04686e5ad2 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -126,7 +126,7 @@ TEST_F(SurfaceTest, QueuesToWindowComposerIsTrueWhenPurgatorized) { } // This test probably doesn't belong here. -TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersSucceed) { +TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersDontSucceed) { sp anw(mSurface); // Verify the screenshot works with no protected buffers. @@ -134,7 +134,8 @@ TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersSucceed) { sp display(sf->getBuiltInDisplay( ISurfaceComposer::eDisplayIdMain)); sp outBuffer; - ASSERT_EQ(NO_ERROR, sf->captureScreen(display, &outBuffer, Rect(), + bool ignored; + ASSERT_EQ(NO_ERROR, sf->captureScreen(display, &outBuffer, ignored, Rect(), 64, 64, 0, 0x7fffffff, false)); ASSERT_EQ(NO_ERROR, native_window_api_connect(anw.get(), @@ -165,7 +166,7 @@ TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersSucceed) { &buf)); ASSERT_EQ(NO_ERROR, anw->queueBuffer(anw.get(), buf, -1)); } - ASSERT_EQ(NO_ERROR, sf->captureScreen(display, &outBuffer, Rect(), + ASSERT_EQ(NO_ERROR, sf->captureScreen(display, &outBuffer, ignored, Rect(), 64, 64, 0, 0x7fffffff, false)); } @@ -601,6 +602,7 @@ public: ColorMode /*colorMode*/) override { return NO_ERROR; } status_t captureScreen(const sp& /*display*/, sp* /*outBuffer*/, + bool& /* outCapturedSecureLayers */, Rect /*sourceCrop*/, uint32_t /*reqWidth*/, uint32_t /*reqHeight*/, int32_t /*minLayerZ*/, int32_t /*maxLayerZ*/, bool /*useIdentityTransform*/, diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c1dc65b11a..25cb5896e1 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4832,7 +4832,8 @@ private: const int mApi; }; -status_t SurfaceFlinger::captureScreen(const sp& display, sp* outBuffer, +status_t SurfaceFlinger::captureScreen(const sp& display, + sp* outBuffer, bool& outCapturedSecureLayers, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, @@ -4860,7 +4861,8 @@ status_t SurfaceFlinger::captureScreen(const sp& display, sp& layerHandleBinder, @@ -4974,13 +4976,16 @@ status_t SurfaceFlinger::captureLayers(const sp& layerHandleBinder, visitor(layer); }); }; - return captureScreenCommon(renderArea, traverseLayers, outBuffer, false); + bool outCapturedSecureLayers = false; + return captureScreenCommon(renderArea, traverseLayers, outBuffer, false, + outCapturedSecureLayers); } status_t SurfaceFlinger::captureScreenCommon(RenderArea& renderArea, TraverseLayersFunction traverseLayers, sp* outBuffer, - bool useIdentityTransform) { + bool useIdentityTransform, + bool& outCapturedSecureLayers) { ATRACE_CALL(); renderArea.updateDimensions(mPrimaryDisplayOrientation); @@ -5018,7 +5023,8 @@ status_t SurfaceFlinger::captureScreenCommon(RenderArea& renderArea, Mutex::Autolock _l(mStateLock); renderArea.render([&]() { result = captureScreenImplLocked(renderArea, traverseLayers, (*outBuffer).get(), - useIdentityTransform, forSystem, &fd); + useIdentityTransform, forSystem, &fd, + outCapturedSecureLayers); }); } @@ -5169,21 +5175,19 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, status_t SurfaceFlinger::captureScreenImplLocked(const RenderArea& renderArea, TraverseLayersFunction traverseLayers, ANativeWindowBuffer* buffer, - bool useIdentityTransform, - bool forSystem, - int* outSyncFd) { + bool useIdentityTransform, bool forSystem, + int* outSyncFd, bool& outCapturedSecureLayers) { ATRACE_CALL(); - bool secureLayerIsVisible = false; - traverseLayers([&](Layer* layer) { - secureLayerIsVisible = secureLayerIsVisible || (layer->isVisible() && layer->isSecure()); + outCapturedSecureLayers = + outCapturedSecureLayers || (layer->isVisible() && layer->isSecure()); }); // We allow the system server to take screenshots of secure layers for // use in situations like the Screen-rotation animation and place // the impetus on WindowManager to not persist them. - if (secureLayerIsVisible && !forSystem) { + if (outCapturedSecureLayers && !forSystem) { ALOGW("FB is protected: PERMISSION_DENIED"); return PERMISSION_DENIED; } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 2577f179cb..60bf94ffec 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -422,6 +422,7 @@ private: virtual sp createDisplayEventConnection( ISurfaceComposer::VsyncSource vsyncSource = eVsyncSourceApp); virtual status_t captureScreen(const sp& display, sp* outBuffer, + bool& outCapturedSecureLayers, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, ISurfaceComposer::Rotation rotation, bool captureSecureLayers); @@ -573,11 +574,11 @@ private: bool yswap, bool useIdentityTransform); status_t captureScreenCommon(RenderArea& renderArea, TraverseLayersFunction traverseLayers, sp* outBuffer, - bool useIdentityTransform); + bool useIdentityTransform, bool& outCapturedSecureLayers); status_t captureScreenImplLocked(const RenderArea& renderArea, TraverseLayersFunction traverseLayers, ANativeWindowBuffer* buffer, bool useIdentityTransform, - bool forSystem, int* outSyncFd); + bool forSystem, int* outSyncFd, bool& outCapturedSecureLayers); void traverseLayersInDisplay(const sp& display, int32_t minLayerZ, int32_t maxLayerZ, const LayerVector::Visitor& visitor); diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp index deca17799c..6ce2075a3d 100644 --- a/services/surfaceflinger/tests/Transaction_test.cpp +++ b/services/surfaceflinger/tests/Transaction_test.cpp @@ -941,9 +941,11 @@ TEST_F(LayerTransactionTest, SetFlagsSecureEUidSystem) { // Here we pass captureSecureLayers = true and since we are AID_SYSTEM we should be able // to receive them...we are expected to take care with the results. + bool outCapturedSecureLayers = false; ASSERT_EQ(NO_ERROR, - composer->captureScreen(mDisplay, &outBuffer, + composer->captureScreen(mDisplay, &outBuffer, outCapturedSecureLayers, Rect(), 0, 0, 0, INT_MAX, false, ISurfaceComposer::eRotateNone, true)); + ASSERT_EQ(true, outCapturedSecureLayers); ScreenCapture sc(outBuffer); sc.expectColor(Rect(0, 0, 32, 32), Color::RED); } -- cgit v1.2.3-59-g8ed1b From 84e46603cc222a82155af710ae26101eda407aca Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Fri, 12 Apr 2019 17:54:32 -0700 Subject: [RESTRICT AUTOMERGE] libvulkan: drop the advertised swapchain spec version to 68 Bug: 130182551 Test: build and CtsDeqpTestCases Change-Id: If8b7e59879c9eaf1ee453cb43f2d4a7f06bf0ac9 (cherry picked from commit ee7a9ca859add62690d1413d0e9c88f8ec42984d) --- vulkan/libvulkan/driver.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 56bc35ec70..19d0146dc9 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -945,7 +945,9 @@ VkResult EnumerateDeviceExtensionProperties( memcpy(prop.extensionName, VK_KHR_SWAPCHAIN_EXTENSION_NAME, sizeof(VK_KHR_SWAPCHAIN_EXTENSION_NAME)); - prop.specVersion = VK_KHR_SWAPCHAIN_SPEC_VERSION; + // b/130182551 VK_KHR_SWAPCHAIN_SPEC_VERSION > 68 has structs the + // loader doesn't handle properly. So drop the spec version to 68. + prop.specVersion = 68; } } -- cgit v1.2.3-59-g8ed1b From 4c66d2660cf17605431dbb62006ff934a029142e Mon Sep 17 00:00:00 2001 From: Pawin Vongmasa Date: Thu, 25 Apr 2019 03:44:52 -0700 Subject: Zero-initialize HIDL structs before passing Test: cts-tradefed run cts-dev --module CtsMediaTestCases --compatibility:module-arg CtsMediaTestCases:include-annotation:android.platform.test.annotations.RequiresDevice (only existing failures/flakes) Bug: 131267328 Bug: 131356202 Change-Id: I91e6e0c692d470f4d3a713068545cedf5ae925a7 Merged-In: I91e6e0c692d470f4d3a713068545cedf5ae925a7 (cherry picked from commit c4a5f5ec811b149428e0835b7aabc6c4c802e720) --- libs/gui/bufferqueue/1.0/H2BGraphicBufferProducer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/gui/bufferqueue/1.0/H2BGraphicBufferProducer.cpp b/libs/gui/bufferqueue/1.0/H2BGraphicBufferProducer.cpp index 3b89291dc8..fe99620f99 100644 --- a/libs/gui/bufferqueue/1.0/H2BGraphicBufferProducer.cpp +++ b/libs/gui/bufferqueue/1.0/H2BGraphicBufferProducer.cpp @@ -1056,7 +1056,7 @@ status_t H2BGraphicBufferProducer::detachNextBuffer( status_t H2BGraphicBufferProducer::attachBuffer( int* outSlot, const sp& buffer) { - AnwBuffer tBuffer; + AnwBuffer tBuffer{}; wrapAs(&tBuffer, *buffer); status_t fnStatus; status_t transStatus = toStatusT(mBase->attachBuffer(tBuffer, @@ -1071,7 +1071,7 @@ status_t H2BGraphicBufferProducer::queueBuffer( int slot, const QueueBufferInput& input, QueueBufferOutput* output) { - HGraphicBufferProducer::QueueBufferInput tInput; + HGraphicBufferProducer::QueueBufferInput tInput{}; native_handle_t* nh; if (!wrapAs(&tInput, &nh, input)) { ALOGE("H2BGraphicBufferProducer::queueBuffer - " -- cgit v1.2.3-59-g8ed1b From 1cc813bb3b817e20e38ff9cb44533ebc23490ce8 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Tue, 19 Feb 2019 10:05:00 -0800 Subject: [RESTRICT AUTOMERGE]: Exclude secure layers from most screenshots taken by the system server. In pre-P versions of Android, it was allowed to screenshot secure layers if the buffer queue producer which was the target of the screenshot was owned by the system (in this case SurfaceFlinger). This really was a synonym for: The screen rotation animation was allowed to capture secure layers, but the other code paths weren't. In O we mistakenly changed this check to always allow the system server to capture secure layers via the captureScreen path (the captureLayers path used for TaskSnapshots was unaffected). This can result in data leakage in cases where the system server takes screenshots on behalf of other parts of the system (e.g. for the assistant). To mitigate this we provide an explicit switch for the system server to specify whether it wishes to capture Secure layers. While this is dangerous, I think it is less dangerous than the previous implicit switch of capturing secure layers based on which type of BufferQueue was passed in. The flag defaults to not capturing secure layers and we set it to true in the one place we need it (for the screen rotation animation). Non privileged clients can still not capture secure layers at all directly. Test: SetFlagsSecureEUidSystem Bug: 120610669 Change-Id: I288ad3bbb0444306e90fe3bb15e51b447539dea5 (cherry picked from commit 5c99901e77437e403ba643c9dc839ee9659acada) --- libs/gui/ISurfaceComposer.cpp | 6 +- libs/gui/SurfaceComposerClient.cpp | 13 ++- libs/gui/include/gui/ISurfaceComposer.h | 3 +- libs/gui/include/gui/SurfaceComposerClient.h | 4 + libs/gui/tests/Surface_test.cpp | 3 +- services/surfaceflinger/DisplayDevice.h | 9 +- services/surfaceflinger/SurfaceFlinger.cpp | 6 +- services/surfaceflinger/SurfaceFlinger.h | 2 +- services/surfaceflinger/tests/Transaction_test.cpp | 101 +++++++++++++++++++++ 9 files changed, 135 insertions(+), 12 deletions(-) diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index d2d27e8239..e230b3affb 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -105,7 +105,7 @@ public: virtual status_t captureScreen(const sp& display, sp* outBuffer, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, - ISurfaceComposer::Rotation rotation) { + ISurfaceComposer::Rotation rotation, bool captureSecureLayers) { Parcel data, reply; data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor()); data.writeStrongBinder(display); @@ -116,6 +116,7 @@ public: data.writeInt32(maxLayerZ); data.writeInt32(static_cast(useIdentityTransform)); data.writeInt32(static_cast(rotation)); + data.writeInt32(static_cast(captureSecureLayers)); status_t err = remote()->transact(BnSurfaceComposer::CAPTURE_SCREEN, data, &reply); if (err != NO_ERROR) { @@ -643,10 +644,11 @@ status_t BnSurfaceComposer::onTransact( int32_t maxLayerZ = data.readInt32(); bool useIdentityTransform = static_cast(data.readInt32()); int32_t rotation = data.readInt32(); + bool captureSecureLayers = static_cast(data.readInt32()); status_t res = captureScreen(display, &outBuffer, sourceCrop, reqWidth, reqHeight, minLayerZ, maxLayerZ, useIdentityTransform, - static_cast(rotation)); + static_cast(rotation), captureSecureLayers); reply->writeInt32(res); if (res == NO_ERROR) { reply->write(*outBuffer); diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index f3c6fd2f87..263c7ef9e0 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -768,18 +768,27 @@ status_t SurfaceComposerClient::getHdrCapabilities(const sp& display, status_t ScreenshotClient::capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, - sp* outBuffer) { + bool captureSecureLayers, sp* outBuffer) { sp s(ComposerService::getComposerService()); if (s == NULL) return NO_INIT; status_t ret = s->captureScreen(display, outBuffer, sourceCrop, reqWidth, reqHeight, minLayerZ, maxLayerZ, useIdentityTransform, - static_cast(rotation)); + static_cast(rotation), + captureSecureLayers); if (ret != NO_ERROR) { return ret; } return ret; } +status_t ScreenshotClient::capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, + uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, + bool useIdentityTransform, uint32_t rotation, + sp* outBuffer) { + return capture(display, sourceCrop, reqWidth, reqHeight, + minLayerZ, maxLayerZ, useIdentityTransform, rotation, false, outBuffer); +} + status_t ScreenshotClient::captureLayers(const sp& layerHandle, Rect sourceCrop, float frameScale, sp* outBuffer) { sp s(ComposerService::getComposerService()); diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index 99a3a75502..fc7579ff11 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -180,7 +180,8 @@ public: virtual status_t captureScreen(const sp& display, sp* outBuffer, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, - Rotation rotation = eRotateNone) = 0; + Rotation rotation = eRotateNone, + bool captureSecureLayers = false) = 0; /** * Capture a subtree of the layer hierarchy, potentially ignoring the root node. diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index ad8a8b09d0..7d72661717 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -312,6 +312,10 @@ class ScreenshotClient { public: // if cropping isn't required, callers may pass in a default Rect, e.g.: // capture(display, producer, Rect(), reqWidth, ...); + static status_t capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, + uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, + bool useIdentityTransform, uint32_t rotation, + bool captureSecureLayers, sp* outBuffer); static status_t capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 6e196bfac8..959d782506 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -604,7 +604,8 @@ public: Rect /*sourceCrop*/, uint32_t /*reqWidth*/, uint32_t /*reqHeight*/, int32_t /*minLayerZ*/, int32_t /*maxLayerZ*/, bool /*useIdentityTransform*/, - Rotation /*rotation*/) override { return NO_ERROR; } + Rotation /*rotation*/, + bool /*captureSecureLayers*/) override { return NO_ERROR; } virtual status_t captureLayers(const sp& /*parentHandle*/, sp* /*outBuffer*/, const Rect& /*sourceCrop*/, float /*frameScale*/, bool /*childrenOnly*/) override { diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 6c3bd91793..1262209773 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -341,21 +341,24 @@ public: : DisplayRenderArea(device, device->getBounds(), device->getHeight(), device->getWidth(), rotation) {} DisplayRenderArea(const sp device, Rect sourceCrop, uint32_t reqHeight, - uint32_t reqWidth, ISurfaceComposer::Rotation rotation) + uint32_t reqWidth, ISurfaceComposer::Rotation rotation, + bool allowSecureLayers = true) : RenderArea(reqHeight, reqWidth, CaptureFill::OPAQUE, rotation), mDevice(device), - mSourceCrop(sourceCrop) {} + mSourceCrop(sourceCrop), + mAllowSecureLayers(allowSecureLayers) {} const Transform& getTransform() const override { return mDevice->getTransform(); } Rect getBounds() const override { return mDevice->getBounds(); } int getHeight() const override { return mDevice->getHeight(); } int getWidth() const override { return mDevice->getWidth(); } - bool isSecure() const override { return mDevice->isSecure(); } + bool isSecure() const override { return mAllowSecureLayers && mDevice->isSecure(); } bool needsFiltering() const override { return mDevice->needsFiltering(); } Rect getSourceCrop() const override { return mSourceCrop; } private: const sp mDevice; const Rect mSourceCrop; + const bool mAllowSecureLayers; }; }; // namespace android diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 11e7ff0f68..c1dc65b11a 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4836,7 +4836,8 @@ status_t SurfaceFlinger::captureScreen(const sp& display, sp& display, sp& display, sp* outBuffer, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, - ISurfaceComposer::Rotation rotation); + ISurfaceComposer::Rotation rotation, bool captureSecureLayers); virtual status_t captureLayers(const sp& parentHandle, sp* outBuffer, const Rect& sourceCrop, float frameScale, bool childrenOnly); virtual status_t getDisplayStats(const sp& display, diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp index 5108279043..deca17799c 100644 --- a/services/surfaceflinger/tests/Transaction_test.cpp +++ b/services/surfaceflinger/tests/Transaction_test.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -36,6 +37,8 @@ #include #include +#include +#include namespace android { @@ -67,6 +70,30 @@ std::ostream& operator<<(std::ostream& os, const Color& color) { return os; } +// Fill a region with the specified color. +void fillANativeWindowBufferColor(const ANativeWindow_Buffer& buffer, const Rect& rect, + const Color& color) { + Rect r(0, 0, buffer.width, buffer.height); + if (!r.intersect(rect, &r)) { + return; + } + + int32_t width = r.right - r.left; + int32_t height = r.bottom - r.top; + + for (int32_t row = 0; row < height; row++) { + uint8_t* dst = + static_cast(buffer.bits) + (buffer.stride * (r.top + row) + r.left) * 4; + for (int32_t column = 0; column < width; column++) { + dst[0] = color.r; + dst[1] = color.g; + dst[2] = color.b; + dst[3] = color.a; + dst += 4; + } + } +} + // Fill a region with the specified color. void fillBufferColor(const ANativeWindow_Buffer& buffer, const Rect& rect, const Color& color) { int32_t x = rect.left; @@ -319,6 +346,16 @@ protected: return layer; } + ANativeWindow_Buffer getBufferQueueLayerBuffer(const sp& layer) { + // wait for previous transactions (such as setSize) to complete + Transaction().apply(true); + + ANativeWindow_Buffer buffer = {}; + EXPECT_EQ(NO_ERROR, layer->getSurface()->lock(&buffer, nullptr)); + + return buffer; + } + ANativeWindow_Buffer getLayerBuffer(const sp& layer) { // wait for previous transactions (such as setSize) to complete Transaction().apply(true); @@ -336,6 +373,21 @@ protected: waitForLayerBuffers(); } + void postBufferQueueLayerBuffer(const sp& layer) { + ASSERT_EQ(NO_ERROR, layer->getSurface()->unlockAndPost()); + + // wait for the newly posted buffer to be latched + waitForLayerBuffers(); + } + + virtual void fillBufferQueueLayerColor(const sp& layer, const Color& color, + int32_t bufferWidth, int32_t bufferHeight) { + ANativeWindow_Buffer buffer; + ASSERT_NO_FATAL_FAILURE(buffer = getBufferQueueLayerBuffer(layer)); + fillANativeWindowBufferColor(buffer, Rect(0, 0, bufferWidth, bufferHeight), color); + postBufferQueueLayerBuffer(layer); + } + void fillLayerColor(const sp& layer, const Color& color) { ANativeWindow_Buffer buffer; ASSERT_NO_FATAL_FAILURE(buffer = getLayerBuffer(layer)); @@ -847,6 +899,55 @@ TEST_F(LayerTransactionTest, SetFlagsSecure) { false)); } +/** RAII Wrapper around get/seteuid */ +class UIDFaker { + uid_t oldId; +public: + UIDFaker(uid_t uid) { + oldId = geteuid(); + seteuid(uid); + } + ~UIDFaker() { + seteuid(oldId); + } +}; + +TEST_F(LayerTransactionTest, SetFlagsSecureEUidSystem) { + sp layer; + ASSERT_NO_FATAL_FAILURE(layer = createLayer("test", 32, 32)); + ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(layer, Color::RED, 32, 32)); + + sp composer = ComposerService::getComposerService(); + sp outBuffer; + Transaction() + .setFlags(layer, layer_state_t::eLayerSecure, layer_state_t::eLayerSecure) + .apply(true); + + ASSERT_EQ(PERMISSION_DENIED, + composer->captureScreen(mDisplay, &outBuffer, Rect(), 0, 0, 0, INT_MAX, false)); + + UIDFaker f(AID_SYSTEM); + + // By default the system can capture screenshots with secure layers but they + // will be blacked out + ASSERT_EQ(NO_ERROR, + composer->captureScreen(mDisplay, &outBuffer, Rect(), 0, 0, 0, INT_MAX, false)); + + { + SCOPED_TRACE("as system"); + auto shot = screenshot(); + shot->expectColor(Rect(0, 0, 32, 32), Color::BLACK); + } + + // Here we pass captureSecureLayers = true and since we are AID_SYSTEM we should be able + // to receive them...we are expected to take care with the results. + ASSERT_EQ(NO_ERROR, + composer->captureScreen(mDisplay, &outBuffer, + Rect(), 0, 0, 0, INT_MAX, false, ISurfaceComposer::eRotateNone, true)); + ScreenCapture sc(outBuffer); + sc.expectColor(Rect(0, 0, 32, 32), Color::RED); +} + TEST_F(LayerTransactionTest, SetTransparentRegionHintBasic) { const Rect top(0, 0, 32, 16); const Rect bottom(0, 16, 32, 32); -- cgit v1.2.3-59-g8ed1b From f3d8d3f6873738447b82744e3e550ceefd09236a Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Tue, 2 Apr 2019 16:32:58 -0700 Subject: [RESTRICT AUTOMERGE] SurfaceFlinger: Indicate whether we have captured secure layers. For purposes of the screen rotation animation the system server is allowed to capture secure (not protected) layers and trusted not to persist screenshots which may contain secure layers. However when displaying the screen rotation animation, the layer the screenshot is placed on will itself not be secure, so if we record the animation the recording will contain persisted versions of the secure content. Here we forward whether the screenshot contains secure content so that system server can do the right thing. Bug: b/69703445 Test: Transaction_test#SetFlagsSecureEUidSystem Change-Id: If493a39257b5e15410360a3df23f3e0fc8cf295c (cherry picked from commit a1586de21f6c9191b99d2f3c815fcd15c48114e1) --- libs/gui/ISurfaceComposer.cpp | 10 ++++++-- libs/gui/SurfaceComposerClient.cpp | 10 ++++---- libs/gui/include/gui/ISurfaceComposer.h | 15 ++++++++++-- libs/gui/include/gui/SurfaceComposerClient.h | 3 ++- libs/gui/tests/Surface_test.cpp | 8 ++++--- services/surfaceflinger/SurfaceFlinger.cpp | 28 ++++++++++++---------- services/surfaceflinger/SurfaceFlinger.h | 5 ++-- services/surfaceflinger/tests/Transaction_test.cpp | 4 +++- 8 files changed, 56 insertions(+), 27 deletions(-) diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index e230b3affb..cec86e2a04 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -103,6 +103,7 @@ public: } virtual status_t captureScreen(const sp& display, sp* outBuffer, + bool& outCapturedSecureLayers, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, ISurfaceComposer::Rotation rotation, bool captureSecureLayers) { @@ -130,6 +131,8 @@ public: *outBuffer = new GraphicBuffer(); reply.read(**outBuffer); + outCapturedSecureLayers = reply.readBool(); + return err; } @@ -646,12 +649,15 @@ status_t BnSurfaceComposer::onTransact( int32_t rotation = data.readInt32(); bool captureSecureLayers = static_cast(data.readInt32()); - status_t res = captureScreen(display, &outBuffer, sourceCrop, reqWidth, reqHeight, - minLayerZ, maxLayerZ, useIdentityTransform, + bool capturedSecureLayers = false; + status_t res = captureScreen(display, &outBuffer, capturedSecureLayers, sourceCrop, reqWidth, + reqHeight, minLayerZ, maxLayerZ, useIdentityTransform, static_cast(rotation), captureSecureLayers); + reply->writeInt32(res); if (res == NO_ERROR) { reply->write(*outBuffer); + reply->writeBool(capturedSecureLayers); } return NO_ERROR; } diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 263c7ef9e0..100257629e 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -768,11 +768,12 @@ status_t SurfaceComposerClient::getHdrCapabilities(const sp& display, status_t ScreenshotClient::capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, - bool captureSecureLayers, sp* outBuffer) { + bool captureSecureLayers, sp* outBuffer, + bool& outCapturedSecureLayers) { sp s(ComposerService::getComposerService()); if (s == NULL) return NO_INIT; - status_t ret = s->captureScreen(display, outBuffer, sourceCrop, reqWidth, reqHeight, minLayerZ, - maxLayerZ, useIdentityTransform, + status_t ret = s->captureScreen(display, outBuffer, outCapturedSecureLayers, sourceCrop, + reqWidth, reqHeight, minLayerZ, maxLayerZ, useIdentityTransform, static_cast(rotation), captureSecureLayers); if (ret != NO_ERROR) { @@ -785,8 +786,9 @@ status_t ScreenshotClient::capture(const sp& display, Rect sourceCrop, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, sp* outBuffer) { + bool ignored; return capture(display, sourceCrop, reqWidth, reqHeight, - minLayerZ, maxLayerZ, useIdentityTransform, rotation, false, outBuffer); + minLayerZ, maxLayerZ, useIdentityTransform, rotation, false, outBuffer, ignored); } status_t ScreenshotClient::captureLayers(const sp& layerHandle, Rect sourceCrop, diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index fc7579ff11..0db21a56e3 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -178,11 +178,22 @@ public: * This function will fail if there is a secure window on screen. */ virtual status_t captureScreen(const sp& display, sp* outBuffer, - Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, - int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, + bool& outCapturedSecureLayers, Rect sourceCrop, + uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, + int32_t maxLayerZ, bool useIdentityTransform, Rotation rotation = eRotateNone, bool captureSecureLayers = false) = 0; + virtual status_t captureScreen(const sp& display, sp* outBuffer, + Rect sourceCrop, + uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, + int32_t maxLayerZ, bool useIdentityTransform, + Rotation rotation = eRotateNone, + bool captureSecureLayers = false) { + bool ignored; + return captureScreen(display, outBuffer, ignored, sourceCrop, reqWidth, reqHeight, minLayerZ, + maxLayerZ, useIdentityTransform, rotation, captureSecureLayers); + } /** * Capture a subtree of the layer hierarchy, potentially ignoring the root node. */ diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 7d72661717..49bb687561 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -315,7 +315,8 @@ public: static status_t capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, - bool captureSecureLayers, sp* outBuffer); + bool captureSecureLayers, sp* outBuffer, + bool& outCapturedSecureLayers); static status_t capture(const sp& display, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, uint32_t rotation, diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 959d782506..04686e5ad2 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -126,7 +126,7 @@ TEST_F(SurfaceTest, QueuesToWindowComposerIsTrueWhenPurgatorized) { } // This test probably doesn't belong here. -TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersSucceed) { +TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersDontSucceed) { sp anw(mSurface); // Verify the screenshot works with no protected buffers. @@ -134,7 +134,8 @@ TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersSucceed) { sp display(sf->getBuiltInDisplay( ISurfaceComposer::eDisplayIdMain)); sp outBuffer; - ASSERT_EQ(NO_ERROR, sf->captureScreen(display, &outBuffer, Rect(), + bool ignored; + ASSERT_EQ(NO_ERROR, sf->captureScreen(display, &outBuffer, ignored, Rect(), 64, 64, 0, 0x7fffffff, false)); ASSERT_EQ(NO_ERROR, native_window_api_connect(anw.get(), @@ -165,7 +166,7 @@ TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersSucceed) { &buf)); ASSERT_EQ(NO_ERROR, anw->queueBuffer(anw.get(), buf, -1)); } - ASSERT_EQ(NO_ERROR, sf->captureScreen(display, &outBuffer, Rect(), + ASSERT_EQ(NO_ERROR, sf->captureScreen(display, &outBuffer, ignored, Rect(), 64, 64, 0, 0x7fffffff, false)); } @@ -601,6 +602,7 @@ public: ColorMode /*colorMode*/) override { return NO_ERROR; } status_t captureScreen(const sp& /*display*/, sp* /*outBuffer*/, + bool& /* outCapturedSecureLayers */, Rect /*sourceCrop*/, uint32_t /*reqWidth*/, uint32_t /*reqHeight*/, int32_t /*minLayerZ*/, int32_t /*maxLayerZ*/, bool /*useIdentityTransform*/, diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c1dc65b11a..25cb5896e1 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4832,7 +4832,8 @@ private: const int mApi; }; -status_t SurfaceFlinger::captureScreen(const sp& display, sp* outBuffer, +status_t SurfaceFlinger::captureScreen(const sp& display, + sp* outBuffer, bool& outCapturedSecureLayers, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, @@ -4860,7 +4861,8 @@ status_t SurfaceFlinger::captureScreen(const sp& display, sp& layerHandleBinder, @@ -4974,13 +4976,16 @@ status_t SurfaceFlinger::captureLayers(const sp& layerHandleBinder, visitor(layer); }); }; - return captureScreenCommon(renderArea, traverseLayers, outBuffer, false); + bool outCapturedSecureLayers = false; + return captureScreenCommon(renderArea, traverseLayers, outBuffer, false, + outCapturedSecureLayers); } status_t SurfaceFlinger::captureScreenCommon(RenderArea& renderArea, TraverseLayersFunction traverseLayers, sp* outBuffer, - bool useIdentityTransform) { + bool useIdentityTransform, + bool& outCapturedSecureLayers) { ATRACE_CALL(); renderArea.updateDimensions(mPrimaryDisplayOrientation); @@ -5018,7 +5023,8 @@ status_t SurfaceFlinger::captureScreenCommon(RenderArea& renderArea, Mutex::Autolock _l(mStateLock); renderArea.render([&]() { result = captureScreenImplLocked(renderArea, traverseLayers, (*outBuffer).get(), - useIdentityTransform, forSystem, &fd); + useIdentityTransform, forSystem, &fd, + outCapturedSecureLayers); }); } @@ -5169,21 +5175,19 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, status_t SurfaceFlinger::captureScreenImplLocked(const RenderArea& renderArea, TraverseLayersFunction traverseLayers, ANativeWindowBuffer* buffer, - bool useIdentityTransform, - bool forSystem, - int* outSyncFd) { + bool useIdentityTransform, bool forSystem, + int* outSyncFd, bool& outCapturedSecureLayers) { ATRACE_CALL(); - bool secureLayerIsVisible = false; - traverseLayers([&](Layer* layer) { - secureLayerIsVisible = secureLayerIsVisible || (layer->isVisible() && layer->isSecure()); + outCapturedSecureLayers = + outCapturedSecureLayers || (layer->isVisible() && layer->isSecure()); }); // We allow the system server to take screenshots of secure layers for // use in situations like the Screen-rotation animation and place // the impetus on WindowManager to not persist them. - if (secureLayerIsVisible && !forSystem) { + if (outCapturedSecureLayers && !forSystem) { ALOGW("FB is protected: PERMISSION_DENIED"); return PERMISSION_DENIED; } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 2577f179cb..60bf94ffec 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -422,6 +422,7 @@ private: virtual sp createDisplayEventConnection( ISurfaceComposer::VsyncSource vsyncSource = eVsyncSourceApp); virtual status_t captureScreen(const sp& display, sp* outBuffer, + bool& outCapturedSecureLayers, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform, ISurfaceComposer::Rotation rotation, bool captureSecureLayers); @@ -573,11 +574,11 @@ private: bool yswap, bool useIdentityTransform); status_t captureScreenCommon(RenderArea& renderArea, TraverseLayersFunction traverseLayers, sp* outBuffer, - bool useIdentityTransform); + bool useIdentityTransform, bool& outCapturedSecureLayers); status_t captureScreenImplLocked(const RenderArea& renderArea, TraverseLayersFunction traverseLayers, ANativeWindowBuffer* buffer, bool useIdentityTransform, - bool forSystem, int* outSyncFd); + bool forSystem, int* outSyncFd, bool& outCapturedSecureLayers); void traverseLayersInDisplay(const sp& display, int32_t minLayerZ, int32_t maxLayerZ, const LayerVector::Visitor& visitor); diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp index deca17799c..6ce2075a3d 100644 --- a/services/surfaceflinger/tests/Transaction_test.cpp +++ b/services/surfaceflinger/tests/Transaction_test.cpp @@ -941,9 +941,11 @@ TEST_F(LayerTransactionTest, SetFlagsSecureEUidSystem) { // Here we pass captureSecureLayers = true and since we are AID_SYSTEM we should be able // to receive them...we are expected to take care with the results. + bool outCapturedSecureLayers = false; ASSERT_EQ(NO_ERROR, - composer->captureScreen(mDisplay, &outBuffer, + composer->captureScreen(mDisplay, &outBuffer, outCapturedSecureLayers, Rect(), 0, 0, 0, INT_MAX, false, ISurfaceComposer::eRotateNone, true)); + ASSERT_EQ(true, outCapturedSecureLayers); ScreenCapture sc(outBuffer); sc.expectColor(Rect(0, 0, 32, 32), Color::RED); } -- cgit v1.2.3-59-g8ed1b