gpu: pvr: fix memory context refcount problem leading to leaked handle

Although there is an IOCTL interface for creating memory contexts, in
reality processes can create only a single context. Subsequent create
commands will return the same context _and_ the same handle, so this
resembles more of an 'open' command, except the somewhat orthodox way of
returning the same handle.

In addition there can be kernel only users of the context accounted for
by the current reference count of the context (ui32RefCount).

Removing the user handle should happen when the last process opening
(creating) the context calls the close (destroy) command on the handle.
At the moment the handle is removed only if there are no kernel side or
user space users of the context, which can lead to the handle being
leaked in the following case:

1. create memory context -> ctx_handle created, ctx_refcount=1
2. create buffer -> ctx_refcount=2
3. destroy memory context -> ctx_refcount=1, ctx_handle not removed
4. destroy buffer -> ctx_refcount=0, ctx_handle not removed

To avoid this add a counter tracking the number of opens, so we know
when to remove the handle.

Fixes: NB#245525 - Return value of pvr_put_ctx is not checked
Signed-off-by: Imre Deak <imre.deak@nokia.com>

diff --git a/pvr/bridged_pvr_bridge.c b/pvr/bridged_pvr_bridge.c
index d470bcd280e8abccd7a4662f360b514185dd35e1..1161389876b5ba058f29ea9495329532bee3091a 100644 (file)
--- a/pvr/bridged_pvr_bridge.c
+++ b/pvr/bridged_pvr_bridge.c
@@ -132,7 +132,7 @@ static int PVRSRVCreateDeviceMemContextBW(u32 ui32BridgeID,
        struct PVRSRV_PER_PROCESS_DATA *psPerProc)
 {
        void *hDevCookieInt;
-       void *hDevMemContextInt;
+       struct BM_CONTEXT *ctx;
        u32 i;
        IMG_BOOL bCreated;
 
@@ -151,8 +151,7 @@ static int PVRSRVCreateDeviceMemContextBW(u32 ui32BridgeID,
                return 0;
 
        psCreateDevMemContextOUT->eError = PVRSRVCreateDeviceMemContextKM(
-                               hDevCookieInt, psPerProc,
-                               &hDevMemContextInt,
+                               hDevCookieInt, psPerProc, (void *)&ctx,
                                &psCreateDevMemContextOUT->ui32ClientHeapCount,
                                &psCreateDevMemContextOUT->sHeapInfo[0],
                                &bCreated, pbSharedDeviceMemHeap);
@@ -162,18 +161,20 @@ static int PVRSRVCreateDeviceMemContextBW(u32 ui32BridgeID,
 
        if (bCreated) {
                PVRSRVAllocHandleNR(psPerProc->psHandleBase,
-                               &psCreateDevMemContextOUT->hDevMemContext,
-                               hDevMemContextInt,
+                               &psCreateDevMemContextOUT->hDevMemContext, ctx,
                                PVRSRV_HANDLE_TYPE_DEV_MEM_CONTEXT,
                                PVRSRV_HANDLE_ALLOC_FLAG_NONE);
+               ctx->open_count = 1;
        } else {
                psCreateDevMemContextOUT->eError =
                        PVRSRVFindHandle(psPerProc->psHandleBase,
-                               &psCreateDevMemContextOUT->hDevMemContext,
-                               hDevMemContextInt,
+                               &psCreateDevMemContextOUT->hDevMemContext, ctx,
                                PVRSRV_HANDLE_TYPE_DEV_MEM_CONTEXT);
                if (psCreateDevMemContextOUT->eError != PVRSRV_OK)
                        return 0;
+
+               WARN_ON_ONCE(!ctx->open_count);
+               ctx->open_count++;
        }
 
        for (i = 0; i < psCreateDevMemContextOUT->ui32ClientHeapCount; i++) {
@@ -225,8 +226,8 @@ static int PVRSRVDestroyDeviceMemContextBW(u32 ui32BridgeID,
        struct PVRSRV_PER_PROCESS_DATA *psPerProc)
 {
        void *hDevCookieInt;
-       void *hDevMemContextInt;
-       IMG_BOOL bDestroyed;
+       unsigned long ctx_handle;
+       struct BM_CONTEXT *ctx;
 
        PVRSRV_BRIDGE_ASSERT_CMD(ui32BridgeID,
                        PVRSRV_BRIDGE_DESTROY_DEVMEMCONTEXT);
@@ -239,26 +240,30 @@ static int PVRSRVDestroyDeviceMemContextBW(u32 ui32BridgeID,
        if (psRetOUT->eError != PVRSRV_OK)
                return 0;
 
+       ctx_handle = (unsigned long)psDestroyDevMemContextIN->hDevMemContext;
        psRetOUT->eError = PVRSRVLookupHandle(psPerProc->psHandleBase,
-                               &hDevMemContextInt,
-                               psDestroyDevMemContextIN->hDevMemContext,
-                               PVRSRV_HANDLE_TYPE_DEV_MEM_CONTEXT);
+                                           (void *)&ctx, (void *)ctx_handle,
+                                           PVRSRV_HANDLE_TYPE_DEV_MEM_CONTEXT);
 
        if (psRetOUT->eError != PVRSRV_OK)
                return 0;
 
-       psRetOUT->eError = PVRSRVDestroyDeviceMemContextKM(hDevCookieInt,
-                                               hDevMemContextInt, &bDestroyed);
+       if (!ctx->open_count) {
+               psRetOUT->eError = PVRSRV_ERROR_INVALID_PARAMS;
 
-       if (psRetOUT->eError != PVRSRV_OK)
                return 0;
+       }
 
-       if (bDestroyed)
+       if (!--ctx->open_count)
                psRetOUT->eError = PVRSRVReleaseHandle(psPerProc->psHandleBase,
-                                       psDestroyDevMemContextIN->
-                                       hDevMemContext,
+                                       (void *)ctx_handle,
                                        PVRSRV_HANDLE_TYPE_DEV_MEM_CONTEXT);
 
+       psRetOUT->eError = PVRSRVDestroyDeviceMemContextKM(hDevCookieInt, ctx);
+
+       if (psRetOUT->eError != PVRSRV_OK)
+               return 0;
+
        return 0;
 }
 

diff --git a/pvr/buffer_manager.h b/pvr/buffer_manager.h
index 39472b1ed3e81479c5a17e7e564856c33725b40c..06bdb387c12ae597474b2e197f57481750a60bfa 100644 (file)
--- a/pvr/buffer_manager.h
+++ b/pvr/buffer_manager.h
@@ -86,6 +86,7 @@ struct BM_CONTEXT {
        struct HASH_TABLE *pBufferHash;
        void *hResItem;
        u32 ui32RefCount;
+       int open_count;
        struct BM_CONTEXT *psNext;
 };
 

diff --git a/pvr/devicemem.c b/pvr/devicemem.c
index eb57ea3d517000ecca3943bebe28f05a900f0ca3..1cd3ef46f9e97dcf7ab05524a62bf280cebb9519 100644 (file)
--- a/pvr/devicemem.c
+++ b/pvr/devicemem.c
@@ -188,16 +188,11 @@ enum PVRSRV_ERROR PVRSRVCreateDeviceMemContextKM(void *hDevCookie,
 }
 
 enum PVRSRV_ERROR PVRSRVDestroyDeviceMemContextKM(void *hDevCookie,
-                                            void *hDevMemContext,
-                                            IMG_BOOL *pbDestroyed)
+                                            void *hDevMemContext)
 {
-       int destroyed;
-
        PVR_UNREFERENCED_PARAMETER(hDevCookie);
 
-       destroyed = pvr_put_ctx(hDevMemContext);
-       if (pbDestroyed)
-               *pbDestroyed = destroyed ? IMG_TRUE : IMG_FALSE;
+       pvr_put_ctx(hDevMemContext);
 
        return PVRSRV_OK;
 }

diff --git a/pvr/pvr_bridge_km.h b/pvr/pvr_bridge_km.h
index 2dfe577c7a51ec1d947a368f186e8d3c199a13a9..120c8f89d4014e0e1a828637635f11e9290dc887 100644 (file)
--- a/pvr/pvr_bridge_km.h
+++ b/pvr/pvr_bridge_km.h
@@ -90,7 +90,7 @@ enum PVRSRV_ERROR PVRSRVCreateDeviceMemContextKM(void *hDevCookie,
                IMG_BOOL *pbShared);
 
 enum PVRSRV_ERROR PVRSRVDestroyDeviceMemContextKM(void *hDevCookie,
-               void *hDevMemContext, IMG_BOOL *pbCreated);
+               void *hDevMemContext);
 
 enum PVRSRV_ERROR PVRSRVGetDeviceMemHeapInfoKM(void *hDevCookie,
                void *hDevMemContext, u32 *pui32ClientHeapCount,