Messages in this thread | | | Date | Tue, 29 Nov 2011 14:49:11 -0600 | From | Serge Hallyn <> | 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
| |