Skip to contentSkip to conversation

Interface tightening patches

PATCH Applied Omar Ramirez Luna 1 participant 7 messages Jan 14, 2010 – Jan 18, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover Interface tightening patches

Set of patches containing ioctl interface checks, null pointers checks and imporvement on resource handling on error.

Ernesto Ramos (3):
DSPBRIDGE: Interface tightening to check for invalid input
parameters
DSPBRIDGE: Undo allocation of resources in case of cp_to_usr fails
DSPBRIDGE: NULL Pointer Dereference fix

 arch/arm/plat-omap/include/dspbridge/dbdefs.h |    4 +-
 drivers/dsp/bridge/pmgr/wcd.c                 |  126 ++++++++++++++++++++-----
 drivers/dsp/bridge/rmgr/nldr.c                |    3 +-
 drivers/dsp/bridge/rmgr/node.c                |    6 +-
 drivers/dsp/bridge/rmgr/proc.c                |    9 +-
 drivers/dsp/bridge/services/mem.c             |    4 -
 drivers/dsp/bridge/wmd/chnl_sm.c              |    2 +-
 7 files changed, 113 insertions(+), 41 deletions(-)
CC Nishanth Menon <[email protected]>
CC Hiroshi Doyu <[email protected]>
CC Ameya Palande <[email protected]>
CC Fernando Guzman Lugo <[email protected]>
#1 lore raw

PATCH 1/3 DSPBRIDGE: Interface tightening to check for invalid input parameters

From: Ernesto Ramos <[email protected]>

Add more error checking and range to address usecases accessing kernel APIs directly.

CC Nishanth Menon <[email protected]>
CC Hiroshi Doyu <[email protected]>
CC Ameya Palande <[email protected]>
---
 arch/arm/plat-omap/include/dspbridge/dbdefs.h |    4 +-
 drivers/dsp/bridge/pmgr/wcd.c                 |   91 +++++++++++++++++++-----
 drivers/dsp/bridge/services/mem.c             |    4 -
 3 files changed, 75 insertions(+), 24 deletions(-)
arch/arm/plat-omap/include/dspbridge/dbdefs.h +3 -1
index acd291f..f77de39 100644
@@ -217,7 +217,9 @@
 		DSP_DCDLIBRARYTYPE,
 		DSP_DCDCREATELIBTYPE,
 		DSP_DCDEXECUTELIBTYPE,
-		DSP_DCDDELETELIBTYPE
+		DSP_DCDDELETELIBTYPE,
+		/* DSP_DCDMAXOBJTYPE is meant to be the last DCD object type */
+		DSP_DCDMAXOBJTYPE
 	} ;
 
 /* Processor states */
drivers/dsp/bridge/pmgr/wcd.c +72 -19

225 lines not on this page · raw message

drivers/dsp/bridge/services/mem.c +0 -4
index 85eb477..cdda1e8 100644
@@ -493,10 +493,6 @@ void MEM_FlushCache(void *pMemBuf, u32 cBytes, s32 FlushType)
 		outer_flush_range(__pa((u32)pMemBuf), __pa((u32)pMemBuf +
 				  cBytes));
 	break;
-	default:
-		GT_1trace(MEM_debugMask, GT_6CLASS, "MEM_FlushCache: invalid "
-			  "FlushMemType 0x%x\n", FlushType);
-	break;
 	}
 
 }
Signature · 1 line
1.6.2.4
#2 lore raw

PATCH 2/3 DSPBRIDGE: Undo allocation of resources in case of cp_to_usr fails

From: Ernesto Ramos <[email protected]>

Release resources allocated during MAP, Node Allocation, STRM Buf Allocation in the case of cp_to_usr fails.

CC Nishanth Menon <[email protected]>
CC Hiroshi Doyu <[email protected]>
CC Ameya Palande <[email protected]>
---
 drivers/dsp/bridge/pmgr/wcd.c |   35 ++++++++++++++++++++++++++++-------
 1 files changed, 28 insertions(+), 7 deletions(-)
drivers/dsp/bridge/pmgr/wcd.c +28 -7
index 006ca63..5062053 100644
@@ -1104,8 +1104,11 @@ u32 PROCWRAP_Map(union Trapped_Args *args, void *pr_ctxt)
 			 args->ARGS_PROC_MAPMEM.pReqAddr, &pMapAddr,
 			 args->ARGS_PROC_MAPMEM.ulMapAttr, pr_ctxt);
 	if (DSP_SUCCEEDED(status)) {
-		if (put_user(pMapAddr, args->ARGS_PROC_MAPMEM.ppMapAddr))
+		if (put_user(pMapAddr, args->ARGS_PROC_MAPMEM.ppMapAddr)) {
 			status = DSP_EINVALIDARG;
+			PROC_UnMap(args->ARGS_PROC_MAPMEM.hProcessor,
+				pMapAddr, pr_ctxt);
+		}
 
 	}
 	return status;
@@ -1150,9 +1153,13 @@ u32 PROCWRAP_ReserveMemory(union Trapped_Args *args, void *pr_ctxt)
 	GT_0trace(WCD_debugMask, GT_ENTER, "PROCWRAP_ReserveMemory: entered\n");
 	status = PROC_ReserveMemory(args->ARGS_PROC_RSVMEM.hProcessor,
 				   args->ARGS_PROC_RSVMEM.ulSize, &pRsvAddr);
-	if (put_user(pRsvAddr, args->ARGS_PROC_RSVMEM.ppRsvAddr))
-		status = DSP_EINVALIDARG;
-
+	if (DSP_SUCCEEDED(status)) {
+		if (put_user(pRsvAddr, args->ARGS_PROC_RSVMEM.ppRsvAddr)) {
+			status = DSP_EINVALIDARG;
+			PROC_UnReserveMemory(args->ARGS_PROC_RSVMEM.hProcessor,
+				pRsvAddr);
+		}
+	}
 	return status;
 }
 
@@ -1255,7 +1262,13 @@ u32 NODEWRAP_Allocate(union Trapped_Args *args, void *pr_ctxt)
 				      &nodeId, (struct DSP_CBDATA *)pArgs,
 				      pAttrIn, &hNode, pr_ctxt);
 	}
-	cp_to_usr(args->ARGS_NODE_ALLOCATE.phNode, &hNode, status, 1);
+	if (DSP_SUCCEEDED(status)) {
+		cp_to_usr(args->ARGS_NODE_ALLOCATE.phNode, &hNode, status, 1);
+		if (DSP_FAILED(status)) {
+			status = DSP_EPOINTER;
+			NODE_Delete(hNode, pr_ctxt);
+		}
+	}
 func_cont:
 	if (pArgs)
 		MEM_Free(pArgs);
@@ -1589,8 +1602,16 @@ u32 STRMWRAP_AllocateBuffer(union Trapped_Args *args, void *pr_ctxt)
 	status = STRM_AllocateBuffer(args->ARGS_STRM_ALLOCATEBUFFER.hStream,
 				     args->ARGS_STRM_ALLOCATEBUFFER.uSize,
 				     apBuffer, uNumBufs, pr_ctxt);
-	cp_to_usr(args->ARGS_STRM_ALLOCATEBUFFER.apBuffer, apBuffer, status,
-		 uNumBufs);
+	if (DSP_SUCCEEDED(status)) {
+		cp_to_usr(args->ARGS_STRM_ALLOCATEBUFFER.apBuffer, apBuffer,
+			status, uNumBufs);
+		if (DSP_FAILED(status)) {
+			status = DSP_EPOINTER;
+			STRM_FreeBuffer(
+				args->ARGS_STRM_ALLOCATEBUFFER.hStream,
+				apBuffer, uNumBufs, pr_ctxt);
+		}
+	}
 	if (apBuffer)
 		MEM_Free(apBuffer);
 
Signature · 1 line
1.6.2.4
#3 lore raw

PATCH 3/3 DSPBRIDGE: NULL Pointer Dereference fix

From: Ernesto Ramos <[email protected]>

This patch takes care of the possible null pointers dereferenced within dsp bridge driver.

CC Nishanth Menon <[email protected]>
CC Hiroshi Doyu <[email protected]>
CC Ameya Palande <[email protected]>
---
 drivers/dsp/bridge/rmgr/nldr.c   |    3 ++-
 drivers/dsp/bridge/rmgr/node.c   |    6 +++---
 drivers/dsp/bridge/rmgr/proc.c   |    9 ++++-----
 drivers/dsp/bridge/wmd/chnl_sm.c |    2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)
drivers/dsp/bridge/rmgr/nldr.c +2 -1
index 24eb9c6..e977a94 100644
@@ -1007,7 +1007,8 @@ DSP_STATUS NLDR_Unload(struct NLDR_NODEOBJECT *hNldrNode, enum NLDR_PHASE phase)
 				/* Unload main library */
 				pRootLib = &hNldrNode->root;
 			}
-			UnloadLib(hNldrNode, pRootLib);
+			if (pRootLib)
+				UnloadLib(hNldrNode, pRootLib);
 		} else {
 			if (hNldrNode->fOverlay)
 				UnloadOvly(hNldrNode, phase);
drivers/dsp/bridge/rmgr/node.c +3 -3
index 9127751..fd9e7cf 100644
@@ -906,7 +906,7 @@ DSP_STATUS NODE_ChangePriority(struct NODE_OBJECT *hNode, s32 nPriority)
 	GT_2trace(NODE_debugMask, GT_ENTER, "NODE_ChangePriority: "
 		 "hNode: 0x%x\tnPriority: %d\n", hNode, nPriority);
 
-	if (!MEM_IsValidHandle(hNode, NODE_SIGNATURE)) {
+	if (!MEM_IsValidHandle(hNode, NODE_SIGNATURE) || !hNode->hNodeMgr) {
 		GT_1trace(NODE_debugMask, GT_7CLASS,
 			 "Invalid NODE Handle: 0x%x\n", hNode);
 		status = DSP_EHANDLE;
@@ -2612,7 +2612,7 @@ DSP_STATUS NODE_Terminate(struct NODE_OBJECT *hNode, OUT DSP_STATUS *pStatus)
 
 	GT_1trace(NODE_debugMask, GT_ENTER,
 		 "NODE_Terminate: hNode: 0x%x\n", hNode);
-	if (!MEM_IsValidHandle(hNode, NODE_SIGNATURE)) {
+	if (!MEM_IsValidHandle(hNode, NODE_SIGNATURE) || !hNode->hNodeMgr) {
 		status = DSP_EHANDLE;
 		goto func_end;
 	}
@@ -3329,7 +3329,7 @@ DSP_STATUS NODE_GetUUIDProps(DSP_HPROCESSOR hProcessor,
 		 pNodeId, pNodeProps);
 
 	status = PROC_GetDevObject(hProcessor, &hDevObject);
-	if (DSP_SUCCEEDED(status) && hDevObject != NULL) {
+	if (!hDevObject) {
 		status = DEV_GetNodeManager(hDevObject, &hNodeMgr);
 		if (hNodeMgr == NULL) {
 			status = DSP_EHANDLE;
drivers/dsp/bridge/rmgr/proc.c +4 -5
index f88128e..6693651 100644
@@ -573,12 +573,11 @@ DSP_STATUS PROC_Detach(struct PROCESS_CONTEXT *pr_ctxt)
 	DSP_STATUS status = DSP_SOK;
 	struct PROC_OBJECT *pProcObject = NULL;
 
-	if (pr_ctxt && pr_ctxt->hProcessor)
-		pProcObject = (struct PROC_OBJECT *)pr_ctxt->hProcessor;
-
 	DBC_Require(cRefs > 0);
-	GT_1trace(PROC_DebugMask, GT_ENTER, "Entered PROC_Detach, args:\n\t"
-		"pr_ctxt->phProcessor:  0x%x\n", *pProcObject);
+	GT_0trace(PROC_DebugMask, GT_ENTER, "Entered PROC_Detach\n");
+
+	if (pr_ctxt)
+		pProcObject = (struct PROC_OBJECT *)pr_ctxt->hProcessor;
 
 	if (MEM_IsValidHandle(pProcObject, PROC_SIGNATURE)) {
 		/* Notify the Client */
drivers/dsp/bridge/wmd/chnl_sm.c +1 -1
index f0bd986..7c1d7f7 100644
@@ -326,7 +326,7 @@ DSP_STATUS WMD_CHNL_CancelIO(struct CHNL_OBJECT *hChnl)
 	struct CHNL_MGR *pChnlMgr = NULL;
 
 	/* Check args: */
-	if (MEM_IsValidHandle(pChnl, CHNL_SIGNATURE)) {
+	if (MEM_IsValidHandle(pChnl, CHNL_SIGNATURE) && pChnl->pChnlMgr) {
 		iChnl = pChnl->uId;
 		uMode = pChnl->uMode;
 		pChnlMgr = pChnl->pChnlMgr;
Signature · 1 line
1.6.2.4
author Applied
#4 lore raw

RE: [PATCH 3/3] DSPBRIDGE: NULL Pointer Dereference fix

Quoted 11 lines

From: Omar Ramirez Luna on Wednesday, January 13, 2010 7:11 PM

From: Ernesto Ramos <[email protected]>

This patch takes care of the possible null pointers dereferenced within dsp bridge driver.

CC Nishanth Menon <[email protected]>
CC Hiroshi Doyu <[email protected]>
CC Ameya Palande <[email protected]>
Quoted 7 lines

---

drivers/dsp/bridge/rmgr/nldr.c   |    3 ++-
drivers/dsp/bridge/rmgr/node.c   |    6 +++---
drivers/dsp/bridge/rmgr/proc.c   |    9 ++++-----
drivers/dsp/bridge/wmd/chnl_sm.c |    2 +-
4 files changed, 10 insertions(+), 10 deletions(-)

Pushed to dspbridge.

- omar

author Applied
#5 lore raw

RE: [PATCH 2/3] DSPBRIDGE: Undo allocation of resources in case of cp_to_usr fails

In reply to Omar Ramirez Luna
Quoted 7 lines

From: Ramirez Luna, Omar on Wednesday, January 13, 2010 7:11 PM

From: Ernesto Ramos <[email protected]>

Release resources allocated during MAP, Node Allocation, STRM Buf Allocation in the case of cp_to_usr fails.

Quoted 8 lines
CC Nishanth Menon <[email protected]>
CC Hiroshi Doyu <[email protected]>
CC Ameya Palande <[email protected]>

---
drivers/dsp/bridge/pmgr/wcd.c | 35 ++++++++++++++++++++++++++++-------

1 files changed, 28 insertions(+), 7 deletions(-)

Pushed to dspbridge

- omar

author Applied
#6 lore raw

RE: [PATCH 1/3] DSPBRIDGE: Interface tightening to check for invalid input parameters

In reply to Omar Ramirez Luna
Quoted 11 lines

From: Ramirez Luna, Omar on Wednesday, January 13, 2010 7:11 PM

From: Ernesto Ramos <[email protected]>

Add more error checking and range to address usecases accessing kernel APIs directly.

CC Nishanth Menon <[email protected]>
CC Hiroshi Doyu <[email protected]>
CC Ameya Palande <[email protected]>
Quoted 6 lines

---

arch/arm/plat-omap/include/dspbridge/dbdefs.h |    4 +-
drivers/dsp/bridge/pmgr/wcd.c                 |   91 +++++++++++++++++++-----
drivers/dsp/bridge/services/mem.c             |    4 -
3 files changed, 75 insertions(+), 24 deletions(-)

Pushed to dspbridge

- omar