Skip to content

Commit

Permalink
exec: Implement kernel_execve
Browse files Browse the repository at this point in the history
To allow the kernel not to play games with set_fs to call exec
implement kernel_execve.  The function kernel_execve takes pointers
into kernel memory and copies the values pointed to onto the new
userspace stack.

The calls with arguments from kernel space of do_execve are replaced
with calls to kernel_execve.

The calls do_execve and do_execveat are made static as there are now
no callers outside of exec.

The comments that mention do_execve are updated to refer to
kernel_execve or execve depending on the circumstances.  In addition
to correcting the comments, this makes it easy to grep for do_execve
and verify it is not used.

Inspired-by: https://lkml.kernel.org/r/20200627072704.2447163-1-hch@lst.de
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lkml.kernel.org/r/87wo365ikj.fsf@x220.int.ebiederm.org
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
  • Loading branch information
Eric W. Biederman committed Jul 21, 2020
1 parent d8b9cd5 commit be619f7
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 23 deletions.
2 changes: 1 addition & 1 deletion arch/x86/entry/entry_32.S
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ SYM_CODE_START(ret_from_fork)
CALL_NOSPEC ebx
/*
* A kernel thread is allowed to return here after successfully
* calling do_execve(). Exit to userspace to complete the execve()
* calling kernel_execve(). Exit to userspace to complete the execve()
* syscall.
*/
movl $0, PT_EAX(%esp)
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/entry/entry_64.S
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ SYM_CODE_START(ret_from_fork)
CALL_NOSPEC rbx
/*
* A kernel thread is allowed to return here after successfully
* calling do_execve(). Exit to userspace to complete the execve()
* calling kernel_execve(). Exit to userspace to complete the execve()
* syscall.
*/
movq $0, RAX(%rsp)
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/kernel/unwind_frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ bool unwind_next_frame(struct unwind_state *state)
* This user_mode() check is slightly broader than a PF_KTHREAD
* check because it also catches the awkward situation where a
* newly forked kthread transitions into a user task by calling
* do_execve(), which eventually clears PF_KTHREAD.
* kernel_execve(), which eventually clears PF_KTHREAD.
*/
if (!user_mode(regs))
goto the_end;
Expand Down
88 changes: 86 additions & 2 deletions fs/exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,23 @@ static int count(struct user_arg_ptr argv, int max)
return i;
}

static int count_strings_kernel(const char *const *argv)
{
int i;

if (!argv)
return 0;

for (i = 0; argv[i]; ++i) {
if (i >= MAX_ARG_STRINGS)
return -E2BIG;
if (fatal_signal_pending(current))
return -ERESTARTNOHAND;
cond_resched();
}
return i;
}

static int bprm_stack_limits(struct linux_binprm *bprm)
{
unsigned long limit, ptr_size;
Expand Down Expand Up @@ -624,6 +641,20 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
}
EXPORT_SYMBOL(copy_string_kernel);

static int copy_strings_kernel(int argc, const char *const *argv,
struct linux_binprm *bprm)
{
while (argc-- > 0) {
int ret = copy_string_kernel(argv[argc], bprm);
if (ret < 0)
return ret;
if (fatal_signal_pending(current))
return -ERESTARTNOHAND;
cond_resched();
}
return 0;
}

#ifdef CONFIG_MMU

/*
Expand Down Expand Up @@ -1991,7 +2022,60 @@ static int do_execveat_common(int fd, struct filename *filename,
return retval;
}

int do_execve(struct filename *filename,
int kernel_execve(const char *kernel_filename,
const char *const *argv, const char *const *envp)
{
struct filename *filename;
struct linux_binprm *bprm;
int fd = AT_FDCWD;
int retval;

filename = getname_kernel(kernel_filename);
if (IS_ERR(filename))
return PTR_ERR(filename);

bprm = alloc_bprm(fd, filename);
if (IS_ERR(bprm)) {
retval = PTR_ERR(bprm);
goto out_ret;
}

retval = count_strings_kernel(argv);
if (retval < 0)
goto out_free;
bprm->argc = retval;

retval = count_strings_kernel(envp);
if (retval < 0)
goto out_free;
bprm->envc = retval;

retval = bprm_stack_limits(bprm);
if (retval < 0)
goto out_free;

retval = copy_string_kernel(bprm->filename, bprm);
if (retval < 0)
goto out_free;
bprm->exec = bprm->p;

retval = copy_strings_kernel(bprm->envc, envp, bprm);
if (retval < 0)
goto out_free;

retval = copy_strings_kernel(bprm->argc, argv, bprm);
if (retval < 0)
goto out_free;

retval = bprm_execve(bprm, fd, filename, 0);
out_free:
free_bprm(bprm);
out_ret:
putname(filename);
return retval;
}

static int do_execve(struct filename *filename,
const char __user *const __user *__argv,
const char __user *const __user *__envp)
{
Expand All @@ -2000,7 +2084,7 @@ int do_execve(struct filename *filename,
return do_execveat_common(AT_FDCWD, filename, argv, envp, 0);
}

int do_execveat(int fd, struct filename *filename,
static int do_execveat(int fd, struct filename *filename,
const char __user *const __user *__argv,
const char __user *const __user *__envp,
int flags)
Expand Down
9 changes: 2 additions & 7 deletions include/linux/binfmts.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,7 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm);
extern void set_binfmt(struct linux_binfmt *new);
extern ssize_t read_code(struct file *, unsigned long, loff_t, size_t);

extern int do_execve(struct filename *,
const char __user * const __user *,
const char __user * const __user *);
extern int do_execveat(int, struct filename *,
const char __user * const __user *,
const char __user * const __user *,
int);
int kernel_execve(const char *filename,
const char *const *argv, const char *const *envp);

#endif /* _LINUX_BINFMTS_H */
4 changes: 1 addition & 3 deletions init/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1329,9 +1329,7 @@ static int run_init_process(const char *init_filename)
pr_debug(" with environment:\n");
for (p = envp_init; *p; p++)
pr_debug(" %s\n", *p);
return do_execve(getname_kernel(init_filename),
(const char __user *const __user *)argv_init,
(const char __user *const __user *)envp_init);
return kernel_execve(init_filename, argv_init, envp_init);
}

static int try_to_run_init_process(const char *init_filename)
Expand Down
6 changes: 3 additions & 3 deletions kernel/umh.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ static int call_usermodehelper_exec_async(void *data)

commit_creds(new);

retval = do_execve(getname_kernel(sub_info->path),
(const char __user *const __user *)sub_info->argv,
(const char __user *const __user *)sub_info->envp);
retval = kernel_execve(sub_info->path,
(const char *const *)sub_info->argv,
(const char *const *)sub_info->envp);
out:
sub_info->retval = retval;
/*
Expand Down
2 changes: 1 addition & 1 deletion security/tomoyo/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ struct tomoyo_request_info {
struct tomoyo_obj_info *obj;
/*
* For holding parameters specific to execve() request.
* NULL if not dealing do_execve().
* NULL if not dealing execve().
*/
struct tomoyo_execve *ee;
struct tomoyo_domain_info *domain;
Expand Down
4 changes: 2 additions & 2 deletions security/tomoyo/domain.c
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm)

/*
* Check for domain transition preference if "file execute" matched.
* If preference is given, make do_execve() fail if domain transition
* If preference is given, make execve() fail if domain transition
* has failed, for domain transition preference should be used with
* destination domain defined.
*/
Expand Down Expand Up @@ -810,7 +810,7 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm)
snprintf(ee->tmp, TOMOYO_EXEC_TMPSIZE - 1, "<%s>",
candidate->name);
/*
* Make do_execve() fail if domain transition across namespaces
* Make execve() fail if domain transition across namespaces
* has failed.
*/
reject_on_transition_failure = true;
Expand Down
4 changes: 2 additions & 2 deletions security/tomoyo/tomoyo.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ static int tomoyo_bprm_check_security(struct linux_binprm *bprm)
struct tomoyo_task *s = tomoyo_task(current);

/*
* Execute permission is checked against pathname passed to do_execve()
* Execute permission is checked against pathname passed to execve()
* using current domain.
*/
if (!s->old_domain_info) {
Expand Down Expand Up @@ -307,7 +307,7 @@ static int tomoyo_file_fcntl(struct file *file, unsigned int cmd,
*/
static int tomoyo_file_open(struct file *f)
{
/* Don't check read permission here if called from do_execve(). */
/* Don't check read permission here if called from execve(). */
if (current->in_execve)
return 0;
return tomoyo_check_open_permission(tomoyo_domain(), &f->f_path,
Expand Down

0 comments on commit be619f7

Please sign in to comment.