From 49e5cace38ca405754e797a9f3af8fad2009e6d8 Mon Sep 17 00:00:00 2001 From: Mathew Karimpanal Date: Mon, 16 Apr 2012 20:02:20 -0700 Subject: [PATCH] libhwcomposer: Unregister layer handles that fail validation. Layer buffer handles that fail gralloc validation must be removed from hwcomposer's register of previous bypass and overlay handles. (cherry picked from commit 18c7f1951018b6632bd6e1a4957194c01423b6ee) Change-Id: Idaaf3557b05adb5b3e938e9cd46dac5b104b729e CRs-Fixed: 347157 --- libgralloc/gralloc_priv.h | 19 ++++++++- libhwcomposer/hwcomposer.cpp | 82 ++++++++++++++++++++---------------- 2 files changed, 64 insertions(+), 37 deletions(-) mode change 100755 => 100644 libhwcomposer/hwcomposer.cpp diff --git a/libgralloc/gralloc_priv.h b/libgralloc/gralloc_priv.h index 3db7786..0d1be56 100644 --- a/libgralloc/gralloc_priv.h +++ b/libgralloc/gralloc_priv.h @@ -380,7 +380,24 @@ struct private_handle_t { h->numInts != sNumInts || h->numFds != sNumFds || hnd->magic != sMagic) { - LOGE("invalid gralloc handle (at %p)", h); + LOGE("Invalid gralloc handle (at %p): " + "ver(%d/%d) ints(%d/%d) fds(%d/%d) magic(%c%c%c%c/%c%c%c%c)", + h, + h ? h->version : -1, sizeof(native_handle), + h ? h->numInts : -1, sNumInts, + h ? h->numFds : -1, sNumFds, + hnd ? (((hnd->magic >> 24) & 0xFF)? + ((hnd->magic >> 24) & 0xFF) : '-') : '?', + hnd ? (((hnd->magic >> 16) & 0xFF)? + ((hnd->magic >> 16) & 0xFF) : '-') : '?', + hnd ? (((hnd->magic >> 8) & 0xFF)? + ((hnd->magic >> 8) & 0xFF) : '-') : '?', + hnd ? (((hnd->magic >> 0) & 0xFF)? + ((hnd->magic >> 0) & 0xFF) : '-') : '?', + (sMagic >> 24) & 0xFF, + (sMagic >> 16) & 0xFF, + (sMagic >> 8) & 0xFF, + (sMagic >> 0) & 0xFF); return -EINVAL; } return 0; diff --git a/libhwcomposer/hwcomposer.cpp b/libhwcomposer/hwcomposer.cpp old mode 100755 new mode 100644 index f7fe1c4..8a1989f --- a/libhwcomposer/hwcomposer.cpp +++ b/libhwcomposer/hwcomposer.cpp @@ -179,25 +179,29 @@ int getLayerbypassIndex(hwc_layer_t* layer) } void unlockPreviousBypassBuffers(hwc_context_t* ctx) { - // Unlock the previous bypass buffers. We can blindly unlock the buffers here, - // because buffers will be in this list only if the lock was successfully acquired. - for(int i = 0; i < MAX_BYPASS_LAYERS && ctx->previousBypassHandle[i]; i++) { - private_handle_t *hnd = (private_handle_t*) ctx->previousBypassHandle[i]; - - // Validate the handle to make sure it hasn't been deallocated. - if (private_handle_t::validate(ctx->previousBypassHandle[i])) { + // Unlock the previous bypass buffers. We can blindly unlock the buffers + // here, because buffers will be in this list only if the lock was + // successfully acquired. + for(int i = 0; i < MAX_BYPASS_LAYERS; i++) { + private_handle_t *hnd = (private_handle_t*) ctx->previousBypassHandle[i]; + if (!hnd) continue; - } - // Check if the handle was locked previously - if (private_handle_t::PRIV_FLAGS_HWC_LOCK & hnd->flags) { - if (GENLOCK_FAILURE == genlock_unlock_buffer(ctx->previousBypassHandle[i])) { - LOGE("%s: genlock_unlock_buffer failed", __FUNCTION__); - } else { - ctx->previousBypassHandle[i] = NULL; - // Reset the lock flag - hnd->flags &= ~private_handle_t::PRIV_FLAGS_HWC_LOCK; - } - } + // Validate the handle to make sure it hasn't been deallocated. + if (private_handle_t::validate(hnd)) { + LOGE("%s: Unregistering invalid gralloc handle %p.", __FUNCTION__, hnd); + ctx->previousBypassHandle[i] = NULL; + continue; + } + // Check if the handle was locked previously + if (private_handle_t::PRIV_FLAGS_HWC_LOCK & hnd->flags) { + if (GENLOCK_FAILURE == genlock_unlock_buffer(hnd)) { + LOGE("%s: genlock_unlock_buffer failed", __FUNCTION__); + } else { + ctx->previousBypassHandle[i] = NULL; + // Reset the lock flag + hnd->flags &= ~private_handle_t::PRIV_FLAGS_HWC_LOCK; + } + } } } @@ -525,17 +529,20 @@ void closeExtraPipes(hwc_context_t* ctx) { //Unused pipes must be of higher z-order for (int i = pipes_used ; i < MAX_BYPASS_LAYERS; i++) { - if (ctx->previousBypassHandle[i]) { - private_handle_t *hnd = (private_handle_t*) ctx->previousBypassHandle[i]; - - if (!private_handle_t::validate(ctx->previousBypassHandle[i])) { - if (GENLOCK_FAILURE == genlock_unlock_buffer(ctx->previousBypassHandle[i])) { + private_handle_t *hnd = (private_handle_t*) ctx->previousBypassHandle[i]; + if (hnd) { + if (!private_handle_t::validate(hnd)) { + if (GENLOCK_FAILURE == genlock_unlock_buffer(hnd)) { LOGE("%s: genlock_unlock_buffer failed", __FUNCTION__); } else { ctx->previousBypassHandle[i] = NULL; ctx->bypassBufferLockState[i] = BYPASS_BUFFER_UNLOCKED; hnd->flags &= ~private_handle_t::PRIV_FLAGS_HWC_LOCK; } + } else { + LOGE("%s: Unregistering invalid gralloc handle %p.", + __FUNCTION__, hnd); + ctx->previousBypassHandle[i] = NULL; } } ctx->mOvUI[i]->closeChannel(); @@ -713,15 +720,18 @@ static int prepareOverlay(hwc_context_t *ctx, hwc_layer_t *layer, const int flag void unlockPreviousOverlayBuffer(hwc_context_t* ctx) { - private_handle_t *hnd = (private_handle_t*)ctx->previousOverlayHandle; - if (isBufferLocked(hnd) && !private_handle_t::validate(hnd)) { - if (GENLOCK_NO_ERROR == genlock_unlock_buffer(hnd)) { - //If previous is same as current, keep locked. - if(hnd != ctx->currentOverlayHandle) { - hnd->flags &= ~private_handle_t::PRIV_FLAGS_HWC_LOCK; + private_handle_t *hnd = (private_handle_t*) ctx->previousOverlayHandle; + if (hnd) { + // Validate the handle before attempting to use it. + if (!private_handle_t::validate(hnd) && isBufferLocked(hnd)) { + if (GENLOCK_NO_ERROR == genlock_unlock_buffer(hnd)) { + //If previous is same as current, keep locked. + if(hnd != ctx->currentOverlayHandle) { + hnd->flags &= ~private_handle_t::PRIV_FLAGS_HWC_LOCK; + } + } else { + LOGE("%s: genlock_unlock_buffer failed", __FUNCTION__); } - } else { - LOGE("%s: genlock_unlock_buffer failed", __FUNCTION__); } } ctx->previousOverlayHandle = ctx->currentOverlayHandle; @@ -1207,19 +1217,19 @@ static int drawLayerUsingCopybit(hwc_composer_device_t *dev, hwc_layer_t *layer, { hwc_context_t* ctx = (hwc_context_t*)(dev); if(!ctx) { - LOGE("drawLayerUsingCopybit null context "); + LOGE("%s: null context ", __FUNCTION__); return -1; } private_hwc_module_t* hwcModule = reinterpret_cast(dev->common.module); if(!hwcModule) { - LOGE("drawLayerUsingCopybit null module "); + LOGE("%s: null module ", __FUNCTION__); return -1; } private_handle_t *hnd = (private_handle_t *)layer->handle; if(!hnd) { - LOGE("drawLayerUsingCopybit invalid handle"); + LOGE("%s: invalid handle", __FUNCTION__); return -1; } @@ -1233,13 +1243,13 @@ static int drawLayerUsingCopybit(hwc_composer_device_t *dev, hwc_layer_t *layer, //render buffer android_native_buffer_t *renderBuffer = (android_native_buffer_t *)eglGetRenderBufferANDROID(dpy, surface); if (!renderBuffer) { - LOGE("eglGetRenderBufferANDROID returned NULL buffer"); + LOGE("%s: eglGetRenderBufferANDROID returned NULL buffer", __FUNCTION__); genlock_unlock_buffer(hnd); return -1; } private_handle_t *fbHandle = (private_handle_t *)renderBuffer->handle; if(!fbHandle) { - LOGE("Framebuffer handle is NULL"); + LOGE("%s: Framebuffer handle is NULL", __FUNCTION__); genlock_unlock_buffer(hnd); return -1; }