All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Kees Cook" <keescook@chromium.org>,
	"Andy Lutomirski" <luto@kernel.org>,
	"David Howells" <dhowells@redhat.com>,
	"Serge Hallyn" <serge@hallyn.com>,
	"John Johansen" <john.johansen@canonical.com>,
	"Casey Schaufler" <casey@schaufler-ca.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	"Michal Hocko" <mhocko@kernel.org>,
	"Ben Hutchings" <ben@decadent.org.uk>,
	"Hugh Dickins" <hughd@google.com>,
	"Oleg Nesterov" <oleg@redhat.com>,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	"Rik van Riel" <riel@redhat.com>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"James Morris" <james.l.morris@oracle.com>,
	"Greg Ungerer" <gerg@linux-m68k.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Nicolas Pitre" <nicolas.pitre@linaro.org>,
	"Stephen Smalley" <sds@tycho.nsa.gov>,
	"Paul Moore" <paul@paul-moore.com>,
	"Vivek Goyal" <vgoyal@redhat.com>,
	"Mickaël Salaün" <mic@digikod.net>,
	"Tetsuo Handa" <penguin-kernel@I-love.SAKURA.ne.jp>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: [PATCH 1/2] exec: Move security_bprm_secureexec() earlier
Date: Fri,  7 Jul 2017 12:56:59 -0700	[thread overview]
Message-ID: <1499457420-83038-2-git-send-email-keescook@chromium.org> (raw)
In-Reply-To: <1499457420-83038-1-git-send-email-keescook@chromium.org>

There are several places where exec needs to know if a privilege-gain
has happened. Right now it only checks uid/euid differences, though
capabilities could have been elevated too. This moves the secureexec
check ahead of committing credentials, and retains the value for later
examination. A resulting redundant case of clearing pdeath_signal is
also removed from Smack, and commoncap is updated to examine bprm->cred
instead of current->cred.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/binfmt_elf.c            |  2 +-
 fs/binfmt_elf_fdpic.c      |  2 +-
 fs/exec.c                  | 15 ++++++++++-----
 include/linux/binfmts.h    |  3 ++-
 include/linux/lsm_hooks.h  |  3 ++-
 security/commoncap.c       |  4 +---
 security/smack/smack_lsm.c | 15 ---------------
 7 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 5075fd5c62c8..7f6ec4dac13d 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -254,7 +254,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);
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index cf93a4fad012..5aa9199dfb13 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -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
diff --git a/fs/exec.c b/fs/exec.c
index ddca2cf15f71..1e8d647d8e7c 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1330,12 +1330,18 @@ EXPORT_SYMBOL(would_dump);
 
 void setup_new_exec(struct linux_binprm * bprm)
 {
+	/* This is the point of no return */
+
+	if (security_bprm_secureexec(bprm)) {
+		/* Record for AT_SECURE. */
+		bprm->secureexec = 1;
+	}
+
 	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()))
+	if (!bprm->secureexec)
 		set_dumpable(current->mm, SUID_DUMP_USER);
 	else
 		set_dumpable(current->mm, suid_dumpable);
@@ -1350,9 +1356,8 @@ 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())) {
+	/* prepare to install the new credentials */
+	if (bprm->secureexec) {
 		current->pdeath_signal = 0;
 	} else {
 		if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP)
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 05488da3aee9..1afaa303cad0 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -27,9 +27,10 @@ struct linux_binprm {
 	unsigned int
 		cred_prepared:1,/* true if creds already prepared (multiple
 				 * preps happen for interpreters) */
-		cap_effective:1;/* true if has elevated effective capabilities,
+		cap_effective:1,/* true if has elevated effective capabilities,
 				 * false if not; except for init which inherits
 				 * its parent's caps anyway */
+		secureexec:1;	/* true when gaining privileges */
 #ifdef __alpha__
 	unsigned int taso:1;
 #endif
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 080f34e66017..d1bd24fb4a33 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -72,7 +72,8 @@
  *	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.
+ *	should enable secure mode. Called before bprm_committing_creds(),
+ *	so pending credentials are in @bprm->cred.
  *	@bprm contains the linux_binprm structure.
  *
  * Security hooks for filesystem operations.
diff --git a/security/commoncap.c b/security/commoncap.c
index 7abebd782d5e..482d3aac2fc6 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -624,12 +624,10 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
  * 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();
+	const struct cred *cred = bprm->cred;
 	kuid_t root_uid = make_kuid(cred->user_ns, 0);
 
 	if (!uid_eq(cred->uid, root_uid)) {
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 658f5d8c7e76..0c89afcb24c4 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -954,20 +954,6 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
 }
 
 /**
- * smack_bprm_committing_creds - Prepare to install the new credentials
- * from bprm.
- *
- * @bprm: binprm for exec
- */
-static void smack_bprm_committing_creds(struct linux_binprm *bprm)
-{
-	struct task_smack *bsp = bprm->cred->security;
-
-	if (bsp->smk_task != bsp->smk_forked)
-		current->pdeath_signal = 0;
-}
-
-/**
  * smack_bprm_secureexec - Return the decision to use secureexec.
  * @bprm: binprm for exec
  *
@@ -4645,7 +4631,6 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sb_parse_opts_str, smack_parse_opts_str),
 
 	LSM_HOOK_INIT(bprm_set_creds, smack_bprm_set_creds),
-	LSM_HOOK_INIT(bprm_committing_creds, smack_bprm_committing_creds),
 	LSM_HOOK_INIT(bprm_secureexec, smack_bprm_secureexec),
 
 	LSM_HOOK_INIT(inode_alloc_security, smack_inode_alloc_security),
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: keescook@chromium.org (Kees Cook)
To: linux-security-module@vger.kernel.org
Subject: [PATCH 1/2] exec: Move security_bprm_secureexec() earlier
Date: Fri,  7 Jul 2017 12:56:59 -0700	[thread overview]
Message-ID: <1499457420-83038-2-git-send-email-keescook@chromium.org> (raw)
In-Reply-To: <1499457420-83038-1-git-send-email-keescook@chromium.org>

There are several places where exec needs to know if a privilege-gain
has happened. Right now it only checks uid/euid differences, though
capabilities could have been elevated too. This moves the secureexec
check ahead of committing credentials, and retains the value for later
examination. A resulting redundant case of clearing pdeath_signal is
also removed from Smack, and commoncap is updated to examine bprm->cred
instead of current->cred.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/binfmt_elf.c            |  2 +-
 fs/binfmt_elf_fdpic.c      |  2 +-
 fs/exec.c                  | 15 ++++++++++-----
 include/linux/binfmts.h    |  3 ++-
 include/linux/lsm_hooks.h  |  3 ++-
 security/commoncap.c       |  4 +---
 security/smack/smack_lsm.c | 15 ---------------
 7 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 5075fd5c62c8..7f6ec4dac13d 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -254,7 +254,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);
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index cf93a4fad012..5aa9199dfb13 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -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
diff --git a/fs/exec.c b/fs/exec.c
index ddca2cf15f71..1e8d647d8e7c 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1330,12 +1330,18 @@ EXPORT_SYMBOL(would_dump);
 
 void setup_new_exec(struct linux_binprm * bprm)
 {
+	/* This is the point of no return */
+
+	if (security_bprm_secureexec(bprm)) {
+		/* Record for AT_SECURE. */
+		bprm->secureexec = 1;
+	}
+
 	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()))
+	if (!bprm->secureexec)
 		set_dumpable(current->mm, SUID_DUMP_USER);
 	else
 		set_dumpable(current->mm, suid_dumpable);
@@ -1350,9 +1356,8 @@ 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())) {
+	/* prepare to install the new credentials */
+	if (bprm->secureexec) {
 		current->pdeath_signal = 0;
 	} else {
 		if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP)
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 05488da3aee9..1afaa303cad0 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -27,9 +27,10 @@ struct linux_binprm {
 	unsigned int
 		cred_prepared:1,/* true if creds already prepared (multiple
 				 * preps happen for interpreters) */
-		cap_effective:1;/* true if has elevated effective capabilities,
+		cap_effective:1,/* true if has elevated effective capabilities,
 				 * false if not; except for init which inherits
 				 * its parent's caps anyway */
+		secureexec:1;	/* true when gaining privileges */
 #ifdef __alpha__
 	unsigned int taso:1;
 #endif
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 080f34e66017..d1bd24fb4a33 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -72,7 +72,8 @@
  *	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.
+ *	should enable secure mode. Called before bprm_committing_creds(),
+ *	so pending credentials are in @bprm->cred.
  *	@bprm contains the linux_binprm structure.
  *
  * Security hooks for filesystem operations.
diff --git a/security/commoncap.c b/security/commoncap.c
index 7abebd782d5e..482d3aac2fc6 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -624,12 +624,10 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
  * 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();
+	const struct cred *cred = bprm->cred;
 	kuid_t root_uid = make_kuid(cred->user_ns, 0);
 
 	if (!uid_eq(cred->uid, root_uid)) {
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 658f5d8c7e76..0c89afcb24c4 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -954,20 +954,6 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
 }
 
 /**
- * smack_bprm_committing_creds - Prepare to install the new credentials
- * from bprm.
- *
- * @bprm: binprm for exec
- */
-static void smack_bprm_committing_creds(struct linux_binprm *bprm)
-{
-	struct task_smack *bsp = bprm->cred->security;
-
-	if (bsp->smk_task != bsp->smk_forked)
-		current->pdeath_signal = 0;
-}
-
-/**
  * smack_bprm_secureexec - Return the decision to use secureexec.
  * @bprm: binprm for exec
  *
@@ -4645,7 +4631,6 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sb_parse_opts_str, smack_parse_opts_str),
 
 	LSM_HOOK_INIT(bprm_set_creds, smack_bprm_set_creds),
-	LSM_HOOK_INIT(bprm_committing_creds, smack_bprm_committing_creds),
 	LSM_HOOK_INIT(bprm_secureexec, smack_bprm_secureexec),
 
 	LSM_HOOK_INIT(inode_alloc_security, smack_inode_alloc_security),
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2017-07-07 19:57 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-07 19:56 [PATCH 0/2] exec: Use sane stack rlimit for setuid exec Kees Cook
2017-07-07 19:56 ` Kees Cook
2017-07-07 19:56 ` Kees Cook [this message]
2017-07-07 19:56   ` [PATCH 1/2] exec: Move security_bprm_secureexec() earlier Kees Cook
2017-07-07 19:57 ` [PATCH 2/2] exec: Use sane stack rlimit for setuid exec Kees Cook
2017-07-07 19:57   ` Kees Cook
2017-07-07 20:04 ` [PATCH 0/2] " Linus Torvalds
2017-07-07 20:04   ` Linus Torvalds
2017-07-07 20:09   ` Linus Torvalds
2017-07-07 20:09     ` Linus Torvalds
2017-07-07 22:10     ` Kees Cook
2017-07-07 22:10       ` Kees Cook
2017-07-07 22:13   ` Kees Cook
2017-07-07 22:13     ` Kees Cook
2017-07-07 22:39     ` Linus Torvalds
2017-07-07 22:39       ` Linus Torvalds
2017-07-08  3:59   ` Kees Cook
2017-07-08  3:59     ` Kees Cook
2017-07-07 21:55 ` Andy Lutomirski
2017-07-07 21:55   ` Andy Lutomirski
2017-07-07 22:19   ` Kees Cook
2017-07-07 22:19     ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1499457420-83038-2-git-send-email-keescook@chromium.org \
    --to=keescook@chromium.org \
    --cc=Jason@zx2c4.com \
    --cc=ben@decadent.org.uk \
    --cc=casey@schaufler-ca.com \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=gerg@linux-m68k.org \
    --cc=hughd@google.com \
    --cc=james.l.morris@oracle.com \
    --cc=john.johansen@canonical.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=mic@digikod.net \
    --cc=mingo@kernel.org \
    --cc=nicolas.pitre@linaro.org \
    --cc=oleg@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=riel@redhat.com \
    --cc=sds@tycho.nsa.gov \
    --cc=serge@hallyn.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vgoyal@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.