linux-kernel - Re: [rfc 3/3] prctl: Add PR_SET_MM codes to tune up mm

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]

Message-ID: <20111129202951.GG1775@moon>
Date:	Wed, 30 Nov 2011 00:29:51 +0400
From:	Cyrill Gorcunov <gorcunov@...il.com>
To:	Kees Cook <keescook@...omium.org>
Cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tejun Heo <tj@...nel.org>, Andrew Vagin <avagin@...nvz.org>,
	Serge Hallyn <serge.hallyn@...onical.com>,
	Pavel Emelyanov <xemul@...allels.com>,
	Vasiliy Kulikov <segoon@...nwall.com>
Subject: Re: [rfc 3/3] prctl: Add PR_SET_MM codes to tune up mm_struct entires

On Tue, Nov 29, 2011 at 12:19:38PM -0800, Kees Cook wrote:
> On Tue, Nov 29, 2011 at 11:12:55PM +0400, Cyrill Gorcunov wrote:
> > At restore time we need a mechanism to restore those values
> > back and for this sake PR_SET_MM prctl code is introduced.
> > 
> > Note at moment this inteface is allowed for CAP_SYS_ADMIN
> > only.
> 
> NAK from me; this needs more bounds checking. Though, yes, it absolutely
> must be a privileged action since this is potentially very dangerous. Can
> we invent something stronger than CAP_SYS_ADMIN? ;)

Heh.

> 
> > @@ -1841,6 +1841,58 @@ SYSCALL_DEFINE5(prctl, int, option, unsi
> >  			else
> >  				error = PR_MCE_KILL_DEFAULT;
> >  			break;
> > +		case PR_SET_MM: {
> > +			struct mm_struct *mm;
> > +			struct vm_area_struct *vma;
> > +
> > +			if (arg4 | arg5)
> > +				return -EINVAL;
> > +
> > +			if (!capable(CAP_SYS_ADMIN))
> > +				return -EPERM;
> > +
> > +			error = -ENOENT;
> > +			mm = get_task_mm(current);
> > +			if (!mm)
> > +				return error;
> > +
> > +			down_read(&mm->mmap_sem);
> > +			vma = find_vma(mm, arg3);
> > +			if (!vma)
> > +				goto out;
> 
> arg3 needs to be significantly more carefully validated. find_vma() doesn't
> say that vm_start <= addr, only that vm_end > addr. This effectively
> bypasses all the vma checks (mmap_min_addr, max process size, etc), with
> some pretty crazy side-effects, I think.
> 

Yes, I know it needs some more testing, but apart from vma bounds (yup,
good point with find_vma, I'll fix) I thought about what else should be
checked? I think VMA prototype should be checked to fit "code", "data"
templates, ie code should be at least readable and execytable, but what
about data and stack and brk, should stack be executable? That is the
point where I've got a bit confused and though putting RFC out might be
a good idea to collect opinions.
--
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/