[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-kernel
Subject:    Re: [patch] Staging: tidspbridge: make mmap root-only so it is not a security problem
From:       Pavel Machek <pavel () ucw ! cz>
Date:       2013-11-30 22:58:23
Message-ID: 20131130225822.GA26031 () amd ! pavel ! ucw ! cz
[Download RAW message or body]

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;

is exactly right solution, it just should be in _open...]

									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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
[prev in list] [next in list] [prev in thread] [next in thread]