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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 1 Dec 2013 12:33:41 +0100
From:	Pali Rohár <pali.rohar@...il.com>
To:	Pavel Machek <pavel@....cz>
Cc:	Greg KH <gregkh@...uxfoundation.org>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	Ivajlo Dimitrov <ivo.g.dimitrov.75@...il.com>,
	Ивайло Димитров 
	<freemangordon@....bg>, sre@...g0.de, omar.ramirez@...itl.com,
	tony@...mide.com, felipe.contreras@...il.com, s-anna@...com,
	nm@...com, ohad@...ery.com, stable@...r.kernel.org,
	linux-kernel@...r.kernel.org, nico@...lde.de
Subject: Re: [patch] Staging: tidspbridge: make mmap root-only so it is not a security problem

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@...il.com

Download attachment "signature.asc " of type "application/pgp-signature" (199 bytes)