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 d470bcd..1161389 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 39472b1..06bdb38 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 eb57ea3..1cd3ef4 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 2dfe577..120c8f8 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,