Commit 56305aa9 authored by Eric W. Biederman's avatar Eric W. Biederman
Browse files

exec: Compute file based creds only once 



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.

Reviewed-by: default avatarKees Cook <keescook@chromium.org>
Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
parent a7868323

fs/binfmt_misc.c

+1 −1
Original line number Diff line number Diff line
@@ -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:

fs/exec.c

+26 −37
Original line number Diff line number Diff line
@@ -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);

}

include/linux/binfmts.h

+2 −12
Original line number Diff line number Diff line
@@ -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 */

include/linux/lsm_hook_defs.h

+1 −1
Original line number Diff line number Diff line
@@ -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)

include/linux/lsm_hooks.h

+11 −11
Original line number Diff line number Diff line
@@ -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 add to @bprm->pf_per_clear any personality flags that

 *	The hook must add to @bprm->per_clear any 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.

CRA Git | Maintained and supported by SUSTech CRA and CCSE