[PATCH] [OMAPZOOM]DSPBRIDGE bug in Bridge exit cleanup

[PATCH] [OMAPZOOM]DSPBRIDGE bug in Bridge exit cleanup

[Kanigeri, Hari]

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

The Bridge exit resource cleanup is accessing Process
context pointer after it is freed. Fixed this by holding
the next Process context pointer in a temporary variable.

Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
---
 drivers/dsp/bridge/rmgr/drv_interface.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
index a756997..af98600 100644
--- a/drivers/dsp/bridge/rmgr/drv_interface.c
+++ b/drivers/dsp/bridge/rmgr/drv_interface.c
@@ -537,7 +537,7 @@ static void __exit bridge_exit(void)
 	DSP_STATUS dsp_status = DSP_SOK;
 	HANDLE hDrvObject = NULL;
 	struct PROCESS_CONTEXT    *pCtxtclosed = NULL;
-
+	struct PROCESS_CONTEXT    *pTmp = NULL;
 	GT_0trace(driverTrace, GT_ENTER, "-> driver_exit\n");
 
 
@@ -551,9 +551,10 @@ static void __exit bridge_exit(void)
 			 "process***%d\n", pCtxtclosed->pid);
 		DRV_RemoveAllResources(pCtxtclosed);
 		PROC_Detach(pCtxtclosed->hProcessor);
+		pTmp =  pCtxtclosed->next;
 		DRV_RemoveProcContext((struct DRV_OBJECT *)hDrvObject,
 				     pCtxtclosed, (void *)pCtxtclosed->pid);
-		pCtxtclosed = pCtxtclosed->next;;
+		pCtxtclosed = pTmp;
 	}
 func_cont:
 #ifndef CONFIG_DISABLE_BRIDGE_PM
-- 
1.5.4.3


Thank you,
Best regards,
Hari


[-- Attachment #2: 0001-OMAPZOOM-DSPBRIDGE-bug-in-Bridge-exit-cleanup.patch --]
[-- Type: application/octet-stream, Size: 1522 bytes --]

From 13b9355dcccd6184a4240d49868087ddaa782142 Mon Sep 17 00:00:00 2001
From: Hari Kanigeri <kanigeri@Matrix.(none)>
Date: Mon, 23 Mar 2009 10:04:53 -0600
Subject: [PATCH] [OMAPZOOM]DSPBRIDGE bug in Bridge exit cleanup

The Bridge exit resource cleanup is accessing process
context pointer after it is freed. Fixed this by holding
the next Process context pointer in a temporary variable.

Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
---
 drivers/dsp/bridge/rmgr/drv_interface.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
index a756997..af98600 100644
--- a/drivers/dsp/bridge/rmgr/drv_interface.c
+++ b/drivers/dsp/bridge/rmgr/drv_interface.c
@@ -537,7 +537,7 @@ static void __exit bridge_exit(void)
 	DSP_STATUS dsp_status = DSP_SOK;
 	HANDLE hDrvObject = NULL;
 	struct PROCESS_CONTEXT    *pCtxtclosed = NULL;
-
+	struct PROCESS_CONTEXT    *pTmp = NULL;
 	GT_0trace(driverTrace, GT_ENTER, "-> driver_exit\n");
 
 
@@ -551,9 +551,10 @@ static void __exit bridge_exit(void)
 			 "process***%d\n", pCtxtclosed->pid);
 		DRV_RemoveAllResources(pCtxtclosed);
 		PROC_Detach(pCtxtclosed->hProcessor);
+		pTmp =  pCtxtclosed->next;
 		DRV_RemoveProcContext((struct DRV_OBJECT *)hDrvObject,
 				     pCtxtclosed, (void *)pCtxtclosed->pid);
-		pCtxtclosed = pCtxtclosed->next;;
+		pCtxtclosed = pTmp;
 	}
 func_cont:
 #ifndef CONFIG_DISABLE_BRIDGE_PM
-- 
1.5.4.3

Re: [PATCH] [OMAPZOOM]DSPBRIDGE bug in Bridge exit cleanup

[Ameya Palande]

Hi Hari,

On Mon, Mar 23, 2009 at 8:49 PM, Kanigeri, Hari <h-kanigeri2@ti.com> wrote:
>                DRV_RemoveProcContext((struct DRV_OBJECT *)hDrvObject,
>                                     pCtxtclosed, (void *)pCtxtclosed->pid);

I am trying to understand this patch.
I see that pCtxtclosed is passed to DRV_RemoveProcContext() function
but it is not used anywhere inside it. I was expecting to see a
statement there which should free its memory.

Any pointers?

Cheers,
Ameya.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] [OMAPZOOM]DSPBRIDGE bug in Bridge exit cleanup

[Kanigeri, Hari]

Hi Amey,

> I am trying to understand this patch.
> I see that pCtxtclosed is passed to DRV_RemoveProcContext() function
> but it is not used anywhere inside it. I was expecting to see a
> statement there which should free its memory.

This pointer is getting freed indirectly through the pCtxt2 pointer. The call to the function "DRV_GetProcContext((u32)hProcess, hDRVObject, &pCtxt2, NULL, 0)" returns pCtxt2, which is same as pCtxtclosed that is passed to this function.
In a way, the call to DRV_GetProcContext is kind of redundant, but I think it is present here to provide the support to extract the Process Context when only the PID is passed in. 

Thank you,
Best regards,
Hari

> -----Original Message-----
> From: Ameya Palande [mailto:2ameya@gmail.com]
> Sent: Tuesday, March 24, 2009 5:05 AM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH] [OMAPZOOM]DSPBRIDGE bug in Bridge exit cleanup
> 
> Hi Hari,
> 
> On Mon, Mar 23, 2009 at 8:49 PM, Kanigeri, Hari <h-kanigeri2@ti.com>
> wrote:
> >                DRV_RemoveProcContext((struct DRV_OBJECT *)hDrvObject,
> >                                     pCtxtclosed, (void *)pCtxtclosed-
> >pid);
> 
> I am trying to understand this patch.
> I see that pCtxtclosed is passed to DRV_RemoveProcContext() function
> but it is not used anywhere inside it. I was expecting to see a
> statement there which should free its memory.
> 
> Any pointers?
> 
> Cheers,
> Ameya.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [OMAPZOOM]DSPBRIDGE bug in Bridge exit cleanup

[Ameya Palande]

Hi Hari,

ext Kanigeri, Hari wrote:
> Hi Amey,
> 
>> I am trying to understand this patch.
>> I see that pCtxtclosed is passed to DRV_RemoveProcContext() function
>> but it is not used anywhere inside it. I was expecting to see a
>> statement there which should free its memory.
> 
> This pointer is getting freed indirectly through the pCtxt2 pointer. The call to the function "DRV_GetProcContext((u32)hProcess, hDRVObject, &pCtxt2, NULL, 0)" returns pCtxt2, which is same as pCtxtclosed that is passed to this function.
> In a way, the call to DRV_GetProcContext is kind of redundant, but I think it is present here to provide the support to extract the Process Context when only the PID is passed in.

Thanks for the explanation :)
Is there any need to pass "HANDLE hPCtxt" to DRV_RemoveProcContext() function?

Cheers,
Ameya.

RE: [PATCH] [OMAPZOOM]DSPBRIDGE bug in Bridge exit cleanup

[Kanigeri, Hari]

Ameya,

> Is there any need to pass "HANDLE hPCtxt" to DRV_RemoveProcContext()
> function?
>

-- It should work even if you don't pass the hPCtxt. This function requires some rework to avoid this confusion. Thanks for bringing up this question.

Thank you,
Best regards,
Hari

> -----Original Message-----
> From: Ameya Palande [mailto:ameya.palande@nokia.com]
> Sent: Tuesday, March 24, 2009 6:56 AM
> To: Kanigeri, Hari
> Cc: Ameya Palande; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] [OMAPZOOM]DSPBRIDGE bug in Bridge exit cleanup
> 
> Hi Hari,
> 
> ext Kanigeri, Hari wrote:
> > Hi Amey,
> >
> >> I am trying to understand this patch.
> >> I see that pCtxtclosed is passed to DRV_RemoveProcContext() function
> >> but it is not used anywhere inside it. I was expecting to see a
> >> statement there which should free its memory.
> >
> > This pointer is getting freed indirectly through the pCtxt2 pointer. The
> call to the function "DRV_GetProcContext((u32)hProcess, hDRVObject,
> &pCtxt2, NULL, 0)" returns pCtxt2, which is same as pCtxtclosed that is
> passed to this function.
> > In a way, the call to DRV_GetProcContext is kind of redundant, but I
> think it is present here to provide the support to extract the Process
> Context when only the PID is passed in.
> 
> Thanks for the explanation :)
> Is there any need to pass "HANDLE hPCtxt" to DRV_RemoveProcContext()
> function?
> 
> Cheers,
> Ameya.