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 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.


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