Hi, >-----Original Message----- >From: Hiroshi DOYU [mailto:Hiroshi.DOYU@xxxxxxxxx] >Sent: Tuesday, November 24, 2009 2:51 AM >To: Ramos Falcon, Ernesto >Cc: linux-omap@xxxxxxxxxxxxxxx; Ramirez Luna, Omar; imre.deak@xxxxxxxxx; >Shah, Bhavin >Subject: Re: [PATCH] DSPBRIDGE: Interface tightening to check for invalid > >Hi Ernesto, > >From: "ext Ramos Falcon, Ernesto" <ernesto@xxxxxx> >Subject: [PATCH] DSPBRIDGE: Interface tightening to check for invalid >Date: Mon, 23 Nov 2009 21:12:08 +0100 > >> From f28ec22e48a9b61ac00b7711e035e117bdc71aeb Mon Sep 17 00:00:00 2001 >> From: Ernesto Ramos <ernesto@xxxxxx> >> Date: Mon, 23 Nov 2009 12:16:45 -0600 >> Subject: [PATCH] DSPBRIDGE: Interface tightening to check for invalid >inputs. >> >> Move the error checking to kernel to address usecases >> accessing kernel API directly. > >"memory_check_vma()" wasn't implemented for normal ioctl parameter >checking, but one for bridge cache operations with kernel crash risk, >especially with the following patch: > > http://marc.info/?l=linux-omap&m=125810716419520&w=2 > >This is because these memory area is expected that: > > - its pages have been already populated. > - its pages shouldn't be swapped out. > >Otherwise there's risk of kernel crash at these cache operation, >although "find_vma()" and "follow_page()" may be a bit heavier >operation. > The intention with this patch is to further strengthen the dsp bridge driver. I already executed some test cases to measure performance and the impact seems to be ~ 0.3% for a big amount of memory maps and flush memory commands, the latency impact is not even noticeable. >For most of the normal cases, which just checks ioctl parameters, >normal "access_ok()" would be enough. If it tries to access kernel >address, just it returns error to userland. If it tries to access >invalid address in user address space, the process dies with >SIGSEGV. I think that normally it's ok that a process dies because of >invalid address access rather than performance degrade. > I intended to use access_ok() for the purpose to check ioctl parameters, however, access_ok() seems not to work, I was able to send any invalid address to the bridge driver and no errors where reported. It seems that access_ok is rejecting only addresses that belong to the kernel space but the user would be able to send invalid address anyway. >So I think that "memory_check_vma()" doesn't fit for normal ioctl >parameter checking("copy_from_user"/"copy_to_user"), but "access_ok()" >would work in most of normal cases. > >In macros, __cp_fm_usr()/__cp_to_usr(), adding "access_ok()" checking >for pointers may work fine, I guess. > access_ok() is already called within copy_to_user() but the verification is being done after most of the processing is done, the intentions is to add these checks before any processing happens, and as I mentioned before access_ok does not work for all cases. >> Signed-off-by: Ernesto Ramos <ernesto@xxxxxx> >> --- >> arch/arm/plat-omap/include/dspbridge/dbdefs.h | 3 +- >> drivers/dsp/bridge/pmgr/wcd.c | 263 >+++++++++++++++++++++++-- >> drivers/dsp/bridge/rmgr/proc.c | 42 ---- >> 3 files changed, 249 insertions(+), 59 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
