Commit 3977e285 authored by Eric W. Biederman's avatar Eric W. Biederman
Browse files

exec: Remove the recomputation of bprm->cred 

Recomputing the uids, gids, capabilities, and related flags each time a
new bprm->file is set is error prone and unnecessary.

This set of changes splits per_clear temporarily into two separate
variables.  This is the last change necessary to ensure that
everything that is computed from brpm->file in bprm->cred is
recomputed every time a new bprm->file is set.  Then the code is
refactored to compute bprm->cred from bprm->file when the final
brpm->file is known, removing the need for recomputation entirely.

Doing this in two steps should allow anyone who has problems later to
bisect and tell if it was the semantic change or the refactoring that
caused them problems.

Eric W. Biederman (2):
      exec: Add a per bprm->file version of per_clear
      exec: Compute file based creds only once

 fs/binfmt_misc.c              |  2 +-
 fs/exec.c                     | 57 ++++++++++++++++++-------------------------
 include/linux/binfmts.h       |  9 ++-----
 include/linux/lsm_hook_defs.h |  2 +-
 include/linux/lsm_hooks.h     | 22 +++++++++--------
 include/linux/security.h      |  9 ++++---
 security/commoncap.c          | 22 +++++++++--------
 security/security.c           |  4 +--
 8 files changed, 59 insertions(+), 68 deletions(-)

Merge branch 'exec-norecompute-v2' into exec-next
parents e32f8879 56305aa9

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

+24 −33
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.

	 */
@@ -1364,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;
@@ -1586,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;
@@ -1638,9 +1630,21 @@ static void bprm_fill_uid(struct linux_binprm *bprm) 
	}

}



/*

 * 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).

 */
@@ -1648,19 +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_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 −7
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

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

+12 −10
Original line number Diff line number Diff line
@@ -44,17 +44,19 @@ 
 *	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->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.

 * @bprm_check_security:

CRA Git | Maintained and supported by SUSTech CRA and CCSE