RE: [PATCH] DSPBRIDGE: Validate Processor Handle from user
On Tue, 2010-02-09 at 18:51 +0100, ext Ramos Falcon, Ernesto wrote: > > >-Original Message- > >From: Ameya Palande [mailto:ameya.pala...@nokia.com] > >Sent: Tuesday, February 09, 2010 11:27 AM > >To: Ramos Falcon, Ernesto > >Cc: linux-omap@vger.kernel.org; Contreras Felipe (Nokia-D/Helsinki); Doyu > >Hiroshi (Nokia-D/Helsinki) > >Subject: Re: [PATCH] DSPBRIDGE: Validate Processor Handle from user > > > >Hi Ernesto, > > > >On Tue, 2010-02-09 at 18:07 +0100, ext Ramos Falcon, Ernesto wrote: > >> From 07b9f6d30c9d363ba0c4cefded8068662e1048c4 Mon Sep 17 00:00:00 2001 > >> From: Ernesto Ramos > >> Date: Wed, 3 Feb 2010 19:43:31 -0600 > >> Subject: [PATCH] DSPBRIDGE: Validate Processor Handle from user. > >> > >> Add check to validate the Processor handle received > >> from user. > >> > >> Signed-off-by: Ernesto Ramos > >> --- > >> drivers/dsp/bridge/pmgr/wcd.c | 86 - > >> drivers/dsp/bridge/rmgr/proc.c | 280 ++ > >-- > >> 2 files changed, 179 insertions(+), 187 deletions(-) > > > >My understanding: In bridge_open() we allocate a new process_context and > >store it in filp->private_data which can't be modified / tampered by > >user space. > > > >If this understanding is correct, then why we need to perform any > >validation on data hold be process_context pointer stored in > >flip->private_data? > > > >If you don't trust hProcessor handle received from user space arguments > >then instead of using that we can just use pCtxt->hProcessor! > > > > Agree. We plan to remove the Proc Attach and remove the parameter hProcessor > handle passed to the user but we have not done it yet because it may impact > the API. > > >I don't understand why we need validation so NACK from my side. > > > > We have had some cases where we receive an invalid proc handle from user > which resulted in kernel panic. Why are we using a processor handle passed from user space? Instead of checking validity of this parameter can't we just use pCtxt->hProcessor? This way we can get rid of all the checks! Cheers, Ameya. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] DSPBRIDGE: Validate Processor Handle from user
>-Original Message- >From: Ameya Palande [mailto:ameya.pala...@nokia.com] >Sent: Tuesday, February 09, 2010 11:27 AM >To: Ramos Falcon, Ernesto >Cc: linux-omap@vger.kernel.org; Contreras Felipe (Nokia-D/Helsinki); Doyu >Hiroshi (Nokia-D/Helsinki) >Subject: Re: [PATCH] DSPBRIDGE: Validate Processor Handle from user > >Hi Ernesto, > >On Tue, 2010-02-09 at 18:07 +0100, ext Ramos Falcon, Ernesto wrote: >> From 07b9f6d30c9d363ba0c4cefded8068662e1048c4 Mon Sep 17 00:00:00 2001 >> From: Ernesto Ramos >> Date: Wed, 3 Feb 2010 19:43:31 -0600 >> Subject: [PATCH] DSPBRIDGE: Validate Processor Handle from user. >> >> Add check to validate the Processor handle received >> from user. >> >> Signed-off-by: Ernesto Ramos >> --- >> drivers/dsp/bridge/pmgr/wcd.c | 86 - >> drivers/dsp/bridge/rmgr/proc.c | 280 ++ >-- >> 2 files changed, 179 insertions(+), 187 deletions(-) > >My understanding: In bridge_open() we allocate a new process_context and >store it in filp->private_data which can't be modified / tampered by >user space. > >If this understanding is correct, then why we need to perform any >validation on data hold be process_context pointer stored in >flip->private_data? > >If you don't trust hProcessor handle received from user space arguments >then instead of using that we can just use pCtxt->hProcessor! > Agree. We plan to remove the Proc Attach and remove the parameter hProcessor handle passed to the user but we have not done it yet because it may impact the API. >I don't understand why we need validation so NACK from my side. > We have had some cases where we receive an invalid proc handle from user which resulted in kernel panic. >Cheers, >Ameya. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] DSPBRIDGE: Validate Processor Handle from user
Hi Ernesto, On Tue, 2010-02-09 at 18:07 +0100, ext Ramos Falcon, Ernesto wrote: > From 07b9f6d30c9d363ba0c4cefded8068662e1048c4 Mon Sep 17 00:00:00 2001 > From: Ernesto Ramos > Date: Wed, 3 Feb 2010 19:43:31 -0600 > Subject: [PATCH] DSPBRIDGE: Validate Processor Handle from user. > > Add check to validate the Processor handle received > from user. > > Signed-off-by: Ernesto Ramos > --- > drivers/dsp/bridge/pmgr/wcd.c | 86 - > drivers/dsp/bridge/rmgr/proc.c | 280 > ++-- > 2 files changed, 179 insertions(+), 187 deletions(-) My understanding: In bridge_open() we allocate a new process_context and store it in filp->private_data which can't be modified / tampered by user space. If this understanding is correct, then why we need to perform any validation on data hold be process_context pointer stored in flip->private_data? If you don't trust hProcessor handle received from user space arguments then instead of using that we can just use pCtxt->hProcessor! I don't understand why we need validation so NACK from my side. Cheers, Ameya. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
