Skip to content

Commit

Permalink
Merge tag 'secureexec-v4.14-rc1' of git://git.kernel.org/pub/scm/linu…
Browse files Browse the repository at this point in the history
…x/kernel/git/kees/linux

Pull secureexec update from Kees Cook:
 "This series has the ultimate goal of providing a sane stack rlimit
  when running set*id processes.

  To do this, the bprm_secureexec LSM hook is collapsed into the
  bprm_set_creds hook so the secureexec-ness of an exec can be
  determined early enough to make decisions about rlimits and the
  resulting memory layouts. Other logic acting on the secureexec-ness of
  an exec is similarly consolidated. Capabilities needed some special
  handling, but the refactoring removed other special handling, so that
  was a wash"

* tag 'secureexec-v4.14-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux:
  exec: Consolidate pdeath_signal clearing
  exec: Use sane stack rlimit under secureexec
  exec: Consolidate dumpability logic
  smack: Remove redundant pdeath_signal clearing
  exec: Use secureexec for clearing pdeath_signal
  exec: Use secureexec for setting dumpability
  LSM: drop bprm_secureexec hook
  commoncap: Move cap_elevated calculation into bprm_set_creds
  commoncap: Refactor to remove bprm_secureexec hook
  smack: Refactor to remove bprm_secureexec hook
  selinux: Refactor to remove bprm_secureexec hook
  apparmor: Refactor to remove bprm_secureexec hook
  binfmt: Introduce secureexec flag
  exec: Correct comments about "point of no return"
  exec: Rename bprm->cred_prepared to called_set_creds
  • Loading branch information
torvalds committed Sep 8, 2017
2 parents 44ccba3 + fe8993b commit 828f425
Show file tree
Hide file tree
Showing 16 changed files with 91 additions and 159 deletions.
2 changes: 1 addition & 1 deletion fs/binfmt_elf.c
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
NEW_AUX_ENT(AT_EUID, from_kuid_munged(cred->user_ns, cred->euid));
NEW_AUX_ENT(AT_GID, from_kgid_munged(cred->user_ns, cred->gid));
NEW_AUX_ENT(AT_EGID, from_kgid_munged(cred->user_ns, cred->egid));
NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
NEW_AUX_ENT(AT_SECURE, bprm->secureexec);
NEW_AUX_ENT(AT_RANDOM, (elf_addr_t)(unsigned long)u_rand_bytes);
#ifdef ELF_HWCAP2
NEW_AUX_ENT(AT_HWCAP2, ELF_HWCAP2);
Expand Down
2 changes: 1 addition & 1 deletion fs/binfmt_elf_fdpic.c
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
NEW_AUX_ENT(AT_EUID, (elf_addr_t) from_kuid_munged(cred->user_ns, cred->euid));
NEW_AUX_ENT(AT_GID, (elf_addr_t) from_kgid_munged(cred->user_ns, cred->gid));
NEW_AUX_ENT(AT_EGID, (elf_addr_t) from_kgid_munged(cred->user_ns, cred->egid));
NEW_AUX_ENT(AT_SECURE, security_bprm_secureexec(bprm));
NEW_AUX_ENT(AT_SECURE, bprm->secureexec);
NEW_AUX_ENT(AT_EXECFN, bprm->exec);

#ifdef ARCH_DLINFO
Expand Down
2 changes: 1 addition & 1 deletion fs/binfmt_flat.c
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
* as we're past the point of no return and are dealing with shared
* libraries.
*/
bprm.cred_prepared = 1;
bprm.called_set_creds = 1;

res = prepare_binprm(&bprm);

Expand Down
56 changes: 41 additions & 15 deletions fs/exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -1259,6 +1259,12 @@ void __set_task_comm(struct task_struct *tsk, const char *buf, bool exec)
perf_event_comm(tsk, exec);
}

/*
* Calling this is the point of no return. None of the failures will be
* seen by userspace since either the process is already taking a fatal
* signal (via de_thread() or coredump), or will have SEGV raised
* (after exec_mmap()) by search_binary_handlers (see below).
*/
int flush_old_exec(struct linux_binprm * bprm)
{
int retval;
Expand Down Expand Up @@ -1286,7 +1292,13 @@ int flush_old_exec(struct linux_binprm * bprm)
if (retval)
goto out;

bprm->mm = NULL; /* We're using it now */
/*
* After clearing bprm->mm (to mark that current is using the
* prepared mm now), we have nothing left of the original
* process. If anything from here on returns an error, the check
* in search_binary_handler() will SEGV current.
*/
bprm->mm = NULL;

set_fs(USER_DS);
current->flags &= ~(PF_RANDOMIZE | PF_FORKNOEXEC | PF_KTHREAD |
Expand Down Expand Up @@ -1331,15 +1343,38 @@ EXPORT_SYMBOL(would_dump);

void setup_new_exec(struct linux_binprm * bprm)
{
/*
* Once here, prepare_binrpm() will not be called any more, so
* the final state of setuid/setgid/fscaps can be merged into the
* secureexec flag.
*/
bprm->secureexec |= bprm->cap_elevated;

if (bprm->secureexec) {
/* Make sure parent cannot signal privileged process. */
current->pdeath_signal = 0;

/*
* For secureexec, reset the stack limit to sane default to
* avoid bad behavior from the prior rlimits. This has to
* happen before arch_pick_mmap_layout(), which examines
* RLIMIT_STACK, but after the point of no return to avoid
* needing to clean up the change on failure.
*/
if (current->signal->rlim[RLIMIT_STACK].rlim_cur > _STK_LIM)
current->signal->rlim[RLIMIT_STACK].rlim_cur = _STK_LIM;
}

arch_pick_mmap_layout(current->mm);

/* This is the point of no return */
current->sas_ss_sp = current->sas_ss_size = 0;

if (uid_eq(current_euid(), current_uid()) && gid_eq(current_egid(), current_gid()))
set_dumpable(current->mm, SUID_DUMP_USER);
else
/* Figure out dumpability. */
if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
bprm->secureexec)
set_dumpable(current->mm, suid_dumpable);
else
set_dumpable(current->mm, SUID_DUMP_USER);

arch_setup_new_exec();
perf_event_exec();
Expand All @@ -1351,15 +1386,6 @@ void setup_new_exec(struct linux_binprm * bprm)
*/
current->mm->task_size = TASK_SIZE;

/* install the new credentials */
if (!uid_eq(bprm->cred->uid, current_euid()) ||
!gid_eq(bprm->cred->gid, current_egid())) {
current->pdeath_signal = 0;
} else {
if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP)
set_dumpable(current->mm, suid_dumpable);
}

/* An exec changes our domain. We are no longer part of the thread
group */
current->self_exec_id++;
Expand Down Expand Up @@ -1548,7 +1574,7 @@ int prepare_binprm(struct linux_binprm *bprm)
retval = security_bprm_set_creds(bprm);
if (retval)
return retval;
bprm->cred_prepared = 1;
bprm->called_set_creds = 1;

memset(bprm->buf, 0, BINPRM_BUF_SIZE);
return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
Expand Down
24 changes: 19 additions & 5 deletions include/linux/binfmts.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,25 @@ struct linux_binprm {
struct mm_struct *mm;
unsigned long p; /* current top of mem */
unsigned int
cred_prepared:1,/* true if creds already prepared (multiple
* preps happen for interpreters) */
cap_effective:1;/* true if has elevated effective capabilities,
* false if not; except for init which inherits
* its parent's caps anyway */
/*
* True after the bprm_set_creds hook has been called once
* (multiple calls can be made via prepare_binprm() for
* binfmt_script/misc).
*/
called_set_creds:1,
/*
* True if most recent call to the commoncaps bprm_set_creds
* hook (due to multiple prepare_binprm() calls from the
* binfmt_script/misc handlers) resulted in elevated
* privileges.
*/
cap_elevated:1,
/*
* Set by bprm_set_creds hook to indicate a privilege-gaining
* exec has happened. Used to sanitize execution environment
* and to set AT_SECURE auxv for glibc.
*/
secureexec:1;
#ifdef __alpha__
unsigned int taso:1;
#endif
Expand Down
14 changes: 5 additions & 9 deletions include/linux/lsm_hooks.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@
* interpreters. The hook can tell whether it has already been called by
* checking to see if @bprm->security is non-NULL. If so, then the hook
* may decide either to retain the security information saved earlier or
* to replace it.
* to replace it. The hook must set @bprm->secureexec to 1 if a "secure
* exec" has happened as a result of this hook call. The flag is used to
* indicate the need for a sanitized execution environment, and is also
* passed in the ELF auxiliary table on the initial stack to indicate
* whether libc should enable secure mode.
* @bprm contains the linux_binprm structure.
* Return 0 if the hook is successful and permission is granted.
* @bprm_check_security:
Expand Down Expand Up @@ -71,12 +75,6 @@
* linux_binprm structure. This hook is a good place to perform state
* changes on the process such as clearing out non-inheritable signal
* state. This is called immediately after commit_creds().
* @bprm_secureexec:
* Return a boolean value (0 or 1) indicating whether a "secure exec"
* is required. The flag is passed in the auxiliary table
* on the initial stack to the ELF interpreter to indicate whether libc
* should enable secure mode.
* @bprm contains the linux_binprm structure.
*
* Security hooks for filesystem operations.
*
Expand Down Expand Up @@ -1388,7 +1386,6 @@ union security_list_options {

int (*bprm_set_creds)(struct linux_binprm *bprm);
int (*bprm_check_security)(struct linux_binprm *bprm);
int (*bprm_secureexec)(struct linux_binprm *bprm);
void (*bprm_committing_creds)(struct linux_binprm *bprm);
void (*bprm_committed_creds)(struct linux_binprm *bprm);

Expand Down Expand Up @@ -1710,7 +1707,6 @@ struct security_hook_heads {
struct list_head vm_enough_memory;
struct list_head bprm_set_creds;
struct list_head bprm_check_security;
struct list_head bprm_secureexec;
struct list_head bprm_committing_creds;
struct list_head bprm_committed_creds;
struct list_head sb_alloc_security;
Expand Down
7 changes: 0 additions & 7 deletions include/linux/security.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ extern int cap_capset(struct cred *new, const struct cred *old,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
extern int cap_bprm_set_creds(struct linux_binprm *bprm);
extern int cap_bprm_secureexec(struct linux_binprm *bprm);
extern int cap_inode_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags);
extern int cap_inode_removexattr(struct dentry *dentry, const char *name);
Expand Down Expand Up @@ -232,7 +231,6 @@ int security_bprm_set_creds(struct linux_binprm *bprm);
int security_bprm_check(struct linux_binprm *bprm);
void security_bprm_committing_creds(struct linux_binprm *bprm);
void security_bprm_committed_creds(struct linux_binprm *bprm);
int security_bprm_secureexec(struct linux_binprm *bprm);
int security_sb_alloc(struct super_block *sb);
void security_sb_free(struct super_block *sb);
int security_sb_copy_data(char *orig, char *copy);
Expand Down Expand Up @@ -541,11 +539,6 @@ static inline void security_bprm_committed_creds(struct linux_binprm *bprm)
{
}

static inline int security_bprm_secureexec(struct linux_binprm *bprm)
{
return cap_bprm_secureexec(bprm);
}

static inline int security_sb_alloc(struct super_block *sb)
{
return 0;
Expand Down
21 changes: 2 additions & 19 deletions security/apparmor/domain.c
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
file_inode(bprm->file)->i_mode
};

if (bprm->cred_prepared)
if (bprm->called_set_creds)
return 0;

ctx = cred_ctx(bprm->cred);
Expand Down Expand Up @@ -807,7 +807,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
aa_label_printk(new, GFP_ATOMIC);
dbg_printk("\n");
}
bprm->unsafe |= AA_SECURE_X_NEEDED;
bprm->secureexec = 1;
}

if (label->proxy != new->proxy) {
Expand Down Expand Up @@ -843,23 +843,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
goto done;
}

/**
* apparmor_bprm_secureexec - determine if secureexec is needed
* @bprm: binprm for exec (NOT NULL)
*
* Returns: %1 if secureexec is needed else %0
*/
int apparmor_bprm_secureexec(struct linux_binprm *bprm)
{
/* the decision to use secure exec is computed in set_creds
* and stored in bprm->unsafe.
*/
if (bprm->unsafe & AA_SECURE_X_NEEDED)
return 1;

return 0;
}

/*
* Functions for self directed profile change
*/
Expand Down
1 change: 0 additions & 1 deletion security/apparmor/include/domain.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ struct aa_domain {
#define AA_CHANGE_STACK 8

int apparmor_bprm_set_creds(struct linux_binprm *bprm);
int apparmor_bprm_secureexec(struct linux_binprm *bprm);

void aa_free_domain_entries(struct aa_domain *domain);
int aa_change_hat(const char *hats[], int count, u64 token, int flags);
Expand Down
3 changes: 0 additions & 3 deletions security/apparmor/include/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,6 @@ static inline struct aa_label *aa_get_file_label(struct aa_file_ctx *ctx)
#define AA_X_INHERIT 0x4000
#define AA_X_UNCONFINED 0x8000

/* AA_SECURE_X_NEEDED - is passed in the bprm->unsafe field */
#define AA_SECURE_X_NEEDED 0x8000

/* need to make conditional which ones are being set */
struct path_cond {
kuid_t uid;
Expand Down
1 change: 0 additions & 1 deletion security/apparmor/lsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,6 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(bprm_set_creds, apparmor_bprm_set_creds),
LSM_HOOK_INIT(bprm_committing_creds, apparmor_bprm_committing_creds),
LSM_HOOK_INIT(bprm_committed_creds, apparmor_bprm_committed_creds),
LSM_HOOK_INIT(bprm_secureexec, apparmor_bprm_secureexec),

LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit),
};
Expand Down
50 changes: 11 additions & 39 deletions security/commoncap.c
Original file line number Diff line number Diff line change
Expand Up @@ -285,15 +285,6 @@ int cap_capset(struct cred *new,
return 0;
}

/*
* Clear proposed capability sets for execve().
*/
static inline void bprm_clear_caps(struct linux_binprm *bprm)
{
cap_clear(bprm->cred->cap_permitted);
bprm->cap_effective = false;
}

/**
* cap_inode_need_killpriv - Determine if inode change affects privileges
* @dentry: The inode/dentry in being changed with change marked ATTR_KILL_PRIV
Expand Down Expand Up @@ -443,7 +434,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_c
int rc = 0;
struct cpu_vfs_cap_data vcaps;

bprm_clear_caps(bprm);
cap_clear(bprm->cred->cap_permitted);

if (!file_caps_enabled)
return 0;
Expand Down Expand Up @@ -476,7 +467,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_c

out:
if (rc)
bprm_clear_caps(bprm);
cap_clear(bprm->cred->cap_permitted);

return rc;
}
Expand Down Expand Up @@ -585,8 +576,6 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
if (WARN_ON(!cap_ambient_invariant_ok(new)))
return -EPERM;

bprm->cap_effective = effective;

/*
* Audit candidate if current->cap_effective is set
*
Expand Down Expand Up @@ -614,33 +603,17 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
if (WARN_ON(!cap_ambient_invariant_ok(new)))
return -EPERM;

return 0;
}

/**
* cap_bprm_secureexec - Determine whether a secure execution is required
* @bprm: The execution parameters
*
* Determine whether a secure execution is required, return 1 if it is, and 0
* if it is not.
*
* The credentials have been committed by this point, and so are no longer
* available through @bprm->cred.
*/
int cap_bprm_secureexec(struct linux_binprm *bprm)
{
const struct cred *cred = current_cred();
kuid_t root_uid = make_kuid(cred->user_ns, 0);

if (!uid_eq(cred->uid, root_uid)) {
if (bprm->cap_effective)
return 1;
if (!cap_issubset(cred->cap_permitted, cred->cap_ambient))
return 1;
/* Check for privilege-elevated exec. */
bprm->cap_elevated = 0;
if (is_setid) {
bprm->cap_elevated = 1;
} else if (!uid_eq(new->uid, root_uid)) {
if (effective ||
!cap_issubset(new->cap_permitted, new->cap_ambient))
bprm->cap_elevated = 1;
}

return (!uid_eq(cred->euid, cred->uid) ||
!gid_eq(cred->egid, cred->gid));
return 0;
}

/**
Expand Down Expand Up @@ -1079,7 +1052,6 @@ struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(capget, cap_capget),
LSM_HOOK_INIT(capset, cap_capset),
LSM_HOOK_INIT(bprm_set_creds, cap_bprm_set_creds),
LSM_HOOK_INIT(bprm_secureexec, cap_bprm_secureexec),
LSM_HOOK_INIT(inode_need_killpriv, cap_inode_need_killpriv),
LSM_HOOK_INIT(inode_killpriv, cap_inode_killpriv),
LSM_HOOK_INIT(mmap_addr, cap_mmap_addr),
Expand Down
5 changes: 0 additions & 5 deletions security/security.c
Original file line number Diff line number Diff line change
Expand Up @@ -351,11 +351,6 @@ void security_bprm_committed_creds(struct linux_binprm *bprm)
call_void_hook(bprm_committed_creds, bprm);
}

int security_bprm_secureexec(struct linux_binprm *bprm)
{
return call_int_hook(bprm_secureexec, 0, bprm);
}

int security_sb_alloc(struct super_block *sb)
{
return call_int_hook(sb_alloc_security, 0, sb);
Expand Down
Loading

0 comments on commit 828f425

Please sign in to comment.