From 308ddba68bc59234fba9a509d81cc2708d195c3a Mon Sep 17 00:00:00 2001 From: chaviw Date: Tue, 11 Aug 2020 16:23:51 -0700 Subject: Check status for every ISurfaceComposer Parcel Add macro to check for the status when parceling and return the error if failed. Also log the error when it occurs. Updated all callers in ISurfaceComposer for transactions and screen capture to use the SAFE_PARCEL call. Also updated all parceling in LayerState. Test: Boots Fixes: 162604027 Fixes: 162604029 Fixes: 162247502 Fixes: 162246713 Change-Id: I30f1588a6b6d89d31a0a112681702ecf0cb5d845 --- libs/gui/ISurfaceComposer.cpp | 182 +++++------- libs/gui/LayerState.cpp | 458 +++++++++++++++-------------- libs/gui/include/gui/ISurfaceComposer.h | 13 +- libs/gui/include/gui/LayerState.h | 23 +- libs/gui/tests/Surface_test.cpp | 14 +- services/surfaceflinger/SurfaceFlinger.cpp | 5 +- services/surfaceflinger/SurfaceFlinger.h | 14 +- 7 files changed, 358 insertions(+), 351 deletions(-) diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index 4a12035979..2c50acccf8 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -66,42 +66,39 @@ public: return interface_cast(reply.readStrongBinder()); } - virtual void setTransactionState(const Vector& state, - const Vector& displays, uint32_t flags, - const sp& applyToken, - const InputWindowCommands& commands, - int64_t desiredPresentTime, - const client_cache_t& uncacheBuffer, bool hasListenerCallbacks, - const std::vector& listenerCallbacks) { + virtual status_t setTransactionState( + const Vector& state, const Vector& displays, + uint32_t flags, const sp& applyToken, const InputWindowCommands& commands, + int64_t desiredPresentTime, const client_cache_t& uncacheBuffer, + bool hasListenerCallbacks, const std::vector& listenerCallbacks) { Parcel data, reply; data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor()); - data.writeUint32(static_cast(state.size())); + SAFE_PARCEL(data.writeUint32, static_cast(state.size())); for (const auto& s : state) { - s.write(data); + SAFE_PARCEL(s.write, data); } - data.writeUint32(static_cast(displays.size())); + SAFE_PARCEL(data.writeUint32, static_cast(displays.size())); for (const auto& d : displays) { - d.write(data); + SAFE_PARCEL(d.write, data); } - data.writeUint32(flags); - data.writeStrongBinder(applyToken); - commands.write(data); - data.writeInt64(desiredPresentTime); - data.writeStrongBinder(uncacheBuffer.token.promote()); - data.writeUint64(uncacheBuffer.id); - data.writeBool(hasListenerCallbacks); + SAFE_PARCEL(data.writeUint32, flags); + SAFE_PARCEL(data.writeStrongBinder, applyToken); + SAFE_PARCEL(commands.write, data); + SAFE_PARCEL(data.writeInt64, desiredPresentTime); + SAFE_PARCEL(data.writeStrongBinder, uncacheBuffer.token.promote()); + SAFE_PARCEL(data.writeUint64, uncacheBuffer.id); + SAFE_PARCEL(data.writeBool, hasListenerCallbacks); - if (data.writeVectorSize(listenerCallbacks) == NO_ERROR) { - for (const auto& [listener, callbackIds] : listenerCallbacks) { - data.writeStrongBinder(listener); - data.writeInt64Vector(callbackIds); - } + SAFE_PARCEL(data.writeVectorSize, listenerCallbacks); + for (const auto& [listener, callbackIds] : listenerCallbacks) { + SAFE_PARCEL(data.writeStrongBinder, listener); + SAFE_PARCEL(data.writeInt64Vector, callbackIds); } - remote()->transact(BnSurfaceComposer::SET_TRANSACTION_STATE, data, &reply); + return remote()->transact(BnSurfaceComposer::SET_TRANSACTION_STATE, data, &reply); } virtual void bootFinished() @@ -116,45 +113,31 @@ public: Parcel data, reply; data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor()); - status_t result = args.write(data); - if (result != NO_ERROR) { - ALOGE("captureDisplay failed to parcel args: %d", result); - return result; - } - result = remote()->transact(BnSurfaceComposer::CAPTURE_DISPLAY, data, &reply); + SAFE_PARCEL(args.write, data); + status_t result = remote()->transact(BnSurfaceComposer::CAPTURE_DISPLAY, data, &reply); if (result != NO_ERROR) { ALOGE("captureDisplay failed to transact: %d", result); return result; } - result = reply.readInt32(); - if (result != NO_ERROR) { - ALOGE("captureDisplay failed to readInt32: %d", result); - return result; - } - captureResults.read(reply); - return result; + SAFE_PARCEL(captureResults.read, reply); + return NO_ERROR; } virtual status_t captureDisplay(uint64_t displayOrLayerStack, ScreenCaptureResults& captureResults) { Parcel data, reply; data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor()); - data.writeUint64(displayOrLayerStack); + SAFE_PARCEL(data.writeUint64, displayOrLayerStack) status_t result = remote()->transact(BnSurfaceComposer::CAPTURE_DISPLAY_BY_ID, data, &reply); if (result != NO_ERROR) { ALOGE("captureDisplay failed to transact: %d", result); return result; } - result = reply.readInt32(); - if (result != NO_ERROR) { - ALOGE("captureDisplay failed to readInt32: %d", result); - return result; - } - captureResults.read(reply); - return result; + SAFE_PARCEL(captureResults.read, reply); + return NO_ERROR; } virtual status_t captureLayers(const LayerCaptureArgs& args, @@ -162,25 +145,16 @@ public: Parcel data, reply; data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor()); - status_t result = args.write(data); - if (result != NO_ERROR) { - ALOGE("captureLayers failed to parcel args: %d", result); - return result; - } + SAFE_PARCEL(args.write, data); - result = remote()->transact(BnSurfaceComposer::CAPTURE_LAYERS, data, &reply); + status_t result = remote()->transact(BnSurfaceComposer::CAPTURE_LAYERS, data, &reply); if (result != NO_ERROR) { ALOGE("captureLayers failed to transact: %d", result); return result; } - result = reply.readInt32(); - if (result != NO_ERROR) { - ALOGE("captureLayers failed to readInt32: %d", result); - return result; - } - captureResults.read(reply); - return result; + SAFE_PARCEL(captureResults.read, reply); + return NO_ERROR; } virtual bool authenticateSurfaceTexture( @@ -1218,59 +1192,56 @@ status_t BnSurfaceComposer::onTransact( case SET_TRANSACTION_STATE: { CHECK_INTERFACE(ISurfaceComposer, data, reply); - size_t count = data.readUint32(); - if (count > data.dataSize()) { - return BAD_VALUE; - } + uint32_t count = 0; + SAFE_PARCEL_READ_SIZE(data.readUint32, &count, data.dataSize()); Vector state; state.setCapacity(count); for (size_t i = 0; i < count; i++) { ComposerState s; - if (s.read(data) == BAD_VALUE) { - return BAD_VALUE; - } + SAFE_PARCEL(s.read, data); state.add(s); } - count = data.readUint32(); - if (count > data.dataSize()) { - return BAD_VALUE; - } + SAFE_PARCEL_READ_SIZE(data.readUint32, &count, data.dataSize()); DisplayState d; Vector displays; displays.setCapacity(count); for (size_t i = 0; i < count; i++) { - if (d.read(data) == BAD_VALUE) { - return BAD_VALUE; - } + SAFE_PARCEL(d.read, data); displays.add(d); } - uint32_t stateFlags = data.readUint32(); - sp applyToken = data.readStrongBinder(); + uint32_t stateFlags = 0; + SAFE_PARCEL(data.readUint32, &stateFlags); + sp applyToken; + SAFE_PARCEL(data.readStrongBinder, &applyToken); InputWindowCommands inputWindowCommands; - inputWindowCommands.read(data); + SAFE_PARCEL(inputWindowCommands.read, data); - int64_t desiredPresentTime = data.readInt64(); + int64_t desiredPresentTime = 0; + SAFE_PARCEL(data.readInt64, &desiredPresentTime); client_cache_t uncachedBuffer; - uncachedBuffer.token = data.readStrongBinder(); - uncachedBuffer.id = data.readUint64(); + sp tmpBinder; + SAFE_PARCEL(data.readNullableStrongBinder, &tmpBinder); + uncachedBuffer.token = tmpBinder; + SAFE_PARCEL(data.readUint64, &uncachedBuffer.id); - bool hasListenerCallbacks = data.readBool(); + bool hasListenerCallbacks = false; + SAFE_PARCEL(data.readBool, &hasListenerCallbacks); std::vector listenerCallbacks; - int32_t listenersSize = data.readInt32(); + int32_t listenersSize = 0; + SAFE_PARCEL_READ_SIZE(data.readInt32, &listenersSize, data.dataSize()); for (int32_t i = 0; i < listenersSize; i++) { - auto listener = data.readStrongBinder(); + SAFE_PARCEL(data.readStrongBinder, &tmpBinder); std::vector callbackIds; - data.readInt64Vector(&callbackIds); - listenerCallbacks.emplace_back(listener, callbackIds); + SAFE_PARCEL(data.readInt64Vector, &callbackIds); + listenerCallbacks.emplace_back(tmpBinder, callbackIds); } - setTransactionState(state, displays, stateFlags, applyToken, inputWindowCommands, - desiredPresentTime, uncachedBuffer, hasListenerCallbacks, - listenerCallbacks); - return NO_ERROR; + return setTransactionState(state, displays, stateFlags, applyToken, inputWindowCommands, + desiredPresentTime, uncachedBuffer, hasListenerCallbacks, + listenerCallbacks); } case BOOT_FINISHED: { CHECK_INTERFACE(ISurfaceComposer, data, reply); @@ -1282,48 +1253,35 @@ status_t BnSurfaceComposer::onTransact( DisplayCaptureArgs args; ScreenCaptureResults captureResults; - status_t res = args.read(data); - if (res != NO_ERROR) { - reply->writeInt32(res); - return NO_ERROR; - } - - res = captureDisplay(args, captureResults); - - reply->writeInt32(res); + SAFE_PARCEL(args.read, data); + status_t res = captureDisplay(args, captureResults); if (res == NO_ERROR) { - captureResults.write(*reply); + SAFE_PARCEL(captureResults.write, *reply); } - return NO_ERROR; + return res; } case CAPTURE_DISPLAY_BY_ID: { CHECK_INTERFACE(ISurfaceComposer, data, reply); - uint64_t displayOrLayerStack = data.readUint64(); + uint64_t displayOrLayerStack = 0; + SAFE_PARCEL(data.readUint64, &displayOrLayerStack); ScreenCaptureResults captureResults; status_t res = captureDisplay(displayOrLayerStack, captureResults); - reply->writeInt32(res); if (res == NO_ERROR) { - captureResults.write(*reply); + SAFE_PARCEL(captureResults.write, *reply); } - return NO_ERROR; + return res; } case CAPTURE_LAYERS: { CHECK_INTERFACE(ISurfaceComposer, data, reply); LayerCaptureArgs args; ScreenCaptureResults captureResults; - status_t res = args.read(data); - if (res != NO_ERROR) { - reply->writeInt32(res); - return NO_ERROR; - } - - res = captureLayers(args, captureResults); - reply->writeInt32(res); + SAFE_PARCEL(args.read, data); + status_t res = captureLayers(args, captureResults); if (res == NO_ERROR) { - captureResults.write(*reply); + SAFE_PARCEL(captureResults.write, *reply); } - return NO_ERROR; + return res; } case AUTHENTICATE_SURFACE: { CHECK_INTERFACE(ISurfaceComposer, data, reply); diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 725d3cdfd5..62f7c2351e 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -30,181 +30,189 @@ namespace android { status_t layer_state_t::write(Parcel& output) const { - output.writeStrongBinder(surface); - output.writeUint64(what); - output.writeFloat(x); - output.writeFloat(y); - output.writeInt32(z); - output.writeUint32(w); - output.writeUint32(h); - output.writeUint32(layerStack); - output.writeFloat(alpha); - output.writeUint32(flags); - output.writeUint32(mask); - *reinterpret_cast( - output.writeInplace(sizeof(layer_state_t::matrix22_t))) = matrix; - output.write(crop_legacy); - output.writeStrongBinder(barrierHandle_legacy); - output.writeStrongBinder(reparentHandle); - output.writeUint64(frameNumber_legacy); - output.writeInt32(overrideScalingMode); - output.writeStrongBinder(IInterface::asBinder(barrierGbp_legacy)); - output.writeStrongBinder(relativeLayerHandle); - output.writeStrongBinder(parentHandleForChild); - output.writeFloat(color.r); - output.writeFloat(color.g); - output.writeFloat(color.b); + SAFE_PARCEL(output.writeStrongBinder, surface); + SAFE_PARCEL(output.writeUint64, what); + SAFE_PARCEL(output.writeFloat, x); + SAFE_PARCEL(output.writeFloat, y); + SAFE_PARCEL(output.writeInt32, z); + SAFE_PARCEL(output.writeUint32, w); + SAFE_PARCEL(output.writeUint32, h); + SAFE_PARCEL(output.writeUint32, layerStack); + SAFE_PARCEL(output.writeFloat, alpha); + SAFE_PARCEL(output.writeUint32, flags); + SAFE_PARCEL(output.writeUint32, mask); + SAFE_PARCEL(matrix.write, output); + SAFE_PARCEL(output.write, crop_legacy); + SAFE_PARCEL(output.writeStrongBinder, barrierHandle_legacy); + SAFE_PARCEL(output.writeStrongBinder, reparentHandle); + SAFE_PARCEL(output.writeUint64, frameNumber_legacy); + SAFE_PARCEL(output.writeInt32, overrideScalingMode); + SAFE_PARCEL(output.writeStrongBinder, IInterface::asBinder(barrierGbp_legacy)); + SAFE_PARCEL(output.writeStrongBinder, relativeLayerHandle); + SAFE_PARCEL(output.writeStrongBinder, parentHandleForChild); + SAFE_PARCEL(output.writeFloat, color.r); + SAFE_PARCEL(output.writeFloat, color.g); + SAFE_PARCEL(output.writeFloat, color.b); #ifndef NO_INPUT - inputHandle->writeToParcel(&output); + SAFE_PARCEL(inputHandle->writeToParcel, &output); #endif - output.write(transparentRegion); - output.writeUint32(transform); - output.writeBool(transformToDisplayInverse); - output.write(crop); - output.write(orientedDisplaySpaceRect); + SAFE_PARCEL(output.write, transparentRegion); + SAFE_PARCEL(output.writeUint32, transform); + SAFE_PARCEL(output.writeBool, transformToDisplayInverse); + SAFE_PARCEL(output.write, crop); + SAFE_PARCEL(output.write, orientedDisplaySpaceRect); + if (buffer) { - output.writeBool(true); - output.write(*buffer); + SAFE_PARCEL(output.writeBool, true); + SAFE_PARCEL(output.write, *buffer); } else { - output.writeBool(false); + SAFE_PARCEL(output.writeBool, false); } + if (acquireFence) { - output.writeBool(true); - output.write(*acquireFence); + SAFE_PARCEL(output.writeBool, true); + SAFE_PARCEL(output.write, *acquireFence); } else { - output.writeBool(false); + SAFE_PARCEL(output.writeBool, false); } - output.writeUint32(static_cast(dataspace)); - output.write(hdrMetadata); - output.write(surfaceDamageRegion); - output.writeInt32(api); + + SAFE_PARCEL(output.writeUint32, static_cast(dataspace)); + SAFE_PARCEL(output.write, hdrMetadata); + SAFE_PARCEL(output.write, surfaceDamageRegion); + SAFE_PARCEL(output.writeInt32, api); + if (sidebandStream) { - output.writeBool(true); - output.writeNativeHandle(sidebandStream->handle()); + SAFE_PARCEL(output.writeBool, true); + SAFE_PARCEL(output.writeNativeHandle, sidebandStream->handle()); } else { - output.writeBool(false); + SAFE_PARCEL(output.writeBool, false); } - memcpy(output.writeInplace(16 * sizeof(float)), - colorTransform.asArray(), 16 * sizeof(float)); - output.writeFloat(cornerRadius); - output.writeUint32(backgroundBlurRadius); - output.writeStrongBinder(cachedBuffer.token.promote()); - output.writeUint64(cachedBuffer.id); - output.writeParcelable(metadata); - - output.writeFloat(bgColorAlpha); - output.writeUint32(static_cast(bgColorDataspace)); - output.writeBool(colorSpaceAgnostic); - - auto err = output.writeVectorSize(listeners); - if (err) { - return err; - } + SAFE_PARCEL(output.write, colorTransform.asArray(), 16 * sizeof(float)); + SAFE_PARCEL(output.writeFloat, cornerRadius); + SAFE_PARCEL(output.writeUint32, backgroundBlurRadius); + SAFE_PARCEL(output.writeStrongBinder, cachedBuffer.token.promote()); + SAFE_PARCEL(output.writeUint64, cachedBuffer.id); + SAFE_PARCEL(output.writeParcelable, metadata); + SAFE_PARCEL(output.writeFloat, bgColorAlpha); + SAFE_PARCEL(output.writeUint32, static_cast(bgColorDataspace)); + SAFE_PARCEL(output.writeBool, colorSpaceAgnostic); + SAFE_PARCEL(output.writeVectorSize, listeners); for (auto listener : listeners) { - err = output.writeStrongBinder(listener.transactionCompletedListener); - if (err) { - return err; - } - err = output.writeInt64Vector(listener.callbackIds); - if (err) { - return err; - } - } - output.writeFloat(shadowRadius); - output.writeInt32(frameRateSelectionPriority); - output.writeFloat(frameRate); - output.writeByte(frameRateCompatibility); - output.writeUint32(fixedTransformHint); + SAFE_PARCEL(output.writeStrongBinder, listener.transactionCompletedListener); + SAFE_PARCEL(output.writeInt64Vector, listener.callbackIds); + } + SAFE_PARCEL(output.writeFloat, shadowRadius); + SAFE_PARCEL(output.writeInt32, frameRateSelectionPriority); + SAFE_PARCEL(output.writeFloat, frameRate); + SAFE_PARCEL(output.writeByte, frameRateCompatibility); + SAFE_PARCEL(output.writeUint32, fixedTransformHint); return NO_ERROR; } status_t layer_state_t::read(const Parcel& input) { - surface = input.readStrongBinder(); - what = input.readUint64(); - x = input.readFloat(); - y = input.readFloat(); - z = input.readInt32(); - w = input.readUint32(); - h = input.readUint32(); - layerStack = input.readUint32(); - alpha = input.readFloat(); - flags = static_cast(input.readUint32()); - mask = static_cast(input.readUint32()); - const void* matrix_data = input.readInplace(sizeof(layer_state_t::matrix22_t)); - if (matrix_data) { - matrix = *reinterpret_cast(matrix_data); - } else { - return BAD_VALUE; - } - input.read(crop_legacy); - barrierHandle_legacy = input.readStrongBinder(); - reparentHandle = input.readStrongBinder(); - frameNumber_legacy = input.readUint64(); - overrideScalingMode = input.readInt32(); - barrierGbp_legacy = interface_cast(input.readStrongBinder()); - relativeLayerHandle = input.readStrongBinder(); - parentHandleForChild = input.readStrongBinder(); - color.r = input.readFloat(); - color.g = input.readFloat(); - color.b = input.readFloat(); - + SAFE_PARCEL(input.readNullableStrongBinder, &surface); + SAFE_PARCEL(input.readUint64, &what); + SAFE_PARCEL(input.readFloat, &x); + SAFE_PARCEL(input.readFloat, &y); + SAFE_PARCEL(input.readInt32, &z); + SAFE_PARCEL(input.readUint32, &w); + SAFE_PARCEL(input.readUint32, &h); + SAFE_PARCEL(input.readUint32, &layerStack); + SAFE_PARCEL(input.readFloat, &alpha); + + uint32_t tmpUint32 = 0; + SAFE_PARCEL(input.readUint32, &tmpUint32); + flags = static_cast(tmpUint32); + + SAFE_PARCEL(input.readUint32, &tmpUint32); + mask = static_cast(tmpUint32); + + SAFE_PARCEL(matrix.read, input); + SAFE_PARCEL(input.read, crop_legacy); + SAFE_PARCEL(input.readNullableStrongBinder, &barrierHandle_legacy); + SAFE_PARCEL(input.readNullableStrongBinder, &reparentHandle); + SAFE_PARCEL(input.readUint64, &frameNumber_legacy); + SAFE_PARCEL(input.readInt32, &overrideScalingMode); + + sp tmpBinder; + SAFE_PARCEL(input.readNullableStrongBinder, &tmpBinder); + barrierGbp_legacy = interface_cast(tmpBinder); + + float tmpFloat = 0; + SAFE_PARCEL(input.readNullableStrongBinder, &relativeLayerHandle); + SAFE_PARCEL(input.readNullableStrongBinder, &parentHandleForChild); + SAFE_PARCEL(input.readFloat, &tmpFloat); + color.r = tmpFloat; + SAFE_PARCEL(input.readFloat, &tmpFloat); + color.g = tmpFloat; + SAFE_PARCEL(input.readFloat, &tmpFloat); + color.b = tmpFloat; #ifndef NO_INPUT - inputHandle->readFromParcel(&input); + SAFE_PARCEL(inputHandle->readFromParcel, &input); #endif - input.read(transparentRegion); - transform = input.readUint32(); - transformToDisplayInverse = input.readBool(); - input.read(crop); - input.read(orientedDisplaySpaceRect); - buffer = new GraphicBuffer(); - if (input.readBool()) { - input.read(*buffer); - } - acquireFence = new Fence(); - if (input.readBool()) { - input.read(*acquireFence); - } - dataspace = static_cast(input.readUint32()); - input.read(hdrMetadata); - input.read(surfaceDamageRegion); - api = input.readInt32(); - if (input.readBool()) { - sidebandStream = NativeHandle::create(input.readNativeHandle(), true); + SAFE_PARCEL(input.read, transparentRegion); + SAFE_PARCEL(input.readUint32, &transform); + SAFE_PARCEL(input.readBool, &transformToDisplayInverse); + SAFE_PARCEL(input.read, crop); + SAFE_PARCEL(input.read, orientedDisplaySpaceRect); + + bool tmpBool = false; + SAFE_PARCEL(input.readBool, &tmpBool); + if (tmpBool) { + buffer = new GraphicBuffer(); + SAFE_PARCEL(input.read, *buffer); } - const void* color_transform_data = input.readInplace(16 * sizeof(float)); - if (color_transform_data) { - colorTransform = mat4(static_cast(color_transform_data)); - } else { - return BAD_VALUE; + SAFE_PARCEL(input.readBool, &tmpBool); + if (tmpBool) { + acquireFence = new Fence(); + SAFE_PARCEL(input.read, *acquireFence); } - cornerRadius = input.readFloat(); - backgroundBlurRadius = input.readUint32(); - cachedBuffer.token = input.readStrongBinder(); - cachedBuffer.id = input.readUint64(); - input.readParcelable(&metadata); - bgColorAlpha = input.readFloat(); - bgColorDataspace = static_cast(input.readUint32()); - colorSpaceAgnostic = input.readBool(); + SAFE_PARCEL(input.readUint32, &tmpUint32); + dataspace = static_cast(tmpUint32); - int32_t numListeners = input.readInt32(); + SAFE_PARCEL(input.read, hdrMetadata); + SAFE_PARCEL(input.read, surfaceDamageRegion); + SAFE_PARCEL(input.readInt32, &api); + SAFE_PARCEL(input.readBool, &tmpBool); + if (tmpBool) { + sidebandStream = NativeHandle::create(input.readNativeHandle(), true); + } + + SAFE_PARCEL(input.read, &colorTransform, 16 * sizeof(float)); + SAFE_PARCEL(input.readFloat, &cornerRadius); + SAFE_PARCEL(input.readUint32, &backgroundBlurRadius); + SAFE_PARCEL(input.readNullableStrongBinder, &tmpBinder); + cachedBuffer.token = tmpBinder; + SAFE_PARCEL(input.readUint64, &cachedBuffer.id); + SAFE_PARCEL(input.readParcelable, &metadata); + + SAFE_PARCEL(input.readFloat, &bgColorAlpha); + SAFE_PARCEL(input.readUint32, &tmpUint32); + bgColorDataspace = static_cast(tmpUint32); + SAFE_PARCEL(input.readBool, &colorSpaceAgnostic); + + int32_t numListeners = 0; + SAFE_PARCEL_READ_SIZE(input.readInt32, &numListeners, input.dataSize()); listeners.clear(); for (int i = 0; i < numListeners; i++) { - auto listener = input.readStrongBinder(); + sp listener; std::vector callbackIds; - input.readInt64Vector(&callbackIds); + SAFE_PARCEL(input.readNullableStrongBinder, &listener); + SAFE_PARCEL(input.readInt64Vector, &callbackIds); listeners.emplace_back(listener, callbackIds); } - shadowRadius = input.readFloat(); - frameRateSelectionPriority = input.readInt32(); - frameRate = input.readFloat(); - frameRateCompatibility = input.readByte(); - fixedTransformHint = static_cast(input.readUint32()); + SAFE_PARCEL(input.readFloat, &shadowRadius); + SAFE_PARCEL(input.readInt32, &frameRateSelectionPriority); + SAFE_PARCEL(input.readFloat, &frameRate); + SAFE_PARCEL(input.readByte, &frameRateCompatibility); + SAFE_PARCEL(input.readUint32, &tmpUint32); + fixedTransformHint = static_cast(tmpUint32); return NO_ERROR; } @@ -225,28 +233,34 @@ DisplayState::DisplayState() height(0) {} status_t DisplayState::write(Parcel& output) const { - output.writeStrongBinder(token); - output.writeStrongBinder(IInterface::asBinder(surface)); - output.writeUint32(what); - output.writeUint32(layerStack); - output.writeUint32(toRotationInt(orientation)); - output.write(layerStackSpaceRect); - output.write(orientedDisplaySpaceRect); - output.writeUint32(width); - output.writeUint32(height); + SAFE_PARCEL(output.writeStrongBinder, token); + SAFE_PARCEL(output.writeStrongBinder, IInterface::asBinder(surface)); + SAFE_PARCEL(output.writeUint32, what); + SAFE_PARCEL(output.writeUint32, layerStack); + SAFE_PARCEL(output.writeUint32, toRotationInt(orientation)); + SAFE_PARCEL(output.write, layerStackSpaceRect); + SAFE_PARCEL(output.write, orientedDisplaySpaceRect); + SAFE_PARCEL(output.writeUint32, width); + SAFE_PARCEL(output.writeUint32, height); return NO_ERROR; } status_t DisplayState::read(const Parcel& input) { - token = input.readStrongBinder(); - surface = interface_cast(input.readStrongBinder()); - what = input.readUint32(); - layerStack = input.readUint32(); - orientation = ui::toRotation(input.readUint32()); - input.read(layerStackSpaceRect); - input.read(orientedDisplaySpaceRect); - width = input.readUint32(); - height = input.readUint32(); + SAFE_PARCEL(input.readStrongBinder, &token); + sp tmpBinder; + SAFE_PARCEL(input.readNullableStrongBinder, &tmpBinder); + surface = interface_cast(tmpBinder); + + SAFE_PARCEL(input.readUint32, &what); + SAFE_PARCEL(input.readUint32, &layerStack); + uint32_t tmpUint = 0; + SAFE_PARCEL(input.readUint32, &tmpUint); + orientation = ui::toRotation(tmpUint); + + SAFE_PARCEL(input.read, layerStackSpaceRect); + SAFE_PARCEL(input.read, orientedDisplaySpaceRect); + SAFE_PARCEL(input.readUint32, &width); + SAFE_PARCEL(input.readUint32, &height); return NO_ERROR; } @@ -449,6 +463,22 @@ void layer_state_t::merge(const layer_state_t& other) { } } +status_t layer_state_t::matrix22_t::write(Parcel& output) const { + SAFE_PARCEL(output.writeFloat, dsdx); + SAFE_PARCEL(output.writeFloat, dtdx); + SAFE_PARCEL(output.writeFloat, dtdy); + SAFE_PARCEL(output.writeFloat, dsdy); + return NO_ERROR; +} + +status_t layer_state_t::matrix22_t::read(const Parcel& input) { + SAFE_PARCEL(input.readFloat, &dsdx); + SAFE_PARCEL(input.readFloat, &dtdx); + SAFE_PARCEL(input.readFloat, &dtdy); + SAFE_PARCEL(input.readFloat, &dsdy); + return NO_ERROR; +} + // ------------------------------- InputWindowCommands ---------------------------------------- bool InputWindowCommands::merge(const InputWindowCommands& other) { @@ -470,18 +500,20 @@ void InputWindowCommands::clear() { syncInputWindows = false; } -void InputWindowCommands::write(Parcel& output) const { +status_t InputWindowCommands::write(Parcel& output) const { #ifndef NO_INPUT - output.writeParcelableVector(focusRequests); + SAFE_PARCEL(output.writeParcelableVector, focusRequests); #endif - output.writeBool(syncInputWindows); + SAFE_PARCEL(output.writeBool, syncInputWindows); + return NO_ERROR; } -void InputWindowCommands::read(const Parcel& input) { +status_t InputWindowCommands::read(const Parcel& input) { #ifndef NO_INPUT - input.readParcelableVector(&focusRequests); + SAFE_PARCEL(input.readParcelableVector, &focusRequests); #endif - syncInputWindows = input.readBool(); + SAFE_PARCEL(input.readBool, &syncInputWindows); + return NO_ERROR; } bool ValidateFrameRate(float frameRate, int8_t compatibility, const char* inFunctionName) { @@ -504,93 +536,91 @@ bool ValidateFrameRate(float frameRate, int8_t compatibility, const char* inFunc // ---------------------------------------------------------------------------- status_t CaptureArgs::write(Parcel& output) const { - status_t status = output.writeInt32(static_cast(pixelFormat)) ?: - output.write(sourceCrop) ?: - output.writeFloat(frameScale) ?: - output.writeBool(captureSecureLayers) ?: - output.writeInt32(uid); - return status; + SAFE_PARCEL(output.writeInt32, static_cast(pixelFormat)); + SAFE_PARCEL(output.write, sourceCrop); + SAFE_PARCEL(output.writeFloat, frameScale); + SAFE_PARCEL(output.writeBool, captureSecureLayers); + SAFE_PARCEL(output.writeInt32, uid); + return NO_ERROR; } status_t CaptureArgs::read(const Parcel& input) { int32_t format = 0; - status_t status = input.readInt32(&format) ?: - input.read(sourceCrop) ?: - input.readFloat(&frameScale) ?: - input.readBool(&captureSecureLayers) ?: - input.readInt32(&uid); - + SAFE_PARCEL(input.readInt32, &format); pixelFormat = static_cast(format); - return status; + SAFE_PARCEL(input.read, sourceCrop); + SAFE_PARCEL(input.readFloat, &frameScale); + SAFE_PARCEL(input.readBool, &captureSecureLayers); + SAFE_PARCEL(input.readInt32, &uid); + + return NO_ERROR; } status_t DisplayCaptureArgs::write(Parcel& output) const { - status_t status = CaptureArgs::write(output); + SAFE_PARCEL(CaptureArgs::write, output); - status |= output.writeStrongBinder(displayToken) ?: - output.writeUint32(width) ?: - output.writeUint32(height) ?: - output.writeBool(useIdentityTransform); - return status; + SAFE_PARCEL(output.writeStrongBinder, displayToken); + SAFE_PARCEL(output.writeUint32, width); + SAFE_PARCEL(output.writeUint32, height); + SAFE_PARCEL(output.writeBool, useIdentityTransform); + return NO_ERROR; } status_t DisplayCaptureArgs::read(const Parcel& input) { - status_t status = CaptureArgs::read(input); + SAFE_PARCEL(CaptureArgs::read, input); - status |= input.readStrongBinder(&displayToken) ?: - input.readUint32(&width) ?: - input.readUint32(&height) ?: - input.readBool(&useIdentityTransform); - return status; + SAFE_PARCEL(input.readStrongBinder, &displayToken); + SAFE_PARCEL(input.readUint32, &width); + SAFE_PARCEL(input.readUint32, &height); + SAFE_PARCEL(input.readBool, &useIdentityTransform); + return NO_ERROR; } status_t LayerCaptureArgs::write(Parcel& output) const { - status_t status = CaptureArgs::write(output); + SAFE_PARCEL(CaptureArgs::write, output); - status |= output.writeStrongBinder(layerHandle); - status |= output.writeInt32(excludeHandles.size()); + SAFE_PARCEL(output.writeStrongBinder, layerHandle); + SAFE_PARCEL(output.writeInt32, excludeHandles.size()); for (auto el : excludeHandles) { - status |= output.writeStrongBinder(el); + SAFE_PARCEL(output.writeStrongBinder, el); } - status |= output.writeBool(childrenOnly); - return status; + SAFE_PARCEL(output.writeBool, childrenOnly); + return NO_ERROR; } status_t LayerCaptureArgs::read(const Parcel& input) { - status_t status = CaptureArgs::read(input); + SAFE_PARCEL(CaptureArgs::read, input); - status |= input.readStrongBinder(&layerHandle); + SAFE_PARCEL(input.readStrongBinder, &layerHandle); int32_t numExcludeHandles = 0; - status |= input.readInt32(&numExcludeHandles); + SAFE_PARCEL_READ_SIZE(input.readInt32, &numExcludeHandles, input.dataSize()); excludeHandles.reserve(numExcludeHandles); for (int i = 0; i < numExcludeHandles; i++) { sp binder; - status |= input.readStrongBinder(&binder); + SAFE_PARCEL(input.readStrongBinder, &binder); excludeHandles.emplace(binder); } - status |= input.readBool(&childrenOnly); - return status; + SAFE_PARCEL(input.readBool, &childrenOnly); + return NO_ERROR; } status_t ScreenCaptureResults::write(Parcel& output) const { - status_t status = output.write(*buffer) ?: - output.writeBool(capturedSecureLayers) ?: - output.writeUint32(static_cast(capturedDataspace)); - return status; + SAFE_PARCEL(output.write, *buffer); + SAFE_PARCEL(output.writeBool, capturedSecureLayers); + SAFE_PARCEL(output.writeUint32, static_cast(capturedDataspace)); + return NO_ERROR; } status_t ScreenCaptureResults::read(const Parcel& input) { buffer = new GraphicBuffer(); + SAFE_PARCEL(input.read, *buffer); + SAFE_PARCEL(input.readBool, &capturedSecureLayers); uint32_t dataspace = 0; - status_t status = input.read(*buffer) ?: - input.readBool(&capturedSecureLayers) ?: - input.readUint32(&dataspace); - + SAFE_PARCEL(input.readUint32, &dataspace); capturedDataspace = static_cast(dataspace); - - return status; + return NO_ERROR; } }; // namespace android diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index 926a66fff3..e955ea8f64 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -150,13 +150,12 @@ public: } /* open/close transactions. requires ACCESS_SURFACE_FLINGER permission */ - virtual void setTransactionState(const Vector& state, - const Vector& displays, uint32_t flags, - const sp& applyToken, - const InputWindowCommands& inputWindowCommands, - int64_t desiredPresentTime, - const client_cache_t& uncacheBuffer, bool hasListenerCallbacks, - const std::vector& listenerCallbacks) = 0; + virtual status_t setTransactionState( + const Vector& state, const Vector& displays, + uint32_t flags, const sp& applyToken, + const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime, + const client_cache_t& uncacheBuffer, bool hasListenerCallbacks, + const std::vector& listenerCallbacks) = 0; /* signal that we're done booting. * Requires ACCESS_SURFACE_FLINGER permission diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 187e4786a8..653d8493c1 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -16,6 +16,23 @@ #ifndef ANDROID_SF_LAYER_STATE_H #define ANDROID_SF_LAYER_STATE_H +#define SAFE_PARCEL(FUNC, ...) \ + { \ + status_t error = FUNC(__VA_ARGS__); \ + if (error) { \ + ALOGE("ERROR(%d). Failed to call parcel %s(%s)", error, #FUNC, #__VA_ARGS__); \ + return error; \ + } \ + } + +#define SAFE_PARCEL_READ_SIZE(FUNC, COUNT, SIZE) \ + { \ + SAFE_PARCEL(FUNC, COUNT); \ + if (static_cast(*COUNT) > SIZE) { \ + ALOGE("ERROR(BAD_VALUE). %s was greater than dataSize", #COUNT); \ + return BAD_VALUE; \ + } \ + } #include #include @@ -156,6 +173,8 @@ struct layer_state_t { float dtdx{0}; float dtdy{0}; float dsdy{0}; + status_t write(Parcel& output) const; + status_t read(const Parcel& input); }; sp surface; uint64_t what; @@ -293,8 +312,8 @@ struct InputWindowCommands { // Merges the passed in commands and returns true if there were any changes. bool merge(const InputWindowCommands& other); void clear(); - void write(Parcel& output) const; - void read(const Parcel& input); + status_t write(Parcel& output) const; + status_t read(const Parcel& input); }; static inline int compare_type(const ComposerState& lhs, const ComposerState& rhs) { diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 2f8e412ff4..9fd8c42a89 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -681,13 +681,13 @@ public: void destroyDisplay(const sp& /*display */) override {} std::vector getPhysicalDisplayIds() const override { return {}; } sp getPhysicalDisplayToken(PhysicalDisplayId) const override { return nullptr; } - void setTransactionState(const Vector& /*state*/, - const Vector& /*displays*/, uint32_t /*flags*/, - const sp& /*applyToken*/, - const InputWindowCommands& /*inputWindowCommands*/, - int64_t /*desiredPresentTime*/, const client_cache_t& /*cachedBuffer*/, - bool /*hasListenerCallbacks*/, - const std::vector& /*listenerCallbacks*/) override { + status_t setTransactionState( + const Vector& /*state*/, const Vector& /*displays*/, + uint32_t /*flags*/, const sp& /*applyToken*/, + const InputWindowCommands& /*inputWindowCommands*/, int64_t /*desiredPresentTime*/, + const client_cache_t& /*cachedBuffer*/, bool /*hasListenerCallbacks*/, + const std::vector& /*listenerCallbacks*/) override { + return NO_ERROR; } void bootFinished() override {} diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f6028d5227..b12158c6f0 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3356,7 +3356,7 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(int64_t desiredPresentTime, return true; } -void SurfaceFlinger::setTransactionState( +status_t SurfaceFlinger::setTransactionState( const Vector& states, const Vector& displays, uint32_t flags, const sp& applyToken, const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime, const client_cache_t& uncacheBuffer, bool hasListenerCallbacks, @@ -3402,12 +3402,13 @@ void SurfaceFlinger::setTransactionState( hasListenerCallbacks, listenerCallbacks, originPID, originUID); setTransactionFlags(eTransactionFlushNeeded); - return; + return NO_ERROR; } applyTransactionState(states, displays, flags, inputWindowCommands, desiredPresentTime, uncacheBuffer, postTime, privileged, hasListenerCallbacks, listenerCallbacks, originPID, originUID, /*isMainThread*/ false); + return NO_ERROR; } void SurfaceFlinger::applyTransactionState( diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 24837b83f7..6497f6b46c 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -411,13 +411,13 @@ private: void destroyDisplay(const sp& displayToken) override; std::vector getPhysicalDisplayIds() const override; sp getPhysicalDisplayToken(PhysicalDisplayId displayId) const override; - void setTransactionState(const Vector& state, - const Vector& displays, uint32_t flags, - const sp& applyToken, - const InputWindowCommands& inputWindowCommands, - int64_t desiredPresentTime, const client_cache_t& uncacheBuffer, - bool hasListenerCallbacks, - const std::vector& listenerCallbacks) override; + status_t setTransactionState(const Vector& state, + const Vector& displays, uint32_t flags, + const sp& applyToken, + const InputWindowCommands& inputWindowCommands, + int64_t desiredPresentTime, const client_cache_t& uncacheBuffer, + bool hasListenerCallbacks, + const std::vector& listenerCallbacks) override; void bootFinished() override; bool authenticateSurfaceTexture( const sp& bufferProducer) const override; -- cgit v1.2.3-59-g8ed1b