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 for Android: free password hash cracker in your pocket

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

Message-ID: <20131201112610.GA19791@amd.pavel.ucw.cz>
Date:	Sun, 1 Dec 2013 12:26:10 +0100
From:	Pavel Machek <pavel@....cz>
To:	Greg KH <gregkh@...uxfoundation.org>
Cc:	Dan Carpenter <dan.carpenter@...cle.com>,
	Ivajlo Dimitrov <ivo.g.dimitrov.75@...il.com>,
	Ивайло Димитров 
	<freemangordon@....bg>, pali.rohar@...il.com, 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

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.

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

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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/