diff mbox

[OMAPZOOM] DSPBRIDGE bug in Bridge exit cleanup

Message ID 8F7AF80515AF0D4D93307E594F3CB40E2342BE9F@dlee03.ent.ti.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Kanigeri, Hari March 23, 2009, 6:49 p.m. UTC
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(-)

Comments

Ameya Palande March 24, 2009, 10:04 a.m. UTC | #1
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
Kanigeri, Hari March 24, 2009, 11:48 a.m. UTC | #2
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
Ameya Palande March 24, 2009, 11:55 a.m. UTC | #3
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.
--
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
Kanigeri, Hari March 24, 2009, 11:58 a.m. UTC | #4
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.

--
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
diff mbox

Patch

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