lkml.org 
[lkml]   [2011]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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


    \
     
     \ /
      Last update: 2011-11-29 21:51    [W:3.317 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site