* [rfc 0/4] prctl: set-mm -- Rework interface, v2
@ 2014-07-24 16:46 ` Cyrill Gorcunov
0 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 16:46 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: gorcunov, keescook, tj, akpm, avagin, ebiederm, hpa,
serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages, jln
Hi, here is a second version. I've tried to address all comments
(for which I'm really grateful). I believe the main question which
remains opened is exe-fd setup. Hopefully the current limitation
(root only) would be enough.
Also, Julien, there is no additional need for ordering test of
@start_stack member since we use find_vma helper.
Please take a look, once you have a spare minutes, thanks!
Cyrill
^ permalink raw reply [flat|nested] 22+ messages in thread
* [rfc 0/4] prctl: set-mm -- Rework interface, v2
@ 2014-07-24 16:46 ` Cyrill Gorcunov
0 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 16:46 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: gorcunov, keescook, tj, akpm, avagin, ebiederm, hpa,
serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages, jln
Hi, here is a second version. I've tried to address all comments
(for which I'm really grateful). I believe the main question which
remains opened is exe-fd setup. Hopefully the current limitation
(root only) would be enough.
Also, Julien, there is no additional need for ordering test of
@start_stack member since we use find_vma helper.
Please take a look, once you have a spare minutes, thanks!
Cyrill
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [rfc 1/4] mm: Introduce may_adjust_brk helper
2014-07-24 16:46 ` Cyrill Gorcunov
@ 2014-07-24 16:46 ` Cyrill Gorcunov
-1 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 16:46 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: gorcunov, keescook, tj, akpm, avagin, ebiederm, hpa,
serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages, jln
[-- Attachment #1: prctl-add-may_adjust_brk --]
[-- Type: text/plain, Size: 1666 bytes --]
To eliminate code duplication lets introduce may_adjust_brk
helper which we will use in brk() and prctl() syscalls.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Vagin <avagin@openvz.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Julien Tinnes <jln@google.com>
---
include/linux/mm.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
Index: linux-2.6.git/include/linux/mm.h
===================================================================
--- linux-2.6.git.orig/include/linux/mm.h
+++ linux-2.6.git/include/linux/mm.h
@@ -18,6 +18,7 @@
#include <linux/pfn.h>
#include <linux/bit_spinlock.h>
#include <linux/shrinker.h>
+#include <linux/resource.h>
struct mempolicy;
struct anon_vma;
@@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
bool *need_rmap_locks);
extern void exit_mmap(struct mm_struct *);
+static inline int may_adjust_brk(unsigned long rlim,
+ unsigned long new_brk,
+ unsigned long start_brk,
+ unsigned long end_data,
+ unsigned long start_data)
+{
+ if (rlim < RLIMIT_DATA) {
+ if (((new_brk - start_brk) + (end_data - start_data)) > rlim)
+ return -ENOSPC;
+ }
+ return 0;
+}
+
extern int mm_take_all_locks(struct mm_struct *mm);
extern void mm_drop_all_locks(struct mm_struct *mm);
^ permalink raw reply [flat|nested] 22+ messages in thread
* [rfc 1/4] mm: Introduce may_adjust_brk helper
@ 2014-07-24 16:46 ` Cyrill Gorcunov
0 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 16:46 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: gorcunov, keescook, tj, akpm, avagin, ebiederm, hpa,
serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages, jln
[-- Attachment #1: prctl-add-may_adjust_brk --]
[-- Type: text/plain, Size: 1891 bytes --]
To eliminate code duplication lets introduce may_adjust_brk
helper which we will use in brk() and prctl() syscalls.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Vagin <avagin@openvz.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Julien Tinnes <jln@google.com>
---
include/linux/mm.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
Index: linux-2.6.git/include/linux/mm.h
===================================================================
--- linux-2.6.git.orig/include/linux/mm.h
+++ linux-2.6.git/include/linux/mm.h
@@ -18,6 +18,7 @@
#include <linux/pfn.h>
#include <linux/bit_spinlock.h>
#include <linux/shrinker.h>
+#include <linux/resource.h>
struct mempolicy;
struct anon_vma;
@@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
bool *need_rmap_locks);
extern void exit_mmap(struct mm_struct *);
+static inline int may_adjust_brk(unsigned long rlim,
+ unsigned long new_brk,
+ unsigned long start_brk,
+ unsigned long end_data,
+ unsigned long start_data)
+{
+ if (rlim < RLIMIT_DATA) {
+ if (((new_brk - start_brk) + (end_data - start_data)) > rlim)
+ return -ENOSPC;
+ }
+ return 0;
+}
+
extern int mm_take_all_locks(struct mm_struct *mm);
extern void mm_drop_all_locks(struct mm_struct *mm);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [rfc 2/4] mm: Use may_adjust_brk helper
2014-07-24 16:46 ` Cyrill Gorcunov
@ 2014-07-24 16:46 ` Cyrill Gorcunov
-1 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 16:46 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: gorcunov, keescook, tj, akpm, avagin, ebiederm, hpa,
serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages, jln
[-- Attachment #1: prctl-use-may_adjust_brk --]
[-- Type: text/plain, Size: 2371 bytes --]
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Vagin <avagin@openvz.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Julien Tinnes <jln@google.com>
---
kernel/sys.c | 10 ++++------
mm/mmap.c | 7 +++----
2 files changed, 7 insertions(+), 10 deletions(-)
Index: linux-2.6.git/kernel/sys.c
===================================================================
--- linux-2.6.git.orig/kernel/sys.c
+++ linux-2.6.git/kernel/sys.c
@@ -1733,9 +1733,8 @@ static int prctl_set_mm(int opt, unsigne
if (addr <= mm->end_data)
goto out;
- if (rlim < RLIM_INFINITY &&
- (mm->brk - addr) +
- (mm->end_data - mm->start_data) > rlim)
+ if (may_adjust_brk(rlim, mm->brk, addr,
+ mm->end_data, mm->start_data))
goto out;
mm->start_brk = addr;
@@ -1745,9 +1744,8 @@ static int prctl_set_mm(int opt, unsigne
if (addr <= mm->end_data)
goto out;
- if (rlim < RLIM_INFINITY &&
- (addr - mm->start_brk) +
- (mm->end_data - mm->start_data) > rlim)
+ if (may_adjust_brk(rlim, addr, mm->start_brk,
+ mm->end_data, mm->start_data))
goto out;
mm->brk = addr;
Index: linux-2.6.git/mm/mmap.c
===================================================================
--- linux-2.6.git.orig/mm/mmap.c
+++ linux-2.6.git/mm/mmap.c
@@ -263,7 +263,7 @@ static unsigned long do_brk(unsigned lon
SYSCALL_DEFINE1(brk, unsigned long, brk)
{
- unsigned long rlim, retval;
+ unsigned long retval;
unsigned long newbrk, oldbrk;
struct mm_struct *mm = current->mm;
unsigned long min_brk;
@@ -293,9 +293,8 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
* segment grow beyond its set limit the in case where the limit is
* not page aligned -Ram Gupta
*/
- rlim = rlimit(RLIMIT_DATA);
- if (rlim < RLIM_INFINITY && (brk - mm->start_brk) +
- (mm->end_data - mm->start_data) > rlim)
+ if (may_adjust_brk(rlimit(RLIMIT_DATA), brk, mm->start_brk,
+ mm->end_data, mm->start_data))
goto out;
newbrk = PAGE_ALIGN(brk);
^ permalink raw reply [flat|nested] 22+ messages in thread
* [rfc 2/4] mm: Use may_adjust_brk helper
@ 2014-07-24 16:46 ` Cyrill Gorcunov
0 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 16:46 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: gorcunov, keescook, tj, akpm, avagin, ebiederm, hpa,
serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages, jln
[-- Attachment #1: prctl-use-may_adjust_brk --]
[-- Type: text/plain, Size: 2596 bytes --]
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Vagin <avagin@openvz.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Julien Tinnes <jln@google.com>
---
kernel/sys.c | 10 ++++------
mm/mmap.c | 7 +++----
2 files changed, 7 insertions(+), 10 deletions(-)
Index: linux-2.6.git/kernel/sys.c
===================================================================
--- linux-2.6.git.orig/kernel/sys.c
+++ linux-2.6.git/kernel/sys.c
@@ -1733,9 +1733,8 @@ static int prctl_set_mm(int opt, unsigne
if (addr <= mm->end_data)
goto out;
- if (rlim < RLIM_INFINITY &&
- (mm->brk - addr) +
- (mm->end_data - mm->start_data) > rlim)
+ if (may_adjust_brk(rlim, mm->brk, addr,
+ mm->end_data, mm->start_data))
goto out;
mm->start_brk = addr;
@@ -1745,9 +1744,8 @@ static int prctl_set_mm(int opt, unsigne
if (addr <= mm->end_data)
goto out;
- if (rlim < RLIM_INFINITY &&
- (addr - mm->start_brk) +
- (mm->end_data - mm->start_data) > rlim)
+ if (may_adjust_brk(rlim, addr, mm->start_brk,
+ mm->end_data, mm->start_data))
goto out;
mm->brk = addr;
Index: linux-2.6.git/mm/mmap.c
===================================================================
--- linux-2.6.git.orig/mm/mmap.c
+++ linux-2.6.git/mm/mmap.c
@@ -263,7 +263,7 @@ static unsigned long do_brk(unsigned lon
SYSCALL_DEFINE1(brk, unsigned long, brk)
{
- unsigned long rlim, retval;
+ unsigned long retval;
unsigned long newbrk, oldbrk;
struct mm_struct *mm = current->mm;
unsigned long min_brk;
@@ -293,9 +293,8 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
* segment grow beyond its set limit the in case where the limit is
* not page aligned -Ram Gupta
*/
- rlim = rlimit(RLIMIT_DATA);
- if (rlim < RLIM_INFINITY && (brk - mm->start_brk) +
- (mm->end_data - mm->start_data) > rlim)
+ if (may_adjust_brk(rlimit(RLIMIT_DATA), brk, mm->start_brk,
+ mm->end_data, mm->start_data))
goto out;
newbrk = PAGE_ALIGN(brk);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [rfc 3/4] prctl: PR_SET_MM -- Factor out mmap_sem when update mm::exe_file
2014-07-24 16:46 ` Cyrill Gorcunov
@ 2014-07-24 16:47 ` Cyrill Gorcunov
-1 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 16:47 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: gorcunov, keescook, tj, akpm, avagin, ebiederm, hpa,
serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages, jln
[-- Attachment #1: prctl-rework-prctl_set_mm_exe_file-locked --]
[-- Type: text/plain, Size: 2599 bytes --]
Instead of taking mm->mmap_sem inside prctl_set_mm_exe_file move
it out of and rename the helper to prctl_set_mm_exe_file_locked.
This will allow to reuse this function in a next patch.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Vagin <avagin@openvz.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Julien Tinnes <jln@google.com>
---
kernel/sys.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
Index: linux-2.6.git/kernel/sys.c
===================================================================
--- linux-2.6.git.orig/kernel/sys.c
+++ linux-2.6.git/kernel/sys.c
@@ -1628,12 +1628,14 @@ SYSCALL_DEFINE1(umask, int, mask)
return mask;
}
-static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
+static int prctl_set_mm_exe_file_locked(struct mm_struct *mm, unsigned int fd)
{
struct fd exe;
struct inode *inode;
int err;
+ VM_BUG_ON(!rwsem_is_locked(&mm->mmap_sem));
+
exe = fdget(fd);
if (!exe.file)
return -EBADF;
@@ -1654,8 +1656,6 @@ static int prctl_set_mm_exe_file(struct
if (err)
goto exit;
- down_write(&mm->mmap_sem);
-
/*
* Forbid mm->exe_file change if old file still mapped.
*/
@@ -1667,7 +1667,7 @@ static int prctl_set_mm_exe_file(struct
if (vma->vm_file &&
path_equal(&vma->vm_file->f_path,
&mm->exe_file->f_path))
- goto exit_unlock;
+ goto exit;
}
/*
@@ -1678,13 +1678,10 @@ static int prctl_set_mm_exe_file(struct
*/
err = -EPERM;
if (test_and_set_bit(MMF_EXE_FILE_CHANGED, &mm->flags))
- goto exit_unlock;
+ goto exit;
err = 0;
set_mm_exe_file(mm, exe.file); /* this grabs a reference to exe.file */
-exit_unlock:
- up_write(&mm->mmap_sem);
-
exit:
fdput(exe);
return err;
@@ -1704,8 +1701,12 @@ static int prctl_set_mm(int opt, unsigne
if (!capable(CAP_SYS_RESOURCE))
return -EPERM;
- if (opt == PR_SET_MM_EXE_FILE)
- return prctl_set_mm_exe_file(mm, (unsigned int)addr);
+ if (opt == PR_SET_MM_EXE_FILE) {
+ down_write(&mm->mmap_sem);
+ error = prctl_set_mm_exe_file_locked(mm, (unsigned int)addr);
+ up_write(&mm->mmap_sem);
+ return error;
+ }
if (addr >= TASK_SIZE || addr < mmap_min_addr)
return -EINVAL;
^ permalink raw reply [flat|nested] 22+ messages in thread
* [rfc 3/4] prctl: PR_SET_MM -- Factor out mmap_sem when update mm::exe_file
@ 2014-07-24 16:47 ` Cyrill Gorcunov
0 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 16:47 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: gorcunov, keescook, tj, akpm, avagin, ebiederm, hpa,
serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages, jln
[-- Attachment #1: prctl-rework-prctl_set_mm_exe_file-locked --]
[-- Type: text/plain, Size: 2824 bytes --]
Instead of taking mm->mmap_sem inside prctl_set_mm_exe_file move
it out of and rename the helper to prctl_set_mm_exe_file_locked.
This will allow to reuse this function in a next patch.
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Vagin <avagin@openvz.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Julien Tinnes <jln@google.com>
---
kernel/sys.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
Index: linux-2.6.git/kernel/sys.c
===================================================================
--- linux-2.6.git.orig/kernel/sys.c
+++ linux-2.6.git/kernel/sys.c
@@ -1628,12 +1628,14 @@ SYSCALL_DEFINE1(umask, int, mask)
return mask;
}
-static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
+static int prctl_set_mm_exe_file_locked(struct mm_struct *mm, unsigned int fd)
{
struct fd exe;
struct inode *inode;
int err;
+ VM_BUG_ON(!rwsem_is_locked(&mm->mmap_sem));
+
exe = fdget(fd);
if (!exe.file)
return -EBADF;
@@ -1654,8 +1656,6 @@ static int prctl_set_mm_exe_file(struct
if (err)
goto exit;
- down_write(&mm->mmap_sem);
-
/*
* Forbid mm->exe_file change if old file still mapped.
*/
@@ -1667,7 +1667,7 @@ static int prctl_set_mm_exe_file(struct
if (vma->vm_file &&
path_equal(&vma->vm_file->f_path,
&mm->exe_file->f_path))
- goto exit_unlock;
+ goto exit;
}
/*
@@ -1678,13 +1678,10 @@ static int prctl_set_mm_exe_file(struct
*/
err = -EPERM;
if (test_and_set_bit(MMF_EXE_FILE_CHANGED, &mm->flags))
- goto exit_unlock;
+ goto exit;
err = 0;
set_mm_exe_file(mm, exe.file); /* this grabs a reference to exe.file */
-exit_unlock:
- up_write(&mm->mmap_sem);
-
exit:
fdput(exe);
return err;
@@ -1704,8 +1701,12 @@ static int prctl_set_mm(int opt, unsigne
if (!capable(CAP_SYS_RESOURCE))
return -EPERM;
- if (opt == PR_SET_MM_EXE_FILE)
- return prctl_set_mm_exe_file(mm, (unsigned int)addr);
+ if (opt == PR_SET_MM_EXE_FILE) {
+ down_write(&mm->mmap_sem);
+ error = prctl_set_mm_exe_file_locked(mm, (unsigned int)addr);
+ up_write(&mm->mmap_sem);
+ return error;
+ }
if (addr >= TASK_SIZE || addr < mmap_min_addr)
return -EINVAL;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [rfc 4/4] prctl: PR_SET_MM -- Introduce PR_SET_MM_MAP operation, v3
2014-07-24 16:46 ` Cyrill Gorcunov
@ 2014-07-24 16:47 ` Cyrill Gorcunov
-1 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 16:47 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: gorcunov, keescook, tj, akpm, avagin, ebiederm, hpa,
serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages, jln
[-- Attachment #1: prctl-rework-new-mm-map-6 --]
[-- Type: text/plain, Size: 14445 bytes --]
During development of c/r we've noticed that in case if we need to
support user namespaces we face a problem with capabilities in
prctl(PR_SET_MM, ...) call, in particular once new user namespace
is created capable(CAP_SYS_RESOURCE) no longer passes.
A approach is to eliminate CAP_SYS_RESOURCE check but pass all
new values in one bundle, which would allow the kernel to make
more intensive test for sanity of values and same time allow us to
support checkpoint/restore of user namespaces.
Thus a new command PR_SET_MM_MAP introduced. It takes a pointer of
prctl_mm_map structure which carries all the members to be updated.
prctl(PR_SET_MM, PR_SET_MM_MAP, struct prctl_mm_map *, size)
struct prctl_mm_map {
__u64 start_code;
__u64 end_code;
__u64 start_data;
__u64 end_data;
__u64 start_brk;
__u64 brk;
__u64 start_stack;
__u64 arg_start;
__u64 arg_end;
__u64 env_start;
__u64 env_end;
__u64 *auxv;
__u32 auxv_size;
__u32 exe_fd;
};
All members except @exe_fd correspond ones of struct mm_struct.
To figure out which available values these members may take here
are meanings of the members.
- start_code, end_code: represent bounds of executable code area
- start_data, end_data: represent bounds of data area
- start_brk, brk: used to calculate bounds for brk() syscall
- start_stack: used when accounting space needed for command
line arguments, environment and shmat() syscall
- arg_start, arg_end, env_start, env_end: represent memory area
supplied for command line arguments and environment variables
- auxv, auxv_size: carries auxiliary vector, Elf format specifics
- exe_fd: file descriptor number for executable link (/proc/self/exe)
Thus we apply the following requirements to the values
1) Any member except @auxv, @auxv_size, @exe_fd is rather an address
in user space thus it must be laying inside [mmap_min_addr, mmap_max_addr)
interval.
2) While @[start|end]_code and @[start|end]_data may point to an nonexisting
VMAs (say a program maps own new .text and .data segments during execution)
the rest of members should belong to VMA which must exist.
3) Addresses must be ordered, ie @start_ member must not be greater or
equal to appropriate @end_ member.
4) As in regular Elf loading procedure we require that @start_brk and
@brk be greater than @end_data.
5) If RLIMIT_DATA rlimit is set to non-infinity new values should not
exceed existing limit. Same applies to RLIMIT_STACK.
6) Auxiliary vector size must not exceed existing one (which is
predefined as AT_VECTOR_SIZE and depends on architecture).
7) File descriptor passed in @exe_file should be pointing
to executable file (because we use existing prctl_set_mm_exe_file_locked
helper it ensures that the file we are going to use as exe link has all
required permission granted).
Now about where these members are involved inside kernel code:
- @start_code and @end_code are used in /proc/$pid/[stat|statm] output;
- @start_data and @end_data are used in /proc/$pid/[stat|statm] output,
also they are considered if there enough space for brk() syscall
result if RLIMIT_DATA is set;
- @start_brk shown in /proc/$pid/stat output and accounted in brk()
syscall if RLIMIT_DATA is set; also this member is tested to
find a symbolic name of mmap event for perf system (we choose
if event is generated for "heap" area); one more aplication is
selinux -- we test if a process has PROCESS__EXECHEAP permission
if trying to make heap area being executable with mprotect() syscall;
- @brk is a current value for brk() syscall which lays inside heap
area, it's shown in /proc/$pid/stat. When syscall brk() succesfully
provides new memory area to a user space upon brk() completion the
mm::brk is updated to carry new value;
Both @start_brk and @brk are actively used in /proc/$pid/maps
and /proc/$pid/smaps output to find a symbolic name "heap" for
VMA being scanned;
- @start_stack is printed out in /proc/$pid/stat and used to
find a symbolic name "stack" for task and threads in
/proc/$pid/maps and /proc/$pid/smaps output, and as the same
as with @start_brk -- perf system uses it for event naming.
Also kernel treat this member as a start address of where
to map vDSO pages and to check if there is enough space
for shmat() syscall;
- @arg_start, @arg_end, @env_start and @env_end are printed out
in /proc/$pid/stat. Another access to the data these members
represent is to read /proc/$pid/environ or /proc/$pid/cmdline.
Any attempt to read these areas kernel tests with access_process_vm
helper so a user must have enough rights for this action;
- @auxv and @auxv_size may be read from /proc/$pid/auxv. Strictly
speaking kernel doesn't care much about which exactly data is
sitting there because it is solely for userspace;
- @exe_fd is referred from /proc/$pid/exe and when generating
coredump. We uses prctl_set_mm_exe_file_locked helper to update
this member, so exe-file link modification remains one-shot
action.
Still note that updating exe-file link now doesn't require sys-resource
capability anymore, after all there is no much profit in preventing setup
own file link (there are a number of ways to execute own code -- ptrace,
ld-preload, so that the only reliable way to find which exactly code
is executed is to inspect running program memory). Still we require
the caller to be at least user-namespace root user.
I believe the old interface should be deprecated and ripped off
in a couple of kernel releases if no one against.
To test if new interface is implemented in the kernel one
can pass PR_SET_MM_MAP_SIZE opcode and the kernel returns
the size of currently supported struct prctl_mm_map.
v2:
- compact macros (by keescook@)
- wrap new code with CONFIG_ (by akpm@)
v3 (by jln@):
- use __prctl_check_order for brk and start_brk
- use may_adjust_brk helper
- make sure that only root can update @exe_fd link
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Vagin <avagin@openvz.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Julien Tinnes <jln@google.com>
---
include/uapi/linux/prctl.h | 25 +++++
kernel/sys.c | 211 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 235 insertions(+), 1 deletion(-)
Index: linux-2.6.git/include/uapi/linux/prctl.h
===================================================================
--- linux-2.6.git.orig/include/uapi/linux/prctl.h
+++ linux-2.6.git/include/uapi/linux/prctl.h
@@ -119,6 +119,31 @@
# define PR_SET_MM_ENV_END 11
# define PR_SET_MM_AUXV 12
# define PR_SET_MM_EXE_FILE 13
+# define PR_SET_MM_MAP 14
+# define PR_SET_MM_MAP_SIZE 15
+
+/*
+ * This structure provides new memory descriptor
+ * map which mostly modifies /proc/pid/stat[m]
+ * output for a task. This mostly done in a
+ * sake of checkpoint/restore functionality.
+ */
+struct prctl_mm_map {
+ __u64 start_code; /* code section bounds */
+ __u64 end_code;
+ __u64 start_data; /* data section bounds */
+ __u64 end_data;
+ __u64 start_brk; /* heap for brk() syscall */
+ __u64 brk;
+ __u64 start_stack; /* stack starts at */
+ __u64 arg_start; /* command line arguments bounds */
+ __u64 arg_end;
+ __u64 env_start; /* environment variables bounds */
+ __u64 env_end;
+ __u64 *auxv; /* auxiliary vector */
+ __u32 auxv_size; /* vector size */
+ __u32 exe_fd; /* /proc/$pid/exe link file */
+};
/*
* Set specific pid that is allowed to ptrace the current task.
Index: linux-2.6.git/kernel/sys.c
===================================================================
--- linux-2.6.git.orig/kernel/sys.c
+++ linux-2.6.git/kernel/sys.c
@@ -1687,6 +1687,208 @@ exit:
return err;
}
+#ifdef CONFIG_CHECKPOINT_RESTORE
+/*
+ * WARNING: we don't require any capability here so be very careful
+ * in what is allowed for modification from userspace.
+ */
+static int validate_prctl_map_locked(struct prctl_mm_map *prctl_map)
+{
+ unsigned long mmap_max_addr = TASK_SIZE;
+ struct mm_struct *mm = current->mm;
+ struct vm_area_struct *stack_vma;
+ int error = 0;
+
+ /*
+ * Make sure the members are not somewhere outside
+ * of allowed address space.
+ */
+#define __prctl_check_addr_space(__member) \
+ ({ \
+ int __rc; \
+ if ((unsigned long)prctl_map->__member < mmap_max_addr && \
+ (unsigned long)prctl_map->__member >= mmap_min_addr) \
+ __rc = 0; \
+ else \
+ __rc = -EINVAL; \
+ __rc; \
+ })
+ error |= __prctl_check_addr_space(start_code);
+ error |= __prctl_check_addr_space(end_code);
+ error |= __prctl_check_addr_space(start_data);
+ error |= __prctl_check_addr_space(end_data);
+ error |= __prctl_check_addr_space(start_stack);
+ error |= __prctl_check_addr_space(start_brk);
+ error |= __prctl_check_addr_space(brk);
+ error |= __prctl_check_addr_space(arg_start);
+ error |= __prctl_check_addr_space(arg_end);
+ error |= __prctl_check_addr_space(env_start);
+ error |= __prctl_check_addr_space(env_end);
+ if (error)
+ goto out;
+#undef __prctl_check_addr_space
+
+ /*
+ * Stack, brk, command line arguments and environment must exist.
+ */
+ stack_vma = find_vma(mm, (unsigned long)prctl_map->start_stack);
+ if (!stack_vma) {
+ error = -EINVAL;
+ goto out;
+ }
+#define __prctl_check_vma(__member) \
+ find_vma(mm, (unsigned long)prctl_map->__member) ? 0 : -EINVAL
+ error |= __prctl_check_vma(start_brk);
+ error |= __prctl_check_vma(brk);
+ error |= __prctl_check_vma(arg_start);
+ error |= __prctl_check_vma(arg_end);
+ error |= __prctl_check_vma(env_start);
+ error |= __prctl_check_vma(env_end);
+ if (error)
+ goto out;
+#undef __prctl_check_vma
+
+ /*
+ * Make sure the pairs are ordered.
+ */
+#define __prctl_check_order(__m1, __op, __m2) \
+ ((unsigned long)prctl_map->__m1 __op \
+ (unsigned long)prctl_map->__m2) ? 0 : -EINVAL
+ error |= __prctl_check_order(start_code, <, end_code);
+ error |= __prctl_check_order(start_data, <, end_data);
+ error |= __prctl_check_order(start_brk, <=, brk);
+ error |= __prctl_check_order(arg_start, <, arg_end);
+ error |= __prctl_check_order(env_start, <, env_end);
+ if (error)
+ goto out;
+#undef __prctl_check_order
+
+ error = -EINVAL;
+
+ /*
+ * @brk should be after @end_data in traditional maps.
+ */
+ if (prctl_map->start_brk <= prctl_map->end_data ||
+ prctl_map->brk <= prctl_map->end_data)
+ goto out;
+
+ /*
+ * Neither we should allow to override limits if they set.
+ */
+ if (may_adjust_brk(rlimit(RLIMIT_DATA), prctl_map->brk,
+ prctl_map->start_brk, prctl_map->end_data,
+ prctl_map->start_data))
+ goto out;
+
+#ifdef CONFIG_STACK_GROWSUP
+ if (may_adjust_brk(rlimit(RLIMIT_STACK),
+ stack_vma->vm_end,
+ prctl_map->start_stack, 0, 0))
+#else
+ if (may_adjust_brk(rlimit(RLIMIT_STACK),
+ prctl_map->start_stack,
+ stack_vma->vm_start, 0, 0))
+#endif
+ goto out;
+
+ /*
+ * Someone is trying to cheat the auxv vector.
+ */
+ if (prctl_map->auxv_size) {
+ if (!prctl_map->auxv ||
+ prctl_map->auxv_size > sizeof(mm->saved_auxv))
+ goto out;
+ }
+
+ /*
+ * Finally, make sure the caller has the rights to
+ * change /proc/pid/exe link: only local root should
+ * be allowed to.
+ */
+ if (prctl_map->exe_fd != (u32)-1) {
+ struct user_namespace *ns = current_user_ns();
+ const struct cred *cred = current_cred();
+
+ if (!uid_eq(cred->uid, make_kuid(ns, 0)) ||
+ !gid_eq(cred->gid, make_kgid(ns, 0)))
+ goto out;
+ }
+
+ error = 0;
+out:
+ return error;
+}
+
+static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
+{
+ struct prctl_mm_map prctl_map = { .exe_fd = (u32)-1, };
+ unsigned long user_auxv[AT_VECTOR_SIZE];
+ struct mm_struct *mm = current->mm;
+ int error = -EINVAL;
+
+ BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
+
+ if (opt == PR_SET_MM_MAP_SIZE)
+ return put_user((unsigned int)sizeof(prctl_map),
+ (unsigned int __user *)addr);
+
+ if (data_size != sizeof(prctl_map))
+ return -EINVAL;
+
+ if (copy_from_user(&prctl_map, addr, sizeof(prctl_map)))
+ return -EFAULT;
+
+ down_read(&mm->mmap_sem);
+
+ if (validate_prctl_map_locked(&prctl_map))
+ goto out;
+
+ if (prctl_map.auxv_size) {
+ up_read(&mm->mmap_sem);
+ memset(user_auxv, 0, sizeof(user_auxv));
+ error = copy_from_user(user_auxv,
+ (const void __user *)prctl_map.auxv,
+ prctl_map.auxv_size);
+ down_read(&mm->mmap_sem);
+ if (error)
+ goto out;
+ }
+
+ if (prctl_map.exe_fd != (u32)-1) {
+ error = prctl_set_mm_exe_file_locked(mm, prctl_map.exe_fd);
+ if (error)
+ goto out;
+ }
+
+ if (prctl_map.auxv_size) {
+ /* Last entry always must be AT_NULL */
+ user_auxv[AT_VECTOR_SIZE - 2] = AT_NULL;
+ user_auxv[AT_VECTOR_SIZE - 1] = AT_NULL;
+
+ task_lock(current);
+ memcpy(mm->saved_auxv, user_auxv, sizeof(user_auxv));
+ task_unlock(current);
+ }
+
+ mm->start_code = prctl_map.start_code;
+ mm->end_code = prctl_map.end_code;
+ mm->start_data = prctl_map.start_data;
+ mm->end_data = prctl_map.end_data;
+ mm->start_brk = prctl_map.start_brk;
+ mm->brk = prctl_map.brk;
+ mm->start_stack = prctl_map.start_stack;
+ mm->arg_start = prctl_map.arg_start;
+ mm->arg_end = prctl_map.arg_end;
+ mm->env_start = prctl_map.env_start;
+ mm->env_end = prctl_map.env_end;
+
+ error = 0;
+out:
+ up_read(&mm->mmap_sem);
+ return error;
+}
+#endif /* CONFIG_CHECKPOINT_RESTORE */
+
static int prctl_set_mm(int opt, unsigned long addr,
unsigned long arg4, unsigned long arg5)
{
@@ -1695,9 +1897,16 @@ static int prctl_set_mm(int opt, unsigne
struct vm_area_struct *vma;
int error;
- if (arg5 || (arg4 && opt != PR_SET_MM_AUXV))
+ if (arg5 || (arg4 && (opt != PR_SET_MM_AUXV &&
+ opt != PR_SET_MM_MAP &&
+ opt != PR_SET_MM_MAP_SIZE)))
return -EINVAL;
+#ifdef CONFIG_CHECKPOINT_RESTORE
+ if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
+ return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
+#endif
+
if (!capable(CAP_SYS_RESOURCE))
return -EPERM;
^ permalink raw reply [flat|nested] 22+ messages in thread
* [rfc 4/4] prctl: PR_SET_MM -- Introduce PR_SET_MM_MAP operation, v3
@ 2014-07-24 16:47 ` Cyrill Gorcunov
0 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 16:47 UTC (permalink / raw)
To: linux-kernel, linux-mm
Cc: gorcunov, keescook, tj, akpm, avagin, ebiederm, hpa,
serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages, jln
[-- Attachment #1: prctl-rework-new-mm-map-6 --]
[-- Type: text/plain, Size: 14670 bytes --]
During development of c/r we've noticed that in case if we need to
support user namespaces we face a problem with capabilities in
prctl(PR_SET_MM, ...) call, in particular once new user namespace
is created capable(CAP_SYS_RESOURCE) no longer passes.
A approach is to eliminate CAP_SYS_RESOURCE check but pass all
new values in one bundle, which would allow the kernel to make
more intensive test for sanity of values and same time allow us to
support checkpoint/restore of user namespaces.
Thus a new command PR_SET_MM_MAP introduced. It takes a pointer of
prctl_mm_map structure which carries all the members to be updated.
prctl(PR_SET_MM, PR_SET_MM_MAP, struct prctl_mm_map *, size)
struct prctl_mm_map {
__u64 start_code;
__u64 end_code;
__u64 start_data;
__u64 end_data;
__u64 start_brk;
__u64 brk;
__u64 start_stack;
__u64 arg_start;
__u64 arg_end;
__u64 env_start;
__u64 env_end;
__u64 *auxv;
__u32 auxv_size;
__u32 exe_fd;
};
All members except @exe_fd correspond ones of struct mm_struct.
To figure out which available values these members may take here
are meanings of the members.
- start_code, end_code: represent bounds of executable code area
- start_data, end_data: represent bounds of data area
- start_brk, brk: used to calculate bounds for brk() syscall
- start_stack: used when accounting space needed for command
line arguments, environment and shmat() syscall
- arg_start, arg_end, env_start, env_end: represent memory area
supplied for command line arguments and environment variables
- auxv, auxv_size: carries auxiliary vector, Elf format specifics
- exe_fd: file descriptor number for executable link (/proc/self/exe)
Thus we apply the following requirements to the values
1) Any member except @auxv, @auxv_size, @exe_fd is rather an address
in user space thus it must be laying inside [mmap_min_addr, mmap_max_addr)
interval.
2) While @[start|end]_code and @[start|end]_data may point to an nonexisting
VMAs (say a program maps own new .text and .data segments during execution)
the rest of members should belong to VMA which must exist.
3) Addresses must be ordered, ie @start_ member must not be greater or
equal to appropriate @end_ member.
4) As in regular Elf loading procedure we require that @start_brk and
@brk be greater than @end_data.
5) If RLIMIT_DATA rlimit is set to non-infinity new values should not
exceed existing limit. Same applies to RLIMIT_STACK.
6) Auxiliary vector size must not exceed existing one (which is
predefined as AT_VECTOR_SIZE and depends on architecture).
7) File descriptor passed in @exe_file should be pointing
to executable file (because we use existing prctl_set_mm_exe_file_locked
helper it ensures that the file we are going to use as exe link has all
required permission granted).
Now about where these members are involved inside kernel code:
- @start_code and @end_code are used in /proc/$pid/[stat|statm] output;
- @start_data and @end_data are used in /proc/$pid/[stat|statm] output,
also they are considered if there enough space for brk() syscall
result if RLIMIT_DATA is set;
- @start_brk shown in /proc/$pid/stat output and accounted in brk()
syscall if RLIMIT_DATA is set; also this member is tested to
find a symbolic name of mmap event for perf system (we choose
if event is generated for "heap" area); one more aplication is
selinux -- we test if a process has PROCESS__EXECHEAP permission
if trying to make heap area being executable with mprotect() syscall;
- @brk is a current value for brk() syscall which lays inside heap
area, it's shown in /proc/$pid/stat. When syscall brk() succesfully
provides new memory area to a user space upon brk() completion the
mm::brk is updated to carry new value;
Both @start_brk and @brk are actively used in /proc/$pid/maps
and /proc/$pid/smaps output to find a symbolic name "heap" for
VMA being scanned;
- @start_stack is printed out in /proc/$pid/stat and used to
find a symbolic name "stack" for task and threads in
/proc/$pid/maps and /proc/$pid/smaps output, and as the same
as with @start_brk -- perf system uses it for event naming.
Also kernel treat this member as a start address of where
to map vDSO pages and to check if there is enough space
for shmat() syscall;
- @arg_start, @arg_end, @env_start and @env_end are printed out
in /proc/$pid/stat. Another access to the data these members
represent is to read /proc/$pid/environ or /proc/$pid/cmdline.
Any attempt to read these areas kernel tests with access_process_vm
helper so a user must have enough rights for this action;
- @auxv and @auxv_size may be read from /proc/$pid/auxv. Strictly
speaking kernel doesn't care much about which exactly data is
sitting there because it is solely for userspace;
- @exe_fd is referred from /proc/$pid/exe and when generating
coredump. We uses prctl_set_mm_exe_file_locked helper to update
this member, so exe-file link modification remains one-shot
action.
Still note that updating exe-file link now doesn't require sys-resource
capability anymore, after all there is no much profit in preventing setup
own file link (there are a number of ways to execute own code -- ptrace,
ld-preload, so that the only reliable way to find which exactly code
is executed is to inspect running program memory). Still we require
the caller to be at least user-namespace root user.
I believe the old interface should be deprecated and ripped off
in a couple of kernel releases if no one against.
To test if new interface is implemented in the kernel one
can pass PR_SET_MM_MAP_SIZE opcode and the kernel returns
the size of currently supported struct prctl_mm_map.
v2:
- compact macros (by keescook@)
- wrap new code with CONFIG_ (by akpm@)
v3 (by jln@):
- use __prctl_check_order for brk and start_brk
- use may_adjust_brk helper
- make sure that only root can update @exe_fd link
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrew Vagin <avagin@openvz.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Julien Tinnes <jln@google.com>
---
include/uapi/linux/prctl.h | 25 +++++
kernel/sys.c | 211 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 235 insertions(+), 1 deletion(-)
Index: linux-2.6.git/include/uapi/linux/prctl.h
===================================================================
--- linux-2.6.git.orig/include/uapi/linux/prctl.h
+++ linux-2.6.git/include/uapi/linux/prctl.h
@@ -119,6 +119,31 @@
# define PR_SET_MM_ENV_END 11
# define PR_SET_MM_AUXV 12
# define PR_SET_MM_EXE_FILE 13
+# define PR_SET_MM_MAP 14
+# define PR_SET_MM_MAP_SIZE 15
+
+/*
+ * This structure provides new memory descriptor
+ * map which mostly modifies /proc/pid/stat[m]
+ * output for a task. This mostly done in a
+ * sake of checkpoint/restore functionality.
+ */
+struct prctl_mm_map {
+ __u64 start_code; /* code section bounds */
+ __u64 end_code;
+ __u64 start_data; /* data section bounds */
+ __u64 end_data;
+ __u64 start_brk; /* heap for brk() syscall */
+ __u64 brk;
+ __u64 start_stack; /* stack starts at */
+ __u64 arg_start; /* command line arguments bounds */
+ __u64 arg_end;
+ __u64 env_start; /* environment variables bounds */
+ __u64 env_end;
+ __u64 *auxv; /* auxiliary vector */
+ __u32 auxv_size; /* vector size */
+ __u32 exe_fd; /* /proc/$pid/exe link file */
+};
/*
* Set specific pid that is allowed to ptrace the current task.
Index: linux-2.6.git/kernel/sys.c
===================================================================
--- linux-2.6.git.orig/kernel/sys.c
+++ linux-2.6.git/kernel/sys.c
@@ -1687,6 +1687,208 @@ exit:
return err;
}
+#ifdef CONFIG_CHECKPOINT_RESTORE
+/*
+ * WARNING: we don't require any capability here so be very careful
+ * in what is allowed for modification from userspace.
+ */
+static int validate_prctl_map_locked(struct prctl_mm_map *prctl_map)
+{
+ unsigned long mmap_max_addr = TASK_SIZE;
+ struct mm_struct *mm = current->mm;
+ struct vm_area_struct *stack_vma;
+ int error = 0;
+
+ /*
+ * Make sure the members are not somewhere outside
+ * of allowed address space.
+ */
+#define __prctl_check_addr_space(__member) \
+ ({ \
+ int __rc; \
+ if ((unsigned long)prctl_map->__member < mmap_max_addr && \
+ (unsigned long)prctl_map->__member >= mmap_min_addr) \
+ __rc = 0; \
+ else \
+ __rc = -EINVAL; \
+ __rc; \
+ })
+ error |= __prctl_check_addr_space(start_code);
+ error |= __prctl_check_addr_space(end_code);
+ error |= __prctl_check_addr_space(start_data);
+ error |= __prctl_check_addr_space(end_data);
+ error |= __prctl_check_addr_space(start_stack);
+ error |= __prctl_check_addr_space(start_brk);
+ error |= __prctl_check_addr_space(brk);
+ error |= __prctl_check_addr_space(arg_start);
+ error |= __prctl_check_addr_space(arg_end);
+ error |= __prctl_check_addr_space(env_start);
+ error |= __prctl_check_addr_space(env_end);
+ if (error)
+ goto out;
+#undef __prctl_check_addr_space
+
+ /*
+ * Stack, brk, command line arguments and environment must exist.
+ */
+ stack_vma = find_vma(mm, (unsigned long)prctl_map->start_stack);
+ if (!stack_vma) {
+ error = -EINVAL;
+ goto out;
+ }
+#define __prctl_check_vma(__member) \
+ find_vma(mm, (unsigned long)prctl_map->__member) ? 0 : -EINVAL
+ error |= __prctl_check_vma(start_brk);
+ error |= __prctl_check_vma(brk);
+ error |= __prctl_check_vma(arg_start);
+ error |= __prctl_check_vma(arg_end);
+ error |= __prctl_check_vma(env_start);
+ error |= __prctl_check_vma(env_end);
+ if (error)
+ goto out;
+#undef __prctl_check_vma
+
+ /*
+ * Make sure the pairs are ordered.
+ */
+#define __prctl_check_order(__m1, __op, __m2) \
+ ((unsigned long)prctl_map->__m1 __op \
+ (unsigned long)prctl_map->__m2) ? 0 : -EINVAL
+ error |= __prctl_check_order(start_code, <, end_code);
+ error |= __prctl_check_order(start_data, <, end_data);
+ error |= __prctl_check_order(start_brk, <=, brk);
+ error |= __prctl_check_order(arg_start, <, arg_end);
+ error |= __prctl_check_order(env_start, <, env_end);
+ if (error)
+ goto out;
+#undef __prctl_check_order
+
+ error = -EINVAL;
+
+ /*
+ * @brk should be after @end_data in traditional maps.
+ */
+ if (prctl_map->start_brk <= prctl_map->end_data ||
+ prctl_map->brk <= prctl_map->end_data)
+ goto out;
+
+ /*
+ * Neither we should allow to override limits if they set.
+ */
+ if (may_adjust_brk(rlimit(RLIMIT_DATA), prctl_map->brk,
+ prctl_map->start_brk, prctl_map->end_data,
+ prctl_map->start_data))
+ goto out;
+
+#ifdef CONFIG_STACK_GROWSUP
+ if (may_adjust_brk(rlimit(RLIMIT_STACK),
+ stack_vma->vm_end,
+ prctl_map->start_stack, 0, 0))
+#else
+ if (may_adjust_brk(rlimit(RLIMIT_STACK),
+ prctl_map->start_stack,
+ stack_vma->vm_start, 0, 0))
+#endif
+ goto out;
+
+ /*
+ * Someone is trying to cheat the auxv vector.
+ */
+ if (prctl_map->auxv_size) {
+ if (!prctl_map->auxv ||
+ prctl_map->auxv_size > sizeof(mm->saved_auxv))
+ goto out;
+ }
+
+ /*
+ * Finally, make sure the caller has the rights to
+ * change /proc/pid/exe link: only local root should
+ * be allowed to.
+ */
+ if (prctl_map->exe_fd != (u32)-1) {
+ struct user_namespace *ns = current_user_ns();
+ const struct cred *cred = current_cred();
+
+ if (!uid_eq(cred->uid, make_kuid(ns, 0)) ||
+ !gid_eq(cred->gid, make_kgid(ns, 0)))
+ goto out;
+ }
+
+ error = 0;
+out:
+ return error;
+}
+
+static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
+{
+ struct prctl_mm_map prctl_map = { .exe_fd = (u32)-1, };
+ unsigned long user_auxv[AT_VECTOR_SIZE];
+ struct mm_struct *mm = current->mm;
+ int error = -EINVAL;
+
+ BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
+
+ if (opt == PR_SET_MM_MAP_SIZE)
+ return put_user((unsigned int)sizeof(prctl_map),
+ (unsigned int __user *)addr);
+
+ if (data_size != sizeof(prctl_map))
+ return -EINVAL;
+
+ if (copy_from_user(&prctl_map, addr, sizeof(prctl_map)))
+ return -EFAULT;
+
+ down_read(&mm->mmap_sem);
+
+ if (validate_prctl_map_locked(&prctl_map))
+ goto out;
+
+ if (prctl_map.auxv_size) {
+ up_read(&mm->mmap_sem);
+ memset(user_auxv, 0, sizeof(user_auxv));
+ error = copy_from_user(user_auxv,
+ (const void __user *)prctl_map.auxv,
+ prctl_map.auxv_size);
+ down_read(&mm->mmap_sem);
+ if (error)
+ goto out;
+ }
+
+ if (prctl_map.exe_fd != (u32)-1) {
+ error = prctl_set_mm_exe_file_locked(mm, prctl_map.exe_fd);
+ if (error)
+ goto out;
+ }
+
+ if (prctl_map.auxv_size) {
+ /* Last entry always must be AT_NULL */
+ user_auxv[AT_VECTOR_SIZE - 2] = AT_NULL;
+ user_auxv[AT_VECTOR_SIZE - 1] = AT_NULL;
+
+ task_lock(current);
+ memcpy(mm->saved_auxv, user_auxv, sizeof(user_auxv));
+ task_unlock(current);
+ }
+
+ mm->start_code = prctl_map.start_code;
+ mm->end_code = prctl_map.end_code;
+ mm->start_data = prctl_map.start_data;
+ mm->end_data = prctl_map.end_data;
+ mm->start_brk = prctl_map.start_brk;
+ mm->brk = prctl_map.brk;
+ mm->start_stack = prctl_map.start_stack;
+ mm->arg_start = prctl_map.arg_start;
+ mm->arg_end = prctl_map.arg_end;
+ mm->env_start = prctl_map.env_start;
+ mm->env_end = prctl_map.env_end;
+
+ error = 0;
+out:
+ up_read(&mm->mmap_sem);
+ return error;
+}
+#endif /* CONFIG_CHECKPOINT_RESTORE */
+
static int prctl_set_mm(int opt, unsigned long addr,
unsigned long arg4, unsigned long arg5)
{
@@ -1695,9 +1897,16 @@ static int prctl_set_mm(int opt, unsigne
struct vm_area_struct *vma;
int error;
- if (arg5 || (arg4 && opt != PR_SET_MM_AUXV))
+ if (arg5 || (arg4 && (opt != PR_SET_MM_AUXV &&
+ opt != PR_SET_MM_MAP &&
+ opt != PR_SET_MM_MAP_SIZE)))
return -EINVAL;
+#ifdef CONFIG_CHECKPOINT_RESTORE
+ if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
+ return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
+#endif
+
if (!capable(CAP_SYS_RESOURCE))
return -EPERM;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 1/4] mm: Introduce may_adjust_brk helper
2014-07-24 16:46 ` Cyrill Gorcunov
@ 2014-07-24 19:18 ` Kees Cook
-1 siblings, 0 replies; 22+ messages in thread
From: Kees Cook @ 2014-07-24 19:18 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: LKML, linux-mm, Tejun Heo, Andrew Morton, Andrew Vagin,
Eric W. Biederman, H. Peter Anvin, Serge Hallyn, Pavel Emelyanov,
Vasiliy Kulikov, KAMEZAWA Hiroyuki, Michael Kerrisk-manpages,
Julien Tinnes
On Thu, Jul 24, 2014 at 9:46 AM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> To eliminate code duplication lets introduce may_adjust_brk
> helper which we will use in brk() and prctl() syscalls.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andrew Vagin <avagin@openvz.org>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Serge Hallyn <serge.hallyn@canonical.com>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Vasiliy Kulikov <segoon@openwall.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Cc: Julien Tinnes <jln@google.com>
> ---
> include/linux/mm.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> Index: linux-2.6.git/include/linux/mm.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/mm.h
> +++ linux-2.6.git/include/linux/mm.h
> @@ -18,6 +18,7 @@
> #include <linux/pfn.h>
> #include <linux/bit_spinlock.h>
> #include <linux/shrinker.h>
> +#include <linux/resource.h>
>
> struct mempolicy;
> struct anon_vma;
> @@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
> bool *need_rmap_locks);
> extern void exit_mmap(struct mm_struct *);
>
> +static inline int may_adjust_brk(unsigned long rlim,
> + unsigned long new_brk,
> + unsigned long start_brk,
> + unsigned long end_data,
> + unsigned long start_data)
> +{
> + if (rlim < RLIMIT_DATA) {
Won't rlim always be the value from a call to rlimit(RLIMIT_DATA)? Is
there a good reason to not just put the rlimit() call in
may_adjust_brk()? This would actually be an optimization in the
prctl_set_mm case, since now it calls rlimit() unconditionally, but
doesn't need to.
-Kees
> + if (((new_brk - start_brk) + (end_data - start_data)) > rlim)
> + return -ENOSPC;
> + }
> + return 0;
> +}
> +
> extern int mm_take_all_locks(struct mm_struct *mm);
> extern void mm_drop_all_locks(struct mm_struct *mm);
>
>
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 1/4] mm: Introduce may_adjust_brk helper
@ 2014-07-24 19:18 ` Kees Cook
0 siblings, 0 replies; 22+ messages in thread
From: Kees Cook @ 2014-07-24 19:18 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: LKML, linux-mm, Tejun Heo, Andrew Morton, Andrew Vagin,
Eric W. Biederman, H. Peter Anvin, Serge Hallyn, Pavel Emelyanov,
Vasiliy Kulikov, KAMEZAWA Hiroyuki, Michael Kerrisk-manpages,
Julien Tinnes
On Thu, Jul 24, 2014 at 9:46 AM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> To eliminate code duplication lets introduce may_adjust_brk
> helper which we will use in brk() and prctl() syscalls.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andrew Vagin <avagin@openvz.org>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Serge Hallyn <serge.hallyn@canonical.com>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Vasiliy Kulikov <segoon@openwall.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Cc: Julien Tinnes <jln@google.com>
> ---
> include/linux/mm.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> Index: linux-2.6.git/include/linux/mm.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/mm.h
> +++ linux-2.6.git/include/linux/mm.h
> @@ -18,6 +18,7 @@
> #include <linux/pfn.h>
> #include <linux/bit_spinlock.h>
> #include <linux/shrinker.h>
> +#include <linux/resource.h>
>
> struct mempolicy;
> struct anon_vma;
> @@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
> bool *need_rmap_locks);
> extern void exit_mmap(struct mm_struct *);
>
> +static inline int may_adjust_brk(unsigned long rlim,
> + unsigned long new_brk,
> + unsigned long start_brk,
> + unsigned long end_data,
> + unsigned long start_data)
> +{
> + if (rlim < RLIMIT_DATA) {
Won't rlim always be the value from a call to rlimit(RLIMIT_DATA)? Is
there a good reason to not just put the rlimit() call in
may_adjust_brk()? This would actually be an optimization in the
prctl_set_mm case, since now it calls rlimit() unconditionally, but
doesn't need to.
-Kees
> + if (((new_brk - start_brk) + (end_data - start_data)) > rlim)
> + return -ENOSPC;
> + }
> + return 0;
> +}
> +
> extern int mm_take_all_locks(struct mm_struct *mm);
> extern void mm_drop_all_locks(struct mm_struct *mm);
>
>
--
Kees Cook
Chrome OS Security
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 1/4] mm: Introduce may_adjust_brk helper
2014-07-24 19:18 ` Kees Cook
@ 2014-07-24 19:21 ` Cyrill Gorcunov
-1 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 19:21 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, linux-mm, Tejun Heo, Andrew Morton, Andrew Vagin,
Eric W. Biederman, H. Peter Anvin, Serge Hallyn, Pavel Emelyanov,
Vasiliy Kulikov, KAMEZAWA Hiroyuki, Michael Kerrisk-manpages,
Julien Tinnes
On Thu, Jul 24, 2014 at 12:18:56PM -0700, Kees Cook wrote:
> >
> > +static inline int may_adjust_brk(unsigned long rlim,
> > + unsigned long new_brk,
> > + unsigned long start_brk,
> > + unsigned long end_data,
> > + unsigned long start_data)
> > +{
> > + if (rlim < RLIMIT_DATA) {
>
> Won't rlim always be the value from a call to rlimit(RLIMIT_DATA)? Is
> there a good reason to not just put the rlimit() call in
> may_adjust_brk()? This would actually be an optimization in the
> prctl_set_mm case, since now it calls rlimit() unconditionally, but
> doesn't need to.
Nope, we use it for rlimit(RLIMIT_STACK) when checking for
@start_stack member.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 1/4] mm: Introduce may_adjust_brk helper
@ 2014-07-24 19:21 ` Cyrill Gorcunov
0 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 19:21 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, linux-mm, Tejun Heo, Andrew Morton, Andrew Vagin,
Eric W. Biederman, H. Peter Anvin, Serge Hallyn, Pavel Emelyanov,
Vasiliy Kulikov, KAMEZAWA Hiroyuki, Michael Kerrisk-manpages,
Julien Tinnes
On Thu, Jul 24, 2014 at 12:18:56PM -0700, Kees Cook wrote:
> >
> > +static inline int may_adjust_brk(unsigned long rlim,
> > + unsigned long new_brk,
> > + unsigned long start_brk,
> > + unsigned long end_data,
> > + unsigned long start_data)
> > +{
> > + if (rlim < RLIMIT_DATA) {
>
> Won't rlim always be the value from a call to rlimit(RLIMIT_DATA)? Is
> there a good reason to not just put the rlimit() call in
> may_adjust_brk()? This would actually be an optimization in the
> prctl_set_mm case, since now it calls rlimit() unconditionally, but
> doesn't need to.
Nope, we use it for rlimit(RLIMIT_STACK) when checking for
@start_stack member.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 4/4] prctl: PR_SET_MM -- Introduce PR_SET_MM_MAP operation, v3
2014-07-24 16:47 ` Cyrill Gorcunov
@ 2014-07-24 19:31 ` Kees Cook
-1 siblings, 0 replies; 22+ messages in thread
From: Kees Cook @ 2014-07-24 19:31 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: LKML, linux-mm, Tejun Heo, Andrew Morton, Andrew Vagin,
Eric W. Biederman, H. Peter Anvin, Serge Hallyn, Pavel Emelyanov,
Vasiliy Kulikov, KAMEZAWA Hiroyuki, Michael Kerrisk-manpages,
Julien Tinnes
On Thu, Jul 24, 2014 at 9:47 AM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> prctl(PR_SET_MM, ...) call, in particular once new user namespace
> is created capable(CAP_SYS_RESOURCE) no longer passes.
>
> A approach is to eliminate CAP_SYS_RESOURCE check but pass all
> new values in one bundle, which would allow the kernel to make
> more intensive test for sanity of values and same time allow us to
> support checkpoint/restore of user namespaces.
>
> Thus a new command PR_SET_MM_MAP introduced. It takes a pointer of
> prctl_mm_map structure which carries all the members to be updated.
>
> prctl(PR_SET_MM, PR_SET_MM_MAP, struct prctl_mm_map *, size)
>
> struct prctl_mm_map {
> __u64 start_code;
> __u64 end_code;
> __u64 start_data;
> __u64 end_data;
> __u64 start_brk;
> __u64 brk;
> __u64 start_stack;
> __u64 arg_start;
> __u64 arg_end;
> __u64 env_start;
> __u64 env_end;
> __u64 *auxv;
> __u32 auxv_size;
> __u32 exe_fd;
> };
>
> All members except @exe_fd correspond ones of struct mm_struct.
> To figure out which available values these members may take here
> are meanings of the members.
>
> - start_code, end_code: represent bounds of executable code area
> - start_data, end_data: represent bounds of data area
> - start_brk, brk: used to calculate bounds for brk() syscall
> - start_stack: used when accounting space needed for command
> line arguments, environment and shmat() syscall
> - arg_start, arg_end, env_start, env_end: represent memory area
> supplied for command line arguments and environment variables
> - auxv, auxv_size: carries auxiliary vector, Elf format specifics
> - exe_fd: file descriptor number for executable link (/proc/self/exe)
>
> Thus we apply the following requirements to the values
>
> 1) Any member except @auxv, @auxv_size, @exe_fd is rather an address
> in user space thus it must be laying inside [mmap_min_addr, mmap_max_addr)
> interval.
>
> 2) While @[start|end]_code and @[start|end]_data may point to an nonexisting
> VMAs (say a program maps own new .text and .data segments during execution)
> the rest of members should belong to VMA which must exist.
>
> 3) Addresses must be ordered, ie @start_ member must not be greater or
> equal to appropriate @end_ member.
>
> 4) As in regular Elf loading procedure we require that @start_brk and
> @brk be greater than @end_data.
>
> 5) If RLIMIT_DATA rlimit is set to non-infinity new values should not
> exceed existing limit. Same applies to RLIMIT_STACK.
>
> 6) Auxiliary vector size must not exceed existing one (which is
> predefined as AT_VECTOR_SIZE and depends on architecture).
>
> 7) File descriptor passed in @exe_file should be pointing
> to executable file (because we use existing prctl_set_mm_exe_file_locked
> helper it ensures that the file we are going to use as exe link has all
> required permission granted).
>
> Now about where these members are involved inside kernel code:
>
> - @start_code and @end_code are used in /proc/$pid/[stat|statm] output;
>
> - @start_data and @end_data are used in /proc/$pid/[stat|statm] output,
> also they are considered if there enough space for brk() syscall
> result if RLIMIT_DATA is set;
>
> - @start_brk shown in /proc/$pid/stat output and accounted in brk()
> syscall if RLIMIT_DATA is set; also this member is tested to
> find a symbolic name of mmap event for perf system (we choose
> if event is generated for "heap" area); one more aplication is
> selinux -- we test if a process has PROCESS__EXECHEAP permission
> if trying to make heap area being executable with mprotect() syscall;
>
> - @brk is a current value for brk() syscall which lays inside heap
> area, it's shown in /proc/$pid/stat. When syscall brk() succesfully
> provides new memory area to a user space upon brk() completion the
> mm::brk is updated to carry new value;
>
> Both @start_brk and @brk are actively used in /proc/$pid/maps
> and /proc/$pid/smaps output to find a symbolic name "heap" for
> VMA being scanned;
>
> - @start_stack is printed out in /proc/$pid/stat and used to
> find a symbolic name "stack" for task and threads in
> /proc/$pid/maps and /proc/$pid/smaps output, and as the same
> as with @start_brk -- perf system uses it for event naming.
> Also kernel treat this member as a start address of where
> to map vDSO pages and to check if there is enough space
> for shmat() syscall;
>
> - @arg_start, @arg_end, @env_start and @env_end are printed out
> in /proc/$pid/stat. Another access to the data these members
> represent is to read /proc/$pid/environ or /proc/$pid/cmdline.
> Any attempt to read these areas kernel tests with access_process_vm
> helper so a user must have enough rights for this action;
>
> - @auxv and @auxv_size may be read from /proc/$pid/auxv. Strictly
> speaking kernel doesn't care much about which exactly data is
> sitting there because it is solely for userspace;
>
> - @exe_fd is referred from /proc/$pid/exe and when generating
> coredump. We uses prctl_set_mm_exe_file_locked helper to update
> this member, so exe-file link modification remains one-shot
> action.
>
> Still note that updating exe-file link now doesn't require sys-resource
> capability anymore, after all there is no much profit in preventing setup
> own file link (there are a number of ways to execute own code -- ptrace,
> ld-preload, so that the only reliable way to find which exactly code
> is executed is to inspect running program memory). Still we require
> the caller to be at least user-namespace root user.
>
> I believe the old interface should be deprecated and ripped off
> in a couple of kernel releases if no one against.
>
> To test if new interface is implemented in the kernel one
> can pass PR_SET_MM_MAP_SIZE opcode and the kernel returns
> the size of currently supported struct prctl_mm_map.
>
> v2:
> - compact macros (by keescook@)
> - wrap new code with CONFIG_ (by akpm@)
>
> v3 (by jln@):
> - use __prctl_check_order for brk and start_brk
> - use may_adjust_brk helper
> - make sure that only root can update @exe_fd link
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andrew Vagin <avagin@openvz.org>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Serge Hallyn <serge.hallyn@canonical.com>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Vasiliy Kulikov <segoon@openwall.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Cc: Julien Tinnes <jln@google.com>
> ---
> include/uapi/linux/prctl.h | 25 +++++
> kernel/sys.c | 211 ++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 235 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.git/include/uapi/linux/prctl.h
> ===================================================================
> --- linux-2.6.git.orig/include/uapi/linux/prctl.h
> +++ linux-2.6.git/include/uapi/linux/prctl.h
> @@ -119,6 +119,31 @@
> # define PR_SET_MM_ENV_END 11
> # define PR_SET_MM_AUXV 12
> # define PR_SET_MM_EXE_FILE 13
> +# define PR_SET_MM_MAP 14
> +# define PR_SET_MM_MAP_SIZE 15
> +
> +/*
> + * This structure provides new memory descriptor
> + * map which mostly modifies /proc/pid/stat[m]
> + * output for a task. This mostly done in a
> + * sake of checkpoint/restore functionality.
> + */
> +struct prctl_mm_map {
> + __u64 start_code; /* code section bounds */
> + __u64 end_code;
> + __u64 start_data; /* data section bounds */
> + __u64 end_data;
> + __u64 start_brk; /* heap for brk() syscall */
> + __u64 brk;
> + __u64 start_stack; /* stack starts at */
> + __u64 arg_start; /* command line arguments bounds */
> + __u64 arg_end;
> + __u64 env_start; /* environment variables bounds */
> + __u64 env_end;
> + __u64 *auxv; /* auxiliary vector */
> + __u32 auxv_size; /* vector size */
> + __u32 exe_fd; /* /proc/$pid/exe link file */
> +};
>
> /*
> * Set specific pid that is allowed to ptrace the current task.
> Index: linux-2.6.git/kernel/sys.c
> ===================================================================
> --- linux-2.6.git.orig/kernel/sys.c
> +++ linux-2.6.git/kernel/sys.c
> @@ -1687,6 +1687,208 @@ exit:
> return err;
> }
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +/*
> + * WARNING: we don't require any capability here so be very careful
> + * in what is allowed for modification from userspace.
> + */
> +static int validate_prctl_map_locked(struct prctl_mm_map *prctl_map)
> +{
> + unsigned long mmap_max_addr = TASK_SIZE;
> + struct mm_struct *mm = current->mm;
> + struct vm_area_struct *stack_vma;
> + int error = 0;
> +
> + /*
> + * Make sure the members are not somewhere outside
> + * of allowed address space.
> + */
> +#define __prctl_check_addr_space(__member) \
> + ({ \
> + int __rc; \
> + if ((unsigned long)prctl_map->__member < mmap_max_addr && \
> + (unsigned long)prctl_map->__member >= mmap_min_addr) \
> + __rc = 0; \
> + else \
> + __rc = -EINVAL; \
> + __rc; \
> + })
> + error |= __prctl_check_addr_space(start_code);
> + error |= __prctl_check_addr_space(end_code);
> + error |= __prctl_check_addr_space(start_data);
> + error |= __prctl_check_addr_space(end_data);
> + error |= __prctl_check_addr_space(start_stack);
> + error |= __prctl_check_addr_space(start_brk);
> + error |= __prctl_check_addr_space(brk);
> + error |= __prctl_check_addr_space(arg_start);
> + error |= __prctl_check_addr_space(arg_end);
> + error |= __prctl_check_addr_space(env_start);
> + error |= __prctl_check_addr_space(env_end);
> + if (error)
> + goto out;
> +#undef __prctl_check_addr_space
> +
> + /*
> + * Stack, brk, command line arguments and environment must exist.
> + */
> + stack_vma = find_vma(mm, (unsigned long)prctl_map->start_stack);
> + if (!stack_vma) {
> + error = -EINVAL;
> + goto out;
> + }
> +#define __prctl_check_vma(__member) \
> + find_vma(mm, (unsigned long)prctl_map->__member) ? 0 : -EINVAL
> + error |= __prctl_check_vma(start_brk);
> + error |= __prctl_check_vma(brk);
> + error |= __prctl_check_vma(arg_start);
> + error |= __prctl_check_vma(arg_end);
> + error |= __prctl_check_vma(env_start);
> + error |= __prctl_check_vma(env_end);
> + if (error)
> + goto out;
> +#undef __prctl_check_vma
> +
> + /*
> + * Make sure the pairs are ordered.
> + */
> +#define __prctl_check_order(__m1, __op, __m2) \
> + ((unsigned long)prctl_map->__m1 __op \
> + (unsigned long)prctl_map->__m2) ? 0 : -EINVAL
> + error |= __prctl_check_order(start_code, <, end_code);
> + error |= __prctl_check_order(start_data, <, end_data);
> + error |= __prctl_check_order(start_brk, <=, brk);
> + error |= __prctl_check_order(arg_start, <, arg_end);
> + error |= __prctl_check_order(env_start, <, env_end);
> + if (error)
> + goto out;
> +#undef __prctl_check_order
> +
> + error = -EINVAL;
> +
> + /*
> + * @brk should be after @end_data in traditional maps.
> + */
> + if (prctl_map->start_brk <= prctl_map->end_data ||
> + prctl_map->brk <= prctl_map->end_data)
> + goto out;
> +
> + /*
> + * Neither we should allow to override limits if they set.
> + */
> + if (may_adjust_brk(rlimit(RLIMIT_DATA), prctl_map->brk,
> + prctl_map->start_brk, prctl_map->end_data,
> + prctl_map->start_data))
> + goto out;
> +
> +#ifdef CONFIG_STACK_GROWSUP
> + if (may_adjust_brk(rlimit(RLIMIT_STACK),
> + stack_vma->vm_end,
> + prctl_map->start_stack, 0, 0))
> +#else
> + if (may_adjust_brk(rlimit(RLIMIT_STACK),
> + prctl_map->start_stack,
> + stack_vma->vm_start, 0, 0))
> +#endif
> + goto out;
Ah! Sorry, I missed this use of may_adjust_brk here. Perhaps rename
it, since we're not checking brk here, and pass the RLIMIT_* value to
the function, which can look it up itself? "check_vma_rlimit" ?
> +
> + /*
> + * Someone is trying to cheat the auxv vector.
> + */
> + if (prctl_map->auxv_size) {
> + if (!prctl_map->auxv ||
> + prctl_map->auxv_size > sizeof(mm->saved_auxv))
> + goto out;
> + }
> +
> + /*
> + * Finally, make sure the caller has the rights to
> + * change /proc/pid/exe link: only local root should
> + * be allowed to.
> + */
> + if (prctl_map->exe_fd != (u32)-1) {
> + struct user_namespace *ns = current_user_ns();
> + const struct cred *cred = current_cred();
> +
> + if (!uid_eq(cred->uid, make_kuid(ns, 0)) ||
> + !gid_eq(cred->gid, make_kgid(ns, 0)))
> + goto out;
> + }
I got tricked for a moment here. :) I see that even if we pass this
check, prctl_set_mm_exe_file will still do the additional checks too
during prctl_set_mm_map. Excellent!
> +
> + error = 0;
> +out:
> + return error;
> +}
> +
> +static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
> +{
> + struct prctl_mm_map prctl_map = { .exe_fd = (u32)-1, };
> + unsigned long user_auxv[AT_VECTOR_SIZE];
> + struct mm_struct *mm = current->mm;
> + int error = -EINVAL;
> +
> + BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
> +
> + if (opt == PR_SET_MM_MAP_SIZE)
> + return put_user((unsigned int)sizeof(prctl_map),
> + (unsigned int __user *)addr);
> +
> + if (data_size != sizeof(prctl_map))
> + return -EINVAL;
> +
> + if (copy_from_user(&prctl_map, addr, sizeof(prctl_map)))
> + return -EFAULT;
> +
> + down_read(&mm->mmap_sem);
> +
> + if (validate_prctl_map_locked(&prctl_map))
> + goto out;
> +
> + if (prctl_map.auxv_size) {
> + up_read(&mm->mmap_sem);
> + memset(user_auxv, 0, sizeof(user_auxv));
> + error = copy_from_user(user_auxv,
> + (const void __user *)prctl_map.auxv,
> + prctl_map.auxv_size);
> + down_read(&mm->mmap_sem);
> + if (error)
> + goto out;
> + }
> +
> + if (prctl_map.exe_fd != (u32)-1) {
> + error = prctl_set_mm_exe_file_locked(mm, prctl_map.exe_fd);
> + if (error)
> + goto out;
> + }
> +
> + if (prctl_map.auxv_size) {
> + /* Last entry always must be AT_NULL */
> + user_auxv[AT_VECTOR_SIZE - 2] = AT_NULL;
> + user_auxv[AT_VECTOR_SIZE - 1] = AT_NULL;
> +
> + task_lock(current);
> + memcpy(mm->saved_auxv, user_auxv, sizeof(user_auxv));
> + task_unlock(current);
> + }
> +
> + mm->start_code = prctl_map.start_code;
> + mm->end_code = prctl_map.end_code;
> + mm->start_data = prctl_map.start_data;
> + mm->end_data = prctl_map.end_data;
> + mm->start_brk = prctl_map.start_brk;
> + mm->brk = prctl_map.brk;
> + mm->start_stack = prctl_map.start_stack;
> + mm->arg_start = prctl_map.arg_start;
> + mm->arg_end = prctl_map.arg_end;
> + mm->env_start = prctl_map.env_start;
> + mm->env_end = prctl_map.env_end;
> +
> + error = 0;
> +out:
> + up_read(&mm->mmap_sem);
> + return error;
> +}
> +#endif /* CONFIG_CHECKPOINT_RESTORE */
> +
> static int prctl_set_mm(int opt, unsigned long addr,
> unsigned long arg4, unsigned long arg5)
> {
> @@ -1695,9 +1897,16 @@ static int prctl_set_mm(int opt, unsigne
> struct vm_area_struct *vma;
> int error;
>
> - if (arg5 || (arg4 && opt != PR_SET_MM_AUXV))
> + if (arg5 || (arg4 && (opt != PR_SET_MM_AUXV &&
> + opt != PR_SET_MM_MAP &&
> + opt != PR_SET_MM_MAP_SIZE)))
> return -EINVAL;
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> + if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
> + return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
> +#endif
> +
> if (!capable(CAP_SYS_RESOURCE))
> return -EPERM;
>
>
I think this is looking good. Thanks for the refactoring!
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 4/4] prctl: PR_SET_MM -- Introduce PR_SET_MM_MAP operation, v3
@ 2014-07-24 19:31 ` Kees Cook
0 siblings, 0 replies; 22+ messages in thread
From: Kees Cook @ 2014-07-24 19:31 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: LKML, linux-mm, Tejun Heo, Andrew Morton, Andrew Vagin,
Eric W. Biederman, H. Peter Anvin, Serge Hallyn, Pavel Emelyanov,
Vasiliy Kulikov, KAMEZAWA Hiroyuki, Michael Kerrisk-manpages,
Julien Tinnes
On Thu, Jul 24, 2014 at 9:47 AM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> During development of c/r we've noticed that in case if we need to
> support user namespaces we face a problem with capabilities in
> prctl(PR_SET_MM, ...) call, in particular once new user namespace
> is created capable(CAP_SYS_RESOURCE) no longer passes.
>
> A approach is to eliminate CAP_SYS_RESOURCE check but pass all
> new values in one bundle, which would allow the kernel to make
> more intensive test for sanity of values and same time allow us to
> support checkpoint/restore of user namespaces.
>
> Thus a new command PR_SET_MM_MAP introduced. It takes a pointer of
> prctl_mm_map structure which carries all the members to be updated.
>
> prctl(PR_SET_MM, PR_SET_MM_MAP, struct prctl_mm_map *, size)
>
> struct prctl_mm_map {
> __u64 start_code;
> __u64 end_code;
> __u64 start_data;
> __u64 end_data;
> __u64 start_brk;
> __u64 brk;
> __u64 start_stack;
> __u64 arg_start;
> __u64 arg_end;
> __u64 env_start;
> __u64 env_end;
> __u64 *auxv;
> __u32 auxv_size;
> __u32 exe_fd;
> };
>
> All members except @exe_fd correspond ones of struct mm_struct.
> To figure out which available values these members may take here
> are meanings of the members.
>
> - start_code, end_code: represent bounds of executable code area
> - start_data, end_data: represent bounds of data area
> - start_brk, brk: used to calculate bounds for brk() syscall
> - start_stack: used when accounting space needed for command
> line arguments, environment and shmat() syscall
> - arg_start, arg_end, env_start, env_end: represent memory area
> supplied for command line arguments and environment variables
> - auxv, auxv_size: carries auxiliary vector, Elf format specifics
> - exe_fd: file descriptor number for executable link (/proc/self/exe)
>
> Thus we apply the following requirements to the values
>
> 1) Any member except @auxv, @auxv_size, @exe_fd is rather an address
> in user space thus it must be laying inside [mmap_min_addr, mmap_max_addr)
> interval.
>
> 2) While @[start|end]_code and @[start|end]_data may point to an nonexisting
> VMAs (say a program maps own new .text and .data segments during execution)
> the rest of members should belong to VMA which must exist.
>
> 3) Addresses must be ordered, ie @start_ member must not be greater or
> equal to appropriate @end_ member.
>
> 4) As in regular Elf loading procedure we require that @start_brk and
> @brk be greater than @end_data.
>
> 5) If RLIMIT_DATA rlimit is set to non-infinity new values should not
> exceed existing limit. Same applies to RLIMIT_STACK.
>
> 6) Auxiliary vector size must not exceed existing one (which is
> predefined as AT_VECTOR_SIZE and depends on architecture).
>
> 7) File descriptor passed in @exe_file should be pointing
> to executable file (because we use existing prctl_set_mm_exe_file_locked
> helper it ensures that the file we are going to use as exe link has all
> required permission granted).
>
> Now about where these members are involved inside kernel code:
>
> - @start_code and @end_code are used in /proc/$pid/[stat|statm] output;
>
> - @start_data and @end_data are used in /proc/$pid/[stat|statm] output,
> also they are considered if there enough space for brk() syscall
> result if RLIMIT_DATA is set;
>
> - @start_brk shown in /proc/$pid/stat output and accounted in brk()
> syscall if RLIMIT_DATA is set; also this member is tested to
> find a symbolic name of mmap event for perf system (we choose
> if event is generated for "heap" area); one more aplication is
> selinux -- we test if a process has PROCESS__EXECHEAP permission
> if trying to make heap area being executable with mprotect() syscall;
>
> - @brk is a current value for brk() syscall which lays inside heap
> area, it's shown in /proc/$pid/stat. When syscall brk() succesfully
> provides new memory area to a user space upon brk() completion the
> mm::brk is updated to carry new value;
>
> Both @start_brk and @brk are actively used in /proc/$pid/maps
> and /proc/$pid/smaps output to find a symbolic name "heap" for
> VMA being scanned;
>
> - @start_stack is printed out in /proc/$pid/stat and used to
> find a symbolic name "stack" for task and threads in
> /proc/$pid/maps and /proc/$pid/smaps output, and as the same
> as with @start_brk -- perf system uses it for event naming.
> Also kernel treat this member as a start address of where
> to map vDSO pages and to check if there is enough space
> for shmat() syscall;
>
> - @arg_start, @arg_end, @env_start and @env_end are printed out
> in /proc/$pid/stat. Another access to the data these members
> represent is to read /proc/$pid/environ or /proc/$pid/cmdline.
> Any attempt to read these areas kernel tests with access_process_vm
> helper so a user must have enough rights for this action;
>
> - @auxv and @auxv_size may be read from /proc/$pid/auxv. Strictly
> speaking kernel doesn't care much about which exactly data is
> sitting there because it is solely for userspace;
>
> - @exe_fd is referred from /proc/$pid/exe and when generating
> coredump. We uses prctl_set_mm_exe_file_locked helper to update
> this member, so exe-file link modification remains one-shot
> action.
>
> Still note that updating exe-file link now doesn't require sys-resource
> capability anymore, after all there is no much profit in preventing setup
> own file link (there are a number of ways to execute own code -- ptrace,
> ld-preload, so that the only reliable way to find which exactly code
> is executed is to inspect running program memory). Still we require
> the caller to be at least user-namespace root user.
>
> I believe the old interface should be deprecated and ripped off
> in a couple of kernel releases if no one against.
>
> To test if new interface is implemented in the kernel one
> can pass PR_SET_MM_MAP_SIZE opcode and the kernel returns
> the size of currently supported struct prctl_mm_map.
>
> v2:
> - compact macros (by keescook@)
> - wrap new code with CONFIG_ (by akpm@)
>
> v3 (by jln@):
> - use __prctl_check_order for brk and start_brk
> - use may_adjust_brk helper
> - make sure that only root can update @exe_fd link
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andrew Vagin <avagin@openvz.org>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Serge Hallyn <serge.hallyn@canonical.com>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Vasiliy Kulikov <segoon@openwall.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Cc: Julien Tinnes <jln@google.com>
> ---
> include/uapi/linux/prctl.h | 25 +++++
> kernel/sys.c | 211 ++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 235 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.git/include/uapi/linux/prctl.h
> ===================================================================
> --- linux-2.6.git.orig/include/uapi/linux/prctl.h
> +++ linux-2.6.git/include/uapi/linux/prctl.h
> @@ -119,6 +119,31 @@
> # define PR_SET_MM_ENV_END 11
> # define PR_SET_MM_AUXV 12
> # define PR_SET_MM_EXE_FILE 13
> +# define PR_SET_MM_MAP 14
> +# define PR_SET_MM_MAP_SIZE 15
> +
> +/*
> + * This structure provides new memory descriptor
> + * map which mostly modifies /proc/pid/stat[m]
> + * output for a task. This mostly done in a
> + * sake of checkpoint/restore functionality.
> + */
> +struct prctl_mm_map {
> + __u64 start_code; /* code section bounds */
> + __u64 end_code;
> + __u64 start_data; /* data section bounds */
> + __u64 end_data;
> + __u64 start_brk; /* heap for brk() syscall */
> + __u64 brk;
> + __u64 start_stack; /* stack starts at */
> + __u64 arg_start; /* command line arguments bounds */
> + __u64 arg_end;
> + __u64 env_start; /* environment variables bounds */
> + __u64 env_end;
> + __u64 *auxv; /* auxiliary vector */
> + __u32 auxv_size; /* vector size */
> + __u32 exe_fd; /* /proc/$pid/exe link file */
> +};
>
> /*
> * Set specific pid that is allowed to ptrace the current task.
> Index: linux-2.6.git/kernel/sys.c
> ===================================================================
> --- linux-2.6.git.orig/kernel/sys.c
> +++ linux-2.6.git/kernel/sys.c
> @@ -1687,6 +1687,208 @@ exit:
> return err;
> }
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +/*
> + * WARNING: we don't require any capability here so be very careful
> + * in what is allowed for modification from userspace.
> + */
> +static int validate_prctl_map_locked(struct prctl_mm_map *prctl_map)
> +{
> + unsigned long mmap_max_addr = TASK_SIZE;
> + struct mm_struct *mm = current->mm;
> + struct vm_area_struct *stack_vma;
> + int error = 0;
> +
> + /*
> + * Make sure the members are not somewhere outside
> + * of allowed address space.
> + */
> +#define __prctl_check_addr_space(__member) \
> + ({ \
> + int __rc; \
> + if ((unsigned long)prctl_map->__member < mmap_max_addr && \
> + (unsigned long)prctl_map->__member >= mmap_min_addr) \
> + __rc = 0; \
> + else \
> + __rc = -EINVAL; \
> + __rc; \
> + })
> + error |= __prctl_check_addr_space(start_code);
> + error |= __prctl_check_addr_space(end_code);
> + error |= __prctl_check_addr_space(start_data);
> + error |= __prctl_check_addr_space(end_data);
> + error |= __prctl_check_addr_space(start_stack);
> + error |= __prctl_check_addr_space(start_brk);
> + error |= __prctl_check_addr_space(brk);
> + error |= __prctl_check_addr_space(arg_start);
> + error |= __prctl_check_addr_space(arg_end);
> + error |= __prctl_check_addr_space(env_start);
> + error |= __prctl_check_addr_space(env_end);
> + if (error)
> + goto out;
> +#undef __prctl_check_addr_space
> +
> + /*
> + * Stack, brk, command line arguments and environment must exist.
> + */
> + stack_vma = find_vma(mm, (unsigned long)prctl_map->start_stack);
> + if (!stack_vma) {
> + error = -EINVAL;
> + goto out;
> + }
> +#define __prctl_check_vma(__member) \
> + find_vma(mm, (unsigned long)prctl_map->__member) ? 0 : -EINVAL
> + error |= __prctl_check_vma(start_brk);
> + error |= __prctl_check_vma(brk);
> + error |= __prctl_check_vma(arg_start);
> + error |= __prctl_check_vma(arg_end);
> + error |= __prctl_check_vma(env_start);
> + error |= __prctl_check_vma(env_end);
> + if (error)
> + goto out;
> +#undef __prctl_check_vma
> +
> + /*
> + * Make sure the pairs are ordered.
> + */
> +#define __prctl_check_order(__m1, __op, __m2) \
> + ((unsigned long)prctl_map->__m1 __op \
> + (unsigned long)prctl_map->__m2) ? 0 : -EINVAL
> + error |= __prctl_check_order(start_code, <, end_code);
> + error |= __prctl_check_order(start_data, <, end_data);
> + error |= __prctl_check_order(start_brk, <=, brk);
> + error |= __prctl_check_order(arg_start, <, arg_end);
> + error |= __prctl_check_order(env_start, <, env_end);
> + if (error)
> + goto out;
> +#undef __prctl_check_order
> +
> + error = -EINVAL;
> +
> + /*
> + * @brk should be after @end_data in traditional maps.
> + */
> + if (prctl_map->start_brk <= prctl_map->end_data ||
> + prctl_map->brk <= prctl_map->end_data)
> + goto out;
> +
> + /*
> + * Neither we should allow to override limits if they set.
> + */
> + if (may_adjust_brk(rlimit(RLIMIT_DATA), prctl_map->brk,
> + prctl_map->start_brk, prctl_map->end_data,
> + prctl_map->start_data))
> + goto out;
> +
> +#ifdef CONFIG_STACK_GROWSUP
> + if (may_adjust_brk(rlimit(RLIMIT_STACK),
> + stack_vma->vm_end,
> + prctl_map->start_stack, 0, 0))
> +#else
> + if (may_adjust_brk(rlimit(RLIMIT_STACK),
> + prctl_map->start_stack,
> + stack_vma->vm_start, 0, 0))
> +#endif
> + goto out;
Ah! Sorry, I missed this use of may_adjust_brk here. Perhaps rename
it, since we're not checking brk here, and pass the RLIMIT_* value to
the function, which can look it up itself? "check_vma_rlimit" ?
> +
> + /*
> + * Someone is trying to cheat the auxv vector.
> + */
> + if (prctl_map->auxv_size) {
> + if (!prctl_map->auxv ||
> + prctl_map->auxv_size > sizeof(mm->saved_auxv))
> + goto out;
> + }
> +
> + /*
> + * Finally, make sure the caller has the rights to
> + * change /proc/pid/exe link: only local root should
> + * be allowed to.
> + */
> + if (prctl_map->exe_fd != (u32)-1) {
> + struct user_namespace *ns = current_user_ns();
> + const struct cred *cred = current_cred();
> +
> + if (!uid_eq(cred->uid, make_kuid(ns, 0)) ||
> + !gid_eq(cred->gid, make_kgid(ns, 0)))
> + goto out;
> + }
I got tricked for a moment here. :) I see that even if we pass this
check, prctl_set_mm_exe_file will still do the additional checks too
during prctl_set_mm_map. Excellent!
> +
> + error = 0;
> +out:
> + return error;
> +}
> +
> +static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data_size)
> +{
> + struct prctl_mm_map prctl_map = { .exe_fd = (u32)-1, };
> + unsigned long user_auxv[AT_VECTOR_SIZE];
> + struct mm_struct *mm = current->mm;
> + int error = -EINVAL;
> +
> + BUILD_BUG_ON(sizeof(user_auxv) != sizeof(mm->saved_auxv));
> +
> + if (opt == PR_SET_MM_MAP_SIZE)
> + return put_user((unsigned int)sizeof(prctl_map),
> + (unsigned int __user *)addr);
> +
> + if (data_size != sizeof(prctl_map))
> + return -EINVAL;
> +
> + if (copy_from_user(&prctl_map, addr, sizeof(prctl_map)))
> + return -EFAULT;
> +
> + down_read(&mm->mmap_sem);
> +
> + if (validate_prctl_map_locked(&prctl_map))
> + goto out;
> +
> + if (prctl_map.auxv_size) {
> + up_read(&mm->mmap_sem);
> + memset(user_auxv, 0, sizeof(user_auxv));
> + error = copy_from_user(user_auxv,
> + (const void __user *)prctl_map.auxv,
> + prctl_map.auxv_size);
> + down_read(&mm->mmap_sem);
> + if (error)
> + goto out;
> + }
> +
> + if (prctl_map.exe_fd != (u32)-1) {
> + error = prctl_set_mm_exe_file_locked(mm, prctl_map.exe_fd);
> + if (error)
> + goto out;
> + }
> +
> + if (prctl_map.auxv_size) {
> + /* Last entry always must be AT_NULL */
> + user_auxv[AT_VECTOR_SIZE - 2] = AT_NULL;
> + user_auxv[AT_VECTOR_SIZE - 1] = AT_NULL;
> +
> + task_lock(current);
> + memcpy(mm->saved_auxv, user_auxv, sizeof(user_auxv));
> + task_unlock(current);
> + }
> +
> + mm->start_code = prctl_map.start_code;
> + mm->end_code = prctl_map.end_code;
> + mm->start_data = prctl_map.start_data;
> + mm->end_data = prctl_map.end_data;
> + mm->start_brk = prctl_map.start_brk;
> + mm->brk = prctl_map.brk;
> + mm->start_stack = prctl_map.start_stack;
> + mm->arg_start = prctl_map.arg_start;
> + mm->arg_end = prctl_map.arg_end;
> + mm->env_start = prctl_map.env_start;
> + mm->env_end = prctl_map.env_end;
> +
> + error = 0;
> +out:
> + up_read(&mm->mmap_sem);
> + return error;
> +}
> +#endif /* CONFIG_CHECKPOINT_RESTORE */
> +
> static int prctl_set_mm(int opt, unsigned long addr,
> unsigned long arg4, unsigned long arg5)
> {
> @@ -1695,9 +1897,16 @@ static int prctl_set_mm(int opt, unsigne
> struct vm_area_struct *vma;
> int error;
>
> - if (arg5 || (arg4 && opt != PR_SET_MM_AUXV))
> + if (arg5 || (arg4 && (opt != PR_SET_MM_AUXV &&
> + opt != PR_SET_MM_MAP &&
> + opt != PR_SET_MM_MAP_SIZE)))
> return -EINVAL;
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> + if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
> + return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
> +#endif
> +
> if (!capable(CAP_SYS_RESOURCE))
> return -EPERM;
>
>
I think this is looking good. Thanks for the refactoring!
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 1/4] mm: Introduce may_adjust_brk helper
2014-07-24 16:46 ` Cyrill Gorcunov
@ 2014-07-24 19:32 ` Serge Hallyn
-1 siblings, 0 replies; 22+ messages in thread
From: Serge Hallyn @ 2014-07-24 19:32 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: linux-kernel, linux-mm, keescook, tj, akpm, avagin, ebiederm,
hpa, serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages,
jln
Quoting Cyrill Gorcunov (gorcunov@openvz.org):
> To eliminate code duplication lets introduce may_adjust_brk
> helper which we will use in brk() and prctl() syscalls.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andrew Vagin <avagin@openvz.org>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Serge Hallyn <serge.hallyn@canonical.com>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Vasiliy Kulikov <segoon@openwall.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Cc: Julien Tinnes <jln@google.com>
> ---
> include/linux/mm.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> Index: linux-2.6.git/include/linux/mm.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/mm.h
> +++ linux-2.6.git/include/linux/mm.h
> @@ -18,6 +18,7 @@
> #include <linux/pfn.h>
> #include <linux/bit_spinlock.h>
> #include <linux/shrinker.h>
> +#include <linux/resource.h>
>
> struct mempolicy;
> struct anon_vma;
> @@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
> bool *need_rmap_locks);
> extern void exit_mmap(struct mm_struct *);
>
> +static inline int may_adjust_brk(unsigned long rlim,
> + unsigned long new_brk,
> + unsigned long start_brk,
> + unsigned long end_data,
> + unsigned long start_data)
> +{
> + if (rlim < RLIMIT_DATA) {
In the code you're replacing, this was RLIM_INFINITY. Did you really
mean for this to be RLIMIT_DATA, aka 2?
> + if (((new_brk - start_brk) + (end_data - start_data)) > rlim)
> + return -ENOSPC;
> + }
> + return 0;
> +}
> +
> extern int mm_take_all_locks(struct mm_struct *mm);
> extern void mm_drop_all_locks(struct mm_struct *mm);
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 1/4] mm: Introduce may_adjust_brk helper
@ 2014-07-24 19:32 ` Serge Hallyn
0 siblings, 0 replies; 22+ messages in thread
From: Serge Hallyn @ 2014-07-24 19:32 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: linux-kernel, linux-mm, keescook, tj, akpm, avagin, ebiederm,
hpa, serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages,
jln
Quoting Cyrill Gorcunov (gorcunov@openvz.org):
> To eliminate code duplication lets introduce may_adjust_brk
> helper which we will use in brk() and prctl() syscalls.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andrew Vagin <avagin@openvz.org>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Serge Hallyn <serge.hallyn@canonical.com>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Vasiliy Kulikov <segoon@openwall.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Cc: Julien Tinnes <jln@google.com>
> ---
> include/linux/mm.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> Index: linux-2.6.git/include/linux/mm.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/mm.h
> +++ linux-2.6.git/include/linux/mm.h
> @@ -18,6 +18,7 @@
> #include <linux/pfn.h>
> #include <linux/bit_spinlock.h>
> #include <linux/shrinker.h>
> +#include <linux/resource.h>
>
> struct mempolicy;
> struct anon_vma;
> @@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
> bool *need_rmap_locks);
> extern void exit_mmap(struct mm_struct *);
>
> +static inline int may_adjust_brk(unsigned long rlim,
> + unsigned long new_brk,
> + unsigned long start_brk,
> + unsigned long end_data,
> + unsigned long start_data)
> +{
> + if (rlim < RLIMIT_DATA) {
In the code you're replacing, this was RLIM_INFINITY. Did you really
mean for this to be RLIMIT_DATA, aka 2?
> + if (((new_brk - start_brk) + (end_data - start_data)) > rlim)
> + return -ENOSPC;
> + }
> + return 0;
> +}
> +
> extern int mm_take_all_locks(struct mm_struct *mm);
> extern void mm_drop_all_locks(struct mm_struct *mm);
>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 4/4] prctl: PR_SET_MM -- Introduce PR_SET_MM_MAP operation, v3
2014-07-24 19:31 ` Kees Cook
@ 2014-07-24 19:44 ` Cyrill Gorcunov
-1 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 19:44 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, linux-mm, Tejun Heo, Andrew Morton, Andrew Vagin,
Eric W. Biederman, H. Peter Anvin, Serge Hallyn, Pavel Emelyanov,
Vasiliy Kulikov, KAMEZAWA Hiroyuki, Michael Kerrisk-manpages,
Julien Tinnes
On Thu, Jul 24, 2014 at 12:31:54PM -0700, Kees Cook wrote:
...
> > +
> > +#ifdef CONFIG_STACK_GROWSUP
> > + if (may_adjust_brk(rlimit(RLIMIT_STACK),
> > + stack_vma->vm_end,
> > + prctl_map->start_stack, 0, 0))
> > +#else
> > + if (may_adjust_brk(rlimit(RLIMIT_STACK),
> > + prctl_map->start_stack,
> > + stack_vma->vm_start, 0, 0))
> > +#endif
> > + goto out;
>
> Ah! Sorry, I missed this use of may_adjust_brk here. Perhaps rename
> it, since we're not checking brk here, and pass the RLIMIT_* value to
> the function, which can look it up itself? "check_vma_rlimit" ?
Yeah, a name is a bit confusing, but I guess check_vma_rlimit() is not
much better ;-) What we do inside -- we test if a sum of two intervals
or arguments in this helper so that it won't care about the logical
context it been called from, but then realized that this would be a way
too much of unneeded complexity. So if noone else pop with better suggestion
on name i'll update it to check_vma_rlimit (because it's more general
in compare to may_adjust_brk :-).
> > +
> > + /*
> > + * Finally, make sure the caller has the rights to
> > + * change /proc/pid/exe link: only local root should
> > + * be allowed to.
> > + */
> > + if (prctl_map->exe_fd != (u32)-1) {
> > + struct user_namespace *ns = current_user_ns();
> > + const struct cred *cred = current_cred();
> > +
> > + if (!uid_eq(cred->uid, make_kuid(ns, 0)) ||
> > + !gid_eq(cred->gid, make_kgid(ns, 0)))
> > + goto out;
> > + }
>
> I got tricked for a moment here. :) I see that even if we pass this
> check, prctl_set_mm_exe_file will still do the additional checks too
> during prctl_set_mm_map. Excellent!
Yeah.
> >
> > +#ifdef CONFIG_CHECKPOINT_RESTORE
> > + if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
> > + return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
> > +#endif
> > +
> > if (!capable(CAP_SYS_RESOURCE))
> > return -EPERM;
> >
> >
>
> I think this is looking good. Thanks for the refactoring!
Thanks a huge for comments!!!
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 4/4] prctl: PR_SET_MM -- Introduce PR_SET_MM_MAP operation, v3
@ 2014-07-24 19:44 ` Cyrill Gorcunov
0 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 19:44 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, linux-mm, Tejun Heo, Andrew Morton, Andrew Vagin,
Eric W. Biederman, H. Peter Anvin, Serge Hallyn, Pavel Emelyanov,
Vasiliy Kulikov, KAMEZAWA Hiroyuki, Michael Kerrisk-manpages,
Julien Tinnes
On Thu, Jul 24, 2014 at 12:31:54PM -0700, Kees Cook wrote:
...
> > +
> > +#ifdef CONFIG_STACK_GROWSUP
> > + if (may_adjust_brk(rlimit(RLIMIT_STACK),
> > + stack_vma->vm_end,
> > + prctl_map->start_stack, 0, 0))
> > +#else
> > + if (may_adjust_brk(rlimit(RLIMIT_STACK),
> > + prctl_map->start_stack,
> > + stack_vma->vm_start, 0, 0))
> > +#endif
> > + goto out;
>
> Ah! Sorry, I missed this use of may_adjust_brk here. Perhaps rename
> it, since we're not checking brk here, and pass the RLIMIT_* value to
> the function, which can look it up itself? "check_vma_rlimit" ?
Yeah, a name is a bit confusing, but I guess check_vma_rlimit() is not
much better ;-) What we do inside -- we test if a sum of two intervals
or arguments in this helper so that it won't care about the logical
context it been called from, but then realized that this would be a way
too much of unneeded complexity. So if noone else pop with better suggestion
on name i'll update it to check_vma_rlimit (because it's more general
in compare to may_adjust_brk :-).
> > +
> > + /*
> > + * Finally, make sure the caller has the rights to
> > + * change /proc/pid/exe link: only local root should
> > + * be allowed to.
> > + */
> > + if (prctl_map->exe_fd != (u32)-1) {
> > + struct user_namespace *ns = current_user_ns();
> > + const struct cred *cred = current_cred();
> > +
> > + if (!uid_eq(cred->uid, make_kuid(ns, 0)) ||
> > + !gid_eq(cred->gid, make_kgid(ns, 0)))
> > + goto out;
> > + }
>
> I got tricked for a moment here. :) I see that even if we pass this
> check, prctl_set_mm_exe_file will still do the additional checks too
> during prctl_set_mm_map. Excellent!
Yeah.
> >
> > +#ifdef CONFIG_CHECKPOINT_RESTORE
> > + if (opt == PR_SET_MM_MAP || opt == PR_SET_MM_MAP_SIZE)
> > + return prctl_set_mm_map(opt, (const void __user *)addr, arg4);
> > +#endif
> > +
> > if (!capable(CAP_SYS_RESOURCE))
> > return -EPERM;
> >
> >
>
> I think this is looking good. Thanks for the refactoring!
Thanks a huge for comments!!!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 1/4] mm: Introduce may_adjust_brk helper
2014-07-24 19:32 ` Serge Hallyn
@ 2014-07-24 19:46 ` Cyrill Gorcunov
-1 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 19:46 UTC (permalink / raw)
To: Serge Hallyn
Cc: linux-kernel, linux-mm, keescook, tj, akpm, avagin, ebiederm,
hpa, serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages,
jln
On Thu, Jul 24, 2014 at 07:32:25PM +0000, Serge Hallyn wrote:
> Quoting Cyrill Gorcunov (gorcunov@openvz.org):
> > To eliminate code duplication lets introduce may_adjust_brk
> > helper which we will use in brk() and prctl() syscalls.
> >
> > Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Tejun Heo <tj@kernel.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Andrew Vagin <avagin@openvz.org>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Cc: Serge Hallyn <serge.hallyn@canonical.com>
> > Cc: Pavel Emelyanov <xemul@parallels.com>
> > Cc: Vasiliy Kulikov <segoon@openwall.com>
> > Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> > Cc: Julien Tinnes <jln@google.com>
> > ---
> > include/linux/mm.h | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > Index: linux-2.6.git/include/linux/mm.h
> > ===================================================================
> > --- linux-2.6.git.orig/include/linux/mm.h
> > +++ linux-2.6.git/include/linux/mm.h
> > @@ -18,6 +18,7 @@
> > #include <linux/pfn.h>
> > #include <linux/bit_spinlock.h>
> > #include <linux/shrinker.h>
> > +#include <linux/resource.h>
> >
> > struct mempolicy;
> > struct anon_vma;
> > @@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
> > bool *need_rmap_locks);
> > extern void exit_mmap(struct mm_struct *);
> >
> > +static inline int may_adjust_brk(unsigned long rlim,
> > + unsigned long new_brk,
> > + unsigned long start_brk,
> > + unsigned long end_data,
> > + unsigned long start_data)
> > +{
> > + if (rlim < RLIMIT_DATA) {
>
> In the code you're replacing, this was RLIM_INFINITY. Did you really
> mean for this to be RLIMIT_DATA, aka 2?
Good catch, thanks Serge! Better would be to pass the type of resource
(as Kees suggested) here instead of @rlim itself and sure to compare
with RLIM_INFINITY.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [rfc 1/4] mm: Introduce may_adjust_brk helper
@ 2014-07-24 19:46 ` Cyrill Gorcunov
0 siblings, 0 replies; 22+ messages in thread
From: Cyrill Gorcunov @ 2014-07-24 19:46 UTC (permalink / raw)
To: Serge Hallyn
Cc: linux-kernel, linux-mm, keescook, tj, akpm, avagin, ebiederm,
hpa, serge.hallyn, xemul, segoon, kamezawa.hiroyu, mtk.manpages,
jln
On Thu, Jul 24, 2014 at 07:32:25PM +0000, Serge Hallyn wrote:
> Quoting Cyrill Gorcunov (gorcunov@openvz.org):
> > To eliminate code duplication lets introduce may_adjust_brk
> > helper which we will use in brk() and prctl() syscalls.
> >
> > Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Tejun Heo <tj@kernel.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Andrew Vagin <avagin@openvz.org>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Cc: Serge Hallyn <serge.hallyn@canonical.com>
> > Cc: Pavel Emelyanov <xemul@parallels.com>
> > Cc: Vasiliy Kulikov <segoon@openwall.com>
> > Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> > Cc: Julien Tinnes <jln@google.com>
> > ---
> > include/linux/mm.h | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > Index: linux-2.6.git/include/linux/mm.h
> > ===================================================================
> > --- linux-2.6.git.orig/include/linux/mm.h
> > +++ linux-2.6.git/include/linux/mm.h
> > @@ -18,6 +18,7 @@
> > #include <linux/pfn.h>
> > #include <linux/bit_spinlock.h>
> > #include <linux/shrinker.h>
> > +#include <linux/resource.h>
> >
> > struct mempolicy;
> > struct anon_vma;
> > @@ -1780,6 +1781,19 @@ extern struct vm_area_struct *copy_vma(s
> > bool *need_rmap_locks);
> > extern void exit_mmap(struct mm_struct *);
> >
> > +static inline int may_adjust_brk(unsigned long rlim,
> > + unsigned long new_brk,
> > + unsigned long start_brk,
> > + unsigned long end_data,
> > + unsigned long start_data)
> > +{
> > + if (rlim < RLIMIT_DATA) {
>
> In the code you're replacing, this was RLIM_INFINITY. Did you really
> mean for this to be RLIMIT_DATA, aka 2?
Good catch, thanks Serge! Better would be to pass the type of resource
(as Kees suggested) here instead of @rlim itself and sure to compare
with RLIM_INFINITY.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2014-07-24 19:46 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-24 16:46 [rfc 0/4] prctl: set-mm -- Rework interface, v2 Cyrill Gorcunov
2014-07-24 16:46 ` Cyrill Gorcunov
2014-07-24 16:46 ` [rfc 1/4] mm: Introduce may_adjust_brk helper Cyrill Gorcunov
2014-07-24 16:46 ` Cyrill Gorcunov
2014-07-24 19:18 ` Kees Cook
2014-07-24 19:18 ` Kees Cook
2014-07-24 19:21 ` Cyrill Gorcunov
2014-07-24 19:21 ` Cyrill Gorcunov
2014-07-24 19:32 ` Serge Hallyn
2014-07-24 19:32 ` Serge Hallyn
2014-07-24 19:46 ` Cyrill Gorcunov
2014-07-24 19:46 ` Cyrill Gorcunov
2014-07-24 16:46 ` [rfc 2/4] mm: Use " Cyrill Gorcunov
2014-07-24 16:46 ` Cyrill Gorcunov
2014-07-24 16:47 ` [rfc 3/4] prctl: PR_SET_MM -- Factor out mmap_sem when update mm::exe_file Cyrill Gorcunov
2014-07-24 16:47 ` Cyrill Gorcunov
2014-07-24 16:47 ` [rfc 4/4] prctl: PR_SET_MM -- Introduce PR_SET_MM_MAP operation, v3 Cyrill Gorcunov
2014-07-24 16:47 ` Cyrill Gorcunov
2014-07-24 19:31 ` Kees Cook
2014-07-24 19:31 ` Kees Cook
2014-07-24 19:44 ` Cyrill Gorcunov
2014-07-24 19:44 ` Cyrill Gorcunov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.