[PATCH 2/2] exec: Compute file based creds only once
Eric W. Biederman
ebiederm at xmission.com
Fri May 29 16:47:29 UTC 2020
Move the computation of creds from prepare_binfmt into begin_new_exec
so that the creds need only be computed once. This is just code
reorganization no semantic changes of any kind are made.
Moving the computation is safe. I have looked through the kernel and
verified none of the binfmts look at bprm->cred directly, and that
there are no helpers that look at bprm->cred indirectly. Which means
that it is not a problem to compute the bprm->cred later in the
execution flow as it is not used until it becomes current->cred.
A new function bprm_creds_from_file is added to contain the work that
needs to be done. bprm_creds_from_file first computes which file
bprm->executable or most likely bprm->file that the bprm->creds
will be computed from.
The funciton bprm_fill_uid is updated to receive the file instead of
accessing bprm->file. The now unnecessary work needed to reset the
bprm->cred->euid, and bprm->cred->egid is removed from brpm_fill_uid.
A small comment to document that bprm_fill_uid now only deals with the
work to handle suid and sgid files. The default case is already
heandled by prepare_exec_creds.
The function security_bprm_repopulate_creds is renamed
security_bprm_creds_from_file and now is explicitly passed the file
from which to compute the creds. The documentation of the
bprm_creds_from_file security hook is updated to explain when the hook
is called and what it needs to do. The file is passed from
cap_bprm_creds_from_file into get_file_caps so that the caps are
computed for the appropriate file. The now unnecessary work in
cap_bprm_creds_from_file to reset the ambient capabilites has been
removed. A small comment to document that the work of
cap_bprm_creds_from_file is to read capabilities from the files
secureity attribute and derive capabilities from the fact the
user had uid 0 has been added.
Signed-off-by: "Eric W. Biederman" <ebiederm at xmission.com>
---
fs/binfmt_misc.c | 2 +-
fs/exec.c | 63 +++++++++++++++--------------------
include/linux/binfmts.h | 14 ++------
include/linux/lsm_hook_defs.h | 2 +-
include/linux/lsm_hooks.h | 22 ++++++------
include/linux/security.h | 9 ++---
security/commoncap.c | 24 +++++++------
security/security.c | 4 +--
8 files changed, 61 insertions(+), 79 deletions(-)
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 53968ea07b57..bc5506619b7e 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -192,7 +192,7 @@ static int load_misc_binary(struct linux_binprm *bprm)
bprm->interpreter = interp_file;
if (fmt->flags & MISC_FMT_CREDENTIALS)
- bprm->preserve_creds = 1;
+ bprm->execfd_creds = 1;
retval = 0;
ret:
diff --git a/fs/exec.c b/fs/exec.c
index 0f793536e393..e8599236290d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -72,6 +72,8 @@
#include <trace/events/sched.h>
+static int bprm_creds_from_file(struct linux_binprm *bprm);
+
int suid_dumpable = 0;
static LIST_HEAD(formats);
@@ -1304,6 +1306,11 @@ int begin_new_exec(struct linux_binprm * bprm)
struct task_struct *me = current;
int retval;
+ /* Once we are committed compute the creds */
+ retval = bprm_creds_from_file(bprm);
+ if (retval)
+ return retval;
+
/*
* Ensure all future errors are fatal.
*/
@@ -1354,7 +1361,6 @@ int begin_new_exec(struct linux_binprm * bprm)
me->flags &= ~(PF_RANDOMIZE | PF_FORKNOEXEC | PF_KTHREAD |
PF_NOFREEZE | PF_NO_SETAFFINITY);
flush_thread();
- bprm->per_clear |= bprm->pf_per_clear;
me->personality &= ~bprm->per_clear;
/*
@@ -1365,13 +1371,6 @@ int begin_new_exec(struct linux_binprm * bprm)
*/
do_close_on_exec(me->files);
- /*
- * 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->active_secureexec;
-
if (bprm->secureexec) {
/* Make sure parent cannot signal privileged process. */
me->pdeath_signal = 0;
@@ -1587,29 +1586,21 @@ static void check_unsafe_exec(struct linux_binprm *bprm)
spin_unlock(&p->fs->lock);
}
-static void bprm_fill_uid(struct linux_binprm *bprm)
+static void bprm_fill_uid(struct linux_binprm *bprm, struct file *file)
{
+ /* Handle suid and sgid on files */
struct inode *inode;
unsigned int mode;
kuid_t uid;
kgid_t gid;
- /*
- * Since this can be called multiple times (via prepare_binprm),
- * we must clear any previous work done when setting set[ug]id
- * bits from any earlier bprm->file uses (for example when run
- * first for a setuid script then again for its interpreter).
- */
- bprm->cred->euid = current_euid();
- bprm->cred->egid = current_egid();
-
- if (!mnt_may_suid(bprm->file->f_path.mnt))
+ if (!mnt_may_suid(file->f_path.mnt))
return;
if (task_no_new_privs(current))
return;
- inode = bprm->file->f_path.dentry->d_inode;
+ inode = file->f_path.dentry->d_inode;
mode = READ_ONCE(inode->i_mode);
if (!(mode & (S_ISUID|S_ISGID)))
return;
@@ -1629,19 +1620,31 @@ static void bprm_fill_uid(struct linux_binprm *bprm)
return;
if (mode & S_ISUID) {
- bprm->pf_per_clear |= PER_CLEAR_ON_SETID;
+ bprm->per_clear |= PER_CLEAR_ON_SETID;
bprm->cred->euid = uid;
}
if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
- bprm->pf_per_clear |= PER_CLEAR_ON_SETID;
+ bprm->per_clear |= PER_CLEAR_ON_SETID;
bprm->cred->egid = gid;
}
}
+/*
+ * Compute brpm->cred based upon the final binary.
+ */
+static int bprm_creds_from_file(struct linux_binprm *bprm)
+{
+ /* Compute creds based on which file? */
+ struct file *file = bprm->execfd_creds ? bprm->executable : bprm->file;
+
+ bprm_fill_uid(bprm, file);
+ return security_bprm_creds_from_file(bprm, file);
+}
+
/*
* Fill the binprm structure from the inode.
- * Check permissions, then read the first BINPRM_BUF_SIZE bytes
+ * Read the first BINPRM_BUF_SIZE bytes
*
* This may be called multiple times for binary chains (scripts for example).
*/
@@ -1649,20 +1652,6 @@ static int prepare_binprm(struct linux_binprm *bprm)
{
loff_t pos = 0;
- /* Can the interpreter get to the executable without races? */
- if (!bprm->preserve_creds) {
- int retval;
-
- /* Recompute parts of bprm->cred based on bprm->file */
- bprm->active_secureexec = 0;
- bprm->pf_per_clear = 0;
- bprm_fill_uid(bprm);
- retval = security_bprm_repopulate_creds(bprm);
- if (retval)
- return retval;
- }
- bprm->preserve_creds = 0;
-
memset(bprm->buf, 0, BINPRM_BUF_SIZE);
return kernel_read(bprm->file, bprm->buf, BINPRM_BUF_SIZE, &pos);
}
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 50025ead0b72..aece1b340e7d 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -29,13 +29,8 @@ struct linux_binprm {
/* Should an execfd be passed to userspace? */
have_execfd:1,
- /* It is safe to use the creds of a script (see binfmt_misc) */
- preserve_creds:1,
- /*
- * True if most recent call to security_bprm_set_creds
- * resulted in elevated privileges.
- */
- active_secureexec:1,
+ /* Use the creds of a script (see binfmt_misc) */
+ execfd_creds:1,
/*
* Set by bprm_creds_for_exec hook to indicate a
* privilege-gaining exec has happened. Used to set
@@ -55,11 +50,6 @@ struct linux_binprm {
struct file * file;
struct cred *cred; /* new credentials */
int unsafe; /* how unsafe this exec is (mask of LSM_UNSAFE_*) */
- /*
- * bits to clear in current->personality
- * recalculated for each bprm->file.
- */
- unsigned int pf_per_clear;
unsigned int per_clear; /* bits to clear in current->personality */
int argc, envc;
const char * filename; /* Name of binary as seen by procps */
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 1e295ba12c0d..adbc6603abba 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -50,7 +50,7 @@ LSM_HOOK(int, 0, settime, const struct timespec64 *ts,
const struct timezone *tz)
LSM_HOOK(int, 0, vm_enough_memory, struct mm_struct *mm, long pages)
LSM_HOOK(int, 0, bprm_creds_for_exec, struct linux_binprm *bprm)
-LSM_HOOK(int, 0, bprm_repopulate_creds, struct linux_binprm *bprm)
+LSM_HOOK(int, 0, bprm_creds_from_file, struct linux_binprm *bprm, struct file *file)
LSM_HOOK(int, 0, bprm_check_security, struct linux_binprm *bprm)
LSM_HOOK(void, LSM_RET_VOID, bprm_committing_creds, struct linux_binprm *bprm)
LSM_HOOK(void, LSM_RET_VOID, bprm_committed_creds, struct linux_binprm *bprm)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index cd3dd0afceb5..37bb3df751c6 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -44,18 +44,18 @@
* request libc enable secure mode.
* @bprm contains the linux_binprm structure.
* Return 0 if the hook is successful and permission is granted.
- * @bprm_repopulate_creds:
- * Assuming that the relevant bits of @bprm->cred->security have been
- * previously set, examine @bprm->file and regenerate them. This is
- * so that the credentials derived from the interpreter the code is
- * actually going to run are used rather than credentials derived
- * from a script. This done because the interpreter binary needs to
- * reopen script, and may end up opening something completely different.
- * This hook may also optionally check permissions (e.g. for
- * transitions between security domains).
- * The hook must set @bprm->active_secureexec to 1 if AT_SECURE should be set to
+ * @bprm_creds_from_file:
+ * If @file is setpcap, suid, sgid or otherwise marked to change
+ * privilege upon exec, update @bprm->cred to reflect that change.
+ * This is called after finding the binary that will be executed.
+ * without an interpreter. This ensures that the credentials will not
+ * be derived from a script that the binary will need to reopen, which
+ * when reopend may end up being a completely different file. This
+ * hook may also optionally check permissions (e.g. for transitions
+ * between security domains).
+ * The hook must set @bprm->secureexec to 1 if AT_SECURE should be set to
* request libc enable secure mode.
- * The hook must set @bprm->pf_per_clear to the personality flags that
+ * The hook must set @bprm->per_clear to the personality flags that
* should be cleared from current->personality.
* @bprm contains the linux_binprm structure.
* Return 0 if the hook is successful and permission is granted.
diff --git a/include/linux/security.h b/include/linux/security.h
index 6dcec9375e8f..8444fae7c5b9 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -140,7 +140,7 @@ extern int cap_capset(struct cred *new, const struct cred *old,
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
-extern int cap_bprm_repopulate_creds(struct linux_binprm *bprm);
+extern int cap_bprm_creds_from_file(struct linux_binprm *bprm, struct file *file);
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);
@@ -277,7 +277,7 @@ int security_syslog(int type);
int security_settime64(const struct timespec64 *ts, const struct timezone *tz);
int security_vm_enough_memory_mm(struct mm_struct *mm, long pages);
int security_bprm_creds_for_exec(struct linux_binprm *bprm);
-int security_bprm_repopulate_creds(struct linux_binprm *bprm);
+int security_bprm_creds_from_file(struct linux_binprm *bprm, struct file *file);
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);
@@ -575,9 +575,10 @@ static inline int security_bprm_creds_for_exec(struct linux_binprm *bprm)
return 0;
}
-static inline int security_bprm_repopulate_creds(struct linux_binprm *bprm)
+static inline int security_bprm_creds_from_file(struct linux_binprm *bprm,
+ struct file *file)
{
- return cap_bprm_repopulate_creds(bprm);
+ return cap_bprm_creds_from_file(bprm, file);
}
static inline int security_bprm_check(struct linux_binprm *bprm)
diff --git a/security/commoncap.c b/security/commoncap.c
index 6de72d22dc6c..59bf3c1674c8 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -647,7 +647,8 @@ int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data
* its xattrs and, if present, apply them to the proposed credentials being
* constructed by execve().
*/
-static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_fcap)
+static int get_file_caps(struct linux_binprm *bprm, struct file *file,
+ bool *effective, bool *has_fcap)
{
int rc = 0;
struct cpu_vfs_cap_data vcaps;
@@ -657,7 +658,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_f
if (!file_caps_enabled)
return 0;
- if (!mnt_may_suid(bprm->file->f_path.mnt))
+ if (!mnt_may_suid(file->f_path.mnt))
return 0;
/*
@@ -665,10 +666,10 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_f
* explicit that capability bits are limited to s_user_ns and its
* descendants.
*/
- if (!current_in_userns(bprm->file->f_path.mnt->mnt_sb->s_user_ns))
+ if (!current_in_userns(file->f_path.mnt->mnt_sb->s_user_ns))
return 0;
- rc = get_vfs_caps_from_disk(bprm->file->f_path.dentry, &vcaps);
+ rc = get_vfs_caps_from_disk(file->f_path.dentry, &vcaps);
if (rc < 0) {
if (rc == -EINVAL)
printk(KERN_NOTICE "Invalid argument reading file caps for %s\n",
@@ -797,26 +798,27 @@ static inline bool nonroot_raised_pE(struct cred *new, const struct cred *old,
}
/**
- * cap_bprm_repopulate_creds - Set up the proposed credentials for execve().
+ * cap_bprm_creds_from_file - Set up the proposed credentials for execve().
* @bprm: The execution parameters, including the proposed creds
+ * @file: The file to pull the credentials from
*
* Set up the proposed credentials for a new execution context being
* constructed by execve(). The proposed creds in @bprm->cred is altered,
* which won't take effect immediately. Returns 0 if successful, -ve on error.
*/
-int cap_bprm_repopulate_creds(struct linux_binprm *bprm)
+int cap_bprm_creds_from_file(struct linux_binprm *bprm, struct file *file)
{
+ /* Process setpcap binaries and capabilities for uid 0 */
const struct cred *old = current_cred();
struct cred *new = bprm->cred;
bool effective = false, has_fcap = false, is_setid;
int ret;
kuid_t root_uid;
- new->cap_ambient = old->cap_ambient;
if (WARN_ON(!cap_ambient_invariant_ok(old)))
return -EPERM;
- ret = get_file_caps(bprm, &effective, &has_fcap);
+ ret = get_file_caps(bprm, file, &effective, &has_fcap);
if (ret < 0)
return ret;
@@ -826,7 +828,7 @@ int cap_bprm_repopulate_creds(struct linux_binprm *bprm)
/* if we have fs caps, clear dangerous personality flags */
if (__cap_gained(permitted, new, old))
- bprm->pf_per_clear |= PER_CLEAR_ON_SETID;
+ bprm->per_clear |= PER_CLEAR_ON_SETID;
/* Don't let someone trace a set[ug]id/setpcap binary with the revised
* credentials unless they have the appropriate permit.
@@ -889,7 +891,7 @@ int cap_bprm_repopulate_creds(struct linux_binprm *bprm)
(!__is_real(root_uid, new) &&
(effective ||
__cap_grew(permitted, ambient, new))))
- bprm->active_secureexec = 1;
+ bprm->secureexec = 1;
return 0;
}
@@ -1346,7 +1348,7 @@ static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(ptrace_traceme, cap_ptrace_traceme),
LSM_HOOK_INIT(capget, cap_capget),
LSM_HOOK_INIT(capset, cap_capset),
- LSM_HOOK_INIT(bprm_repopulate_creds, cap_bprm_repopulate_creds),
+ LSM_HOOK_INIT(bprm_creds_from_file, cap_bprm_creds_from_file),
LSM_HOOK_INIT(inode_need_killpriv, cap_inode_need_killpriv),
LSM_HOOK_INIT(inode_killpriv, cap_inode_killpriv),
LSM_HOOK_INIT(inode_getsecurity, cap_inode_getsecurity),
diff --git a/security/security.c b/security/security.c
index b890b7e2a765..259b8e750aa2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -828,9 +828,9 @@ int security_bprm_creds_for_exec(struct linux_binprm *bprm)
return call_int_hook(bprm_creds_for_exec, 0, bprm);
}
-int security_bprm_repopulate_creds(struct linux_binprm *bprm)
+int security_bprm_creds_from_file(struct linux_binprm *bprm, struct file *file)
{
- return call_int_hook(bprm_repopulate_creds, 0, bprm);
+ return call_int_hook(bprm_creds_from_file, 0, bprm, file);
}
int security_bprm_check(struct linux_binprm *bprm)
--
2.25.0
More information about the Linux-security-module-archive
mailing list