Re: [patch] Staging: tidspbridge: make mmap root-only so it is not a security problem

["Pali Rohár" <pali.rohar@gmail.com>]

[-- Attachment #1: Type: Text/Plain, Size: 2950 bytes --]

On Sunday 01 December 2013 12:26:10 Pavel Machek wrote:
> Hi!
> 
> On Sat 2013-11-30 19:45:01, Greg KH wrote:
> > On Sat, Nov 30, 2013 at 11:58:23PM +0100, Pavel Machek wrote:
> > > On Sat 2013-11-30 14:05:53, Greg KH wrote:
> > > > On Sat, Nov 30, 2013 at 09:42:37PM +0100, Pavel Machek 
wrote:
> > > > > mmap in tidspbridge is missing range-checks. For now,
> > > > > make this interface root-only, so that it does not
> > > > > cause security problems.
> > > > 
> > > > Please fix this properly and don't paper over the real
> > > > problem here.
> > > 
> > > Well, if the driver is left uncompilable, I'm pretty sure
> > > it will bitrot.
> > > 
> > > I do have the hardware, but I'm pretty sure current
> > > mailine does not have enough support to boot Maemo, so it
> > > is non trivial for me to test changes here.
> > > 
> > > And yes, I'd like to get N900 to better shape, but there's
> > > more urgent work to do there. Such as "make sure N900
> > > still boots once omap moves away from device files".
> > > 
> > > [It seems like check should be that
> > > 
> > > vma->vm_pgoff << PAGE_SHIFT >= pdata->phys_mempool_base
> > > and vma->vm_end - vma->vm_start + (vma->vm_pgoff <<
> > > PAGE_SHIFT - pdata->phys_mempool_base) <=
> > > pdata->phys_mempool_size .
> > > 
> > > But... this is some kind of DSP coprocessor, and I am not
> > > sure if just exposing its address space to untrusted
> > > processes is good idea.
> > > 
> > > Heck, are you sure this is security problem in the first
> > > place? Yes, it is unchecked mmap. So what? If the device
> > > is 600 root.root, and if the DSP can take over main
> > > system,
> > > 
> > >         if (!capable(CAP_SYS_RAWIO))
> > >         
> > >                 return -EPERM;
> > 
> > Will that break userspace?  Who opens and mmaps this device?
> >  If you don't know if users do this, how can you say this
> > patch isn't going to break things just as much as not
> > having the driver there at all?
> 
> On maemo, /dev/DspBridge is mode 666. I tried looking up with
> fuser who might use it, but that one does not seem to work on
> maemo.
> 

Hi! See my previous email. gst-dsp plugin using /dev/DspBridge, 
so any application which using gstreamer for viewing videos will 
use it. Try for example builtin media player and some h264 video 
with low resolution. Or directly gst-launch.

> So yes, this would "break" existing users... OTOH maemo does
> not work on mainline kernels, and never did. (Maemo is not
> open source).
> 

If you apply some patches to kernel and also userspace, you can 
run Maemo with (patched) upstream kernel. Just install CSSU devel 
and kernel patches from linux-n900 tree. Then you can test it.

> Anyway, tell me what you prefer. We seem to have chicken and
> egg problem here... I can create the patch but not test it.
> 
> 								Pavel

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]