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]

Date:	Tue, 29 Nov 2011 14:49:11 -0600
From:	Serge Hallyn <serge.hallyn@...onical.com>
To:	Cyrill Gorcunov <gorcunov@...il.com>
CC:	Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tejun Heo <tj@...nel.org>, Andrew Vagin <avagin@...nvz.org>,
	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 11/29/2011 02:29 PM, Cyrill Gorcunov wrote:
> 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.

My memory is a bit hazy here, but cryo 
(http://git.sr71.net/?p=cryo-forhallyn.git;a=summary) did also do this 
from userspace.  As I recall the one problem we had was ... that we 
couldn't lower the mm_start of the first segment?  I think.  But I bring 
it up only because the advantage of doing it this way was that all of 
the ptrace protections automatically applied.

-serge
--
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/