RE: [PATCH] DSPBRIDGE: check pointer before calling Proc_Detach

Ramos Falcon, Ernesto
2010-02-10T17:15:24Z

>-----Original Message-----
>From: Ameya Palande [mailto:ameya.palande@nokia.com]
>Sent: Wednesday, February 10, 2010 7:48 AM
>To: Ramirez Luna, Omar
>Cc: linux-omap; Doyu Hiroshi (Nokia-D/Helsinki); Contreras Felipe (Nokia-
>D/Helsinki); Menon, Nishanth; Ramos Falcon, Ernesto
>Subject: Re: [PATCH] DSPBRIDGE: check pointer before calling Proc_Detach
>
>Hi,
>
>On Thu, 2010-02-04 at 22:17 +0100, ext Omar Ramirez Luna wrote:
>> From: Ernest Ramos Falcon <ernesto@ti.com>
>>
>> This patch adds a check before calling Proc_Detach, otherwise
>> in case a failure occurs on PROC_Attach and hProcessor is
>> invalid when the userspace task closes its handle, unexpected
>> behavior might be seen as this pointer is dereferenced in
>> PROC_Detach.
>>
>> Signed-off-by: Ernest Ramos Falcon <ernesto@ti.com>
>> ---
>> drivers/dsp/bridge/rmgr/drv_interface.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c
>b/drivers/dsp/bridge/rmgr/drv_interface.c
>> index 32aff66..b0f0d93 100644
>> --- a/drivers/dsp/bridge/rmgr/drv_interface.c
>> +++ b/drivers/dsp/bridge/rmgr/drv_interface.c
>> @@ -539,7 +539,8 @@ static int bridge_release(struct inode *ip, struct
>file *filp)
>> pr_ctxt = filp->private_data;
>> flush_signals(current);
>> DRV_RemoveAllResources(pr_ctxt);
>> - PROC_Detach(pr_ctxt);
>> + if (pr_ctxt->hProcessor)
>> + PROC_Detach(pr_ctxt);
>> MEM_Free(pr_ctxt);
>>
>> filp->private_data = NULL;
>
>I guess PROC_Detach already checks for a valid hProcessor handle!
>Why we need this check here again?
>
Sorry, I think this applied for different version of dspbridge.

>Also if you go ahead with this patch, then how are you going to cover a
>use case where userspace application calls PROC_Attach() which fails,
>but without checking the failure userspace application calls
>PROC_Detach()!
>
PROC_Detach can not be called directly from user space, is deprecated; the only place where this is function is called is bridge_release, so nothing would happen. Anyway, dismiss this patch.

>Cheers,
>Ameya.