From 1d1dbf8135ab2f3603cc72e39e0f68784f453c39 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Sun, 6 Mar 2011 18:02:21 +0100 Subject: exec: introduce get_user_arg_ptr() helper Introduce get_user_arg_ptr() helper, convert count() and copy_strings() to use it. No functional changes, preparation. This helper is trivial, it just reads the pointer from argv/envp user-space array. Signed-off-by: Oleg Nesterov Reviewed-by: KOSAKI Motohiro Tested-by: KOSAKI Motohiro --- fs/exec.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) (limited to 'fs/exec.c') diff --git a/fs/exec.c b/fs/exec.c index 5e62d26..b12e24f 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -398,6 +398,17 @@ err: return err; } +static const char __user * +get_user_arg_ptr(const char __user * const __user *argv, int nr) +{ + const char __user *ptr; + + if (get_user(ptr, argv + nr)) + return ERR_PTR(-EFAULT); + + return ptr; +} + /* * count() counts the number of strings in array ARGV. */ @@ -407,13 +418,14 @@ static int count(const char __user * const __user * argv, int max) if (argv != NULL) { for (;;) { - const char __user * p; + const char __user *p = get_user_arg_ptr(argv, i); - if (get_user(p, argv)) - return -EFAULT; if (!p) break; - argv++; + + if (IS_ERR(p)) + return -EFAULT; + if (i++ >= max) return -E2BIG; @@ -443,16 +455,18 @@ static int copy_strings(int argc, const char __user *const __user *argv, int len; unsigned long pos; - if (get_user(str, argv+argc) || - !(len = strnlen_user(str, MAX_ARG_STRLEN))) { - ret = -EFAULT; + ret = -EFAULT; + str = get_user_arg_ptr(argv, argc); + if (IS_ERR(str)) goto out; - } - if (!valid_arg_len(bprm, len)) { - ret = -E2BIG; + len = strnlen_user(str, MAX_ARG_STRLEN); + if (!len) + goto out; + + ret = -E2BIG; + if (!valid_arg_len(bprm, len)) goto out; - } /* We're going to work our way backwords. */ pos = bprm->p; -- cgit v1.1 From ba2d01629d0d167598cfea85adc7926822bbfc45 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Sun, 6 Mar 2011 18:02:37 +0100 Subject: exec: introduce struct user_arg_ptr No functional changes, preparation. Introduce struct user_arg_ptr, change do_execve() paths to use it instead of "char __user * const __user *argv". This makes the argv/envp arguments opaque, we are ready to handle the compat case which needs argv pointing to compat_uptr_t. Suggested-by: Linus Torvalds Signed-off-by: Oleg Nesterov Reviewed-by: KOSAKI Motohiro Tested-by: KOSAKI Motohiro --- fs/exec.c | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) (limited to 'fs/exec.c') diff --git a/fs/exec.c b/fs/exec.c index b12e24f..526a039 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -398,12 +398,15 @@ err: return err; } -static const char __user * -get_user_arg_ptr(const char __user * const __user *argv, int nr) +struct user_arg_ptr { + const char __user *const __user *native; +}; + +static const char __user *get_user_arg_ptr(struct user_arg_ptr argv, int nr) { const char __user *ptr; - if (get_user(ptr, argv + nr)) + if (get_user(ptr, argv.native + nr)) return ERR_PTR(-EFAULT); return ptr; @@ -412,11 +415,11 @@ get_user_arg_ptr(const char __user * const __user *argv, int nr) /* * count() counts the number of strings in array ARGV. */ -static int count(const char __user * const __user * argv, int max) +static int count(struct user_arg_ptr argv, int max) { int i = 0; - if (argv != NULL) { + if (argv.native != NULL) { for (;;) { const char __user *p = get_user_arg_ptr(argv, i); @@ -442,7 +445,7 @@ static int count(const char __user * const __user * argv, int max) * processes's memory to the new process's stack. The call to get_user_pages() * ensures the destination page is created and not swapped out. */ -static int copy_strings(int argc, const char __user *const __user *argv, +static int copy_strings(int argc, struct user_arg_ptr argv, struct linux_binprm *bprm) { struct page *kmapped_page = NULL; @@ -533,14 +536,19 @@ out: /* * Like copy_strings, but get argv and its values from kernel memory. */ -int copy_strings_kernel(int argc, const char *const *argv, +int copy_strings_kernel(int argc, const char *const *__argv, struct linux_binprm *bprm) { int r; mm_segment_t oldfs = get_fs(); + struct user_arg_ptr argv = { + .native = (const char __user *const __user *)__argv, + }; + set_fs(KERNEL_DS); - r = copy_strings(argc, (const char __user *const __user *)argv, bprm); + r = copy_strings(argc, argv, bprm); set_fs(oldfs); + return r; } EXPORT_SYMBOL(copy_strings_kernel); @@ -1393,10 +1401,10 @@ EXPORT_SYMBOL(search_binary_handler); /* * sys_execve() executes a new program. */ -int do_execve(const char * filename, - const char __user *const __user *argv, - const char __user *const __user *envp, - struct pt_regs * regs) +static int do_execve_common(const char *filename, + struct user_arg_ptr argv, + struct user_arg_ptr envp, + struct pt_regs *regs) { struct linux_binprm *bprm; struct file *file; @@ -1503,6 +1511,16 @@ out_ret: return retval; } +int do_execve(const char *filename, + const char __user *const __user *__argv, + const char __user *const __user *__envp, + struct pt_regs *regs) +{ + struct user_arg_ptr argv = { .native = __argv }; + struct user_arg_ptr envp = { .native = __envp }; + return do_execve_common(filename, argv, envp, regs); +} + void set_binfmt(struct linux_binfmt *new) { struct mm_struct *mm = current->mm; -- cgit v1.1 From 0e028465d18b7c6797fcbdea632299d16097c5cd Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Sun, 6 Mar 2011 18:02:54 +0100 Subject: exec: unify do_execve/compat_do_execve code Add the appropriate members into struct user_arg_ptr and teach get_user_arg_ptr() to handle is_compat = T case correctly. This allows us to remove the compat_do_execve() code from fs/compat.c and reimplement compat_do_execve() as the trivial wrapper on top of do_execve_common(is_compat => true). In fact, this fixes another (minor) bug. "compat_uptr_t str" can overflow after "str += len" in compat_copy_strings() if a 64bit application execs via sys32_execve(). Unexport acct_arg_size() and get_arg_page(), fs/compat.c doesn't need them any longer. Signed-off-by: Oleg Nesterov Reviewed-by: KOSAKI Motohiro Tested-by: KOSAKI Motohiro --- fs/exec.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 12 deletions(-) (limited to 'fs/exec.c') diff --git a/fs/exec.c b/fs/exec.c index 526a039..89d788c 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -55,6 +55,7 @@ #include #include #include +#include #include #include @@ -167,7 +168,7 @@ out: #ifdef CONFIG_MMU -void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) +static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) { struct mm_struct *mm = current->mm; long diff = (long)(pages - bprm->vma_pages); @@ -186,7 +187,7 @@ void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) #endif } -struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, +static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, int write) { struct page *page; @@ -305,11 +306,11 @@ static bool valid_arg_len(struct linux_binprm *bprm, long len) #else -void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) +static inline void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) { } -struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, +static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, int write) { struct page *page; @@ -399,17 +400,36 @@ err: } struct user_arg_ptr { - const char __user *const __user *native; +#ifdef CONFIG_COMPAT + bool is_compat; +#endif + union { + const char __user *const __user *native; +#ifdef CONFIG_COMPAT + compat_uptr_t __user *compat; +#endif + } ptr; }; static const char __user *get_user_arg_ptr(struct user_arg_ptr argv, int nr) { - const char __user *ptr; + const char __user *native; + +#ifdef CONFIG_COMPAT + if (unlikely(argv.is_compat)) { + compat_uptr_t compat; + + if (get_user(compat, argv.ptr.compat + nr)) + return ERR_PTR(-EFAULT); - if (get_user(ptr, argv.native + nr)) + return compat_ptr(compat); + } +#endif + + if (get_user(native, argv.ptr.native + nr)) return ERR_PTR(-EFAULT); - return ptr; + return native; } /* @@ -419,7 +439,7 @@ static int count(struct user_arg_ptr argv, int max) { int i = 0; - if (argv.native != NULL) { + if (argv.ptr.native != NULL) { for (;;) { const char __user *p = get_user_arg_ptr(argv, i); @@ -542,7 +562,7 @@ int copy_strings_kernel(int argc, const char *const *__argv, int r; mm_segment_t oldfs = get_fs(); struct user_arg_ptr argv = { - .native = (const char __user *const __user *)__argv, + .ptr.native = (const char __user *const __user *)__argv, }; set_fs(KERNEL_DS); @@ -1516,10 +1536,28 @@ int do_execve(const char *filename, const char __user *const __user *__envp, struct pt_regs *regs) { - struct user_arg_ptr argv = { .native = __argv }; - struct user_arg_ptr envp = { .native = __envp }; + struct user_arg_ptr argv = { .ptr.native = __argv }; + struct user_arg_ptr envp = { .ptr.native = __envp }; + return do_execve_common(filename, argv, envp, regs); +} + +#ifdef CONFIG_COMPAT +int compat_do_execve(char *filename, + compat_uptr_t __user *__argv, + compat_uptr_t __user *__envp, + struct pt_regs *regs) +{ + struct user_arg_ptr argv = { + .is_compat = true, + .ptr.compat = __argv, + }; + struct user_arg_ptr envp = { + .is_compat = true, + .ptr.compat = __envp, + }; return do_execve_common(filename, argv, envp, regs); } +#endif void set_binfmt(struct linux_binfmt *new) { -- cgit v1.1 From ae6b585eeb74670a2dec1fe4394bdfbdb9395cc2 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Sun, 6 Mar 2011 18:03:11 +0100 Subject: exec: document acct_arg_size() Add the comment to explain acct_arg_size(). Signed-off-by: Oleg Nesterov Reviewed-by: KOSAKI Motohiro --- fs/exec.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'fs/exec.c') diff --git a/fs/exec.c b/fs/exec.c index 89d788c..5cb53f0 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -167,7 +167,12 @@ out: } #ifdef CONFIG_MMU - +/* + * The nascent bprm->mm is not visible until exec_mmap() but it can + * use a lot of memory, account these pages in current->mm temporary + * for oom_badness()->get_mm_rss(). Once exec succeeds or fails, we + * change the counter back via acct_arg_size(0). + */ static void acct_arg_size(struct linux_binprm *bprm, unsigned long pages) { struct mm_struct *mm = current->mm; -- cgit v1.1