| 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
| ||
|
Message-Id: <20111207144355.c889e22d.akpm@linux-foundation.org>
Date: Wed, 7 Dec 2011 14:43:55 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Cyrill Gorcunov <gorcunov@...il.com>
Cc: Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.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>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
Michael Kerrisk <mtk.manpages@...il.com>
Subject: Re: [rfc 3/3] prctl: Add PR_SET_MM codes to tune up mm_struct
entires
On Wed, 7 Dec 2011 16:27:18 +0400
Cyrill Gorcunov <gorcunov@...il.com> wrote:
> At process of task restoration we need a way to tune up
> a few members of mm_struct structure such as start_code,
> end_code, start_data, end_data, start_stack, start_brk, brk.
I don't really know what "tune up" means in this context. Can we
please be more specific and detailed here? It appears that the patch
permits userspace to directly modify these fields.
> While most of them have a statistical nature (their values
> are involved into calculation of /proc/<pid>/statm output)
> the start_brk and brk values are used to compute an allowed
> size of program data segment expansion. Which means an arbitrary
> changes of this value might be a bit dangerous operation.
>
> To restrict access to this facility the following requirements
> applied to prctl users:
>
> - The process has to have CAP_SYS_ADMIN capability granted.
> - For all opcodes except start_brk/brk members an appropriate
> VMA area must be existing and should fit certain VMA flags,
> such as:
> - code segment must be executable but not writable;
> - data segment must not be executable.
>
> start_brk/brk values must not intersect with data segment
> and must not exceed RLIMIT_DATA resource limit.
>
> Still the main guard is CAP_SYS_ADMIN capability check.
>
> ...
>
> include/linux/prctl.h | 12 +++++
> kernel/sys.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++
The prctl(2) manpage will need to be updated. Please Cc Michael on all
such changes.
>
> Index: linux-2.6.git/include/linux/prctl.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/prctl.h
> +++ linux-2.6.git/include/linux/prctl.h
> @@ -102,4 +102,16 @@
>
> #define PR_MCE_KILL_GET 34
>
> +/*
> + * Tune up process memory map specifics.
> + */
> +#define PR_SET_MM 35
> +# define PR_SET_MM_START_CODE 1
> +# define PR_SET_MM_END_CODE 2
> +# define PR_SET_MM_START_DATA 3
> +# define PR_SET_MM_END_DATA 4
> +# define PR_SET_MM_START_STACK 5
> +# define PR_SET_MM_START_BRK 6
> +# define PR_SET_MM_BRK 7
> +
> #endif /* _LINUX_PRCTL_H */
> Index: linux-2.6.git/kernel/sys.c
> ===================================================================
> --- linux-2.6.git.orig/kernel/sys.c
> +++ linux-2.6.git/kernel/sys.c
> @@ -1692,6 +1692,118 @@ SYSCALL_DEFINE1(umask, int, mask)
> return mask;
> }
>
> +static int prctl_set_mm(int opt, unsigned long addr)
> +{
> + unsigned long rlim = rlimit(RLIMIT_DATA);
> + unsigned long vm_req_flags;
> + unsigned long vm_bad_flags;
> + struct vm_area_struct *vma;
> + struct mm_struct *mm;
> + int error = 0;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + if (addr >= TASK_SIZE)
> + return -EINVAL;
> +
> + mm = get_task_mm(current);
Is it necessaary to run the expensive get_task_mm() for `current'?
`current' is known to be running and you have control of it here -
nobody will be taking our mm away. Simply use current->mm? The
function actually uses current->mm later on in several places.
> + if (!mm)
> + return -ENOENT;
> +
> + down_read(&mm->mmap_sem);
> + vma = find_vma(mm, addr);
> +
> + if (opt != PR_SET_MM_START_BRK &&
> + opt != PR_SET_MM_BRK) {
80 columns, not 40 :)
> + /* It must be existing VMA */
> + if (!vma || vma->vm_start > addr)
> + goto out;
> + }
> +
> + error = -EINVAL;
> + switch (opt) {
> + case PR_SET_MM_START_CODE:
> + case PR_SET_MM_END_CODE:
> +
You're adding unneeded and unconventional newlines after the `case'
statements.
> + vm_req_flags = VM_READ | VM_EXEC;
> + vm_bad_flags = VM_WRITE | VM_MAYSHARE;
> +
> + if ((vma->vm_flags & vm_req_flags) != vm_req_flags ||
> + (vma->vm_flags & vm_bad_flags))
> + goto out;
> +
> + if (opt == PR_SET_MM_START_CODE)
> + current->mm->start_code = addr;
> + else
> + current->mm->end_code = addr;
> + break;
> +
> + case PR_SET_MM_START_DATA:
> + case PR_SET_MM_END_DATA:
> +
> + vm_req_flags = VM_READ | VM_WRITE;
> + vm_bad_flags = VM_EXEC | VM_MAYSHARE;
> +
> + if ((vma->vm_flags & vm_req_flags) != vm_req_flags ||
> + (vma->vm_flags & vm_bad_flags))
> + goto out;
> +
> + if (opt == PR_SET_MM_START_DATA)
> + current->mm->start_data = addr;
> + else
> + current->mm->end_data = addr;
> + break;
> +
> + case PR_SET_MM_START_STACK:
> +
> +#ifdef CONFIG_STACK_GROWSUP
> + vm_req_flags = VM_READ | VM_WRITE | VM_GROWSUP;
> +#else
> + vm_req_flags = VM_READ | VM_WRITE | VM_GROWSDOWN;
> +#endif
> + if ((vma->vm_flags & vm_req_flags) != vm_req_flags)
> + goto out;
> +
> + current->mm->start_stack = addr;
> + break;
> +
> + case PR_SET_MM_START_BRK:
> + if (addr <= mm->end_data)
> + goto out;
> +
> + if (rlim < RLIM_INFINITY &&
> + (mm->brk - addr) + (mm->end_data - mm->start_data) > rlim)
> + goto out;
> +
> + current->mm->start_brk = addr;
> + break;
> +
> + case PR_SET_MM_BRK:
> + if (addr <= mm->end_data)
> + goto out;
> +
> + if (rlim < RLIM_INFINITY &&
> + (addr - mm->start_brk) + (mm->end_data - mm->start_data) > rlim)
> + goto out;
> +
> + current->mm->brk = addr;
> + break;
> +
> + default:
> + error = -EINVAL;
> + goto out;
> + }
> +
> + error = 0;
> +
> +out:
> + up_read(&mm->mmap_sem);
> + mmput(mm);
> +
> + return error;
> +}
This is starting to add a non-trivial amount of code. Perhaps we need
to introduce a Kconfig variable to control such things as this, to
prevent bloating up kernels which aren't require to support c/r?
>
> ...
>
--
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/
Powered by blists - more mailing lists