linux-kernel - [PATCH 3.16 188/192] sched: move no_new

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <lsq.1507553065.168487102@decadent.org.uk>
Date:   Mon, 09 Oct 2017 13:44:25 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, "Oleg Nesterov" <oleg@...hat.com>,
        "Kees Cook" <keescook@...omium.org>,
        "Andy Lutomirski" <luto@...capital.net>
Subject: [PATCH 3.16 188/192] sched: move no_new_privs into new atomic flags

3.16.49-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Kees Cook <keescook@...omium.org>

commit 1d4457f99928a968767f6405b4a1f50845aa15fd upstream.

Since seccomp transitions between threads requires updates to the
no_new_privs flag to be atomic, the flag must be part of an atomic flag
set. This moves the nnp flag into a separate task field, and introduces
accessors.

Signed-off-by: Kees Cook <keescook@...omium.org>
Reviewed-by: Oleg Nesterov <oleg@...hat.com>
Reviewed-by: Andy Lutomirski <luto@...capital.net>
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 fs/exec.c                  |  4 ++--
 include/linux/sched.h      | 18 +++++++++++++++---
 kernel/seccomp.c           |  2 +-
 kernel/sys.c               |  4 ++--
 security/apparmor/domain.c |  4 ++--
 5 files changed, 22 insertions(+), 10 deletions(-)

--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1269,7 +1269,7 @@ static void check_unsafe_exec(struct lin
 	 * This isn't strictly necessary, but it makes it harder for LSMs to
 	 * mess up.
 	 */
-	if (current->no_new_privs)
+	if (task_no_new_privs(current))
 		bprm->unsafe |= LSM_UNSAFE_NO_NEW_PRIVS;
 
 	t = p;
@@ -1303,7 +1303,7 @@ static void bprm_fill_uid(struct linux_b
 	if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
 		return;
 
-	if (current->no_new_privs)
+	if (task_no_new_privs(current))
 		return;
 
 	inode = file_inode(bprm->file);
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1317,13 +1317,12 @@ struct task_struct {
 				 * execve */
 	unsigned in_iowait:1;
 
-	/* task may not gain privileges */
-	unsigned no_new_privs:1;
-
 	/* Revert to default priority/policy when forking */
 	unsigned sched_reset_on_fork:1;
 	unsigned sched_contributes_to_load:1;
 
+	unsigned long atomic_flags; /* Flags needing atomic access. */
+
 	pid_t pid;
 	pid_t tgid;
 
@@ -1979,6 +1978,19 @@ static inline void memalloc_noio_restore
 	current->flags = (current->flags & ~PF_MEMALLOC_NOIO) | flags;
 }
 
+/* Per-process atomic flags. */
+#define PFA_NO_NEW_PRIVS 0x00000001	/* May not gain new privileges. */
+
+static inline bool task_no_new_privs(struct task_struct *p)
+{
+	return test_bit(PFA_NO_NEW_PRIVS, &p->atomic_flags);
+}
+
+static inline void task_set_no_new_privs(struct task_struct *p)
+{
+	set_bit(PFA_NO_NEW_PRIVS, &p->atomic_flags);
+}
+
 /*
  * task->jobctl flags
  */
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -224,7 +224,7 @@ static long seccomp_attach_filter(struct
 	 * This avoids scenarios where unprivileged tasks can affect the
 	 * behavior of privileged children.
 	 */
-	if (!current->no_new_privs &&
+	if (!task_no_new_privs(current) &&
 	    security_capable_noaudit(current_cred(), current_user_ns(),
 				     CAP_SYS_ADMIN) != 0)
 		return -EACCES;
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1989,12 +1989,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsi
 		if (arg2 != 1 || arg3 || arg4 || arg5)
 			return -EINVAL;
 
-		current->no_new_privs = 1;
+		task_set_no_new_privs(current);
 		break;
 	case PR_GET_NO_NEW_PRIVS:
 		if (arg2 || arg3 || arg4 || arg5)
 			return -EINVAL;
-		return current->no_new_privs ? 1 : 0;
+		return task_no_new_privs(current) ? 1 : 0;
 	case PR_GET_THP_DISABLE:
 		if (arg2 || arg3 || arg4 || arg5)
 			return -EINVAL;
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -621,7 +621,7 @@ int aa_change_hat(const char *hats[], in
 	 * There is no exception for unconfined as change_hat is not
 	 * available.
 	 */
-	if (current->no_new_privs)
+	if (task_no_new_privs(current))
 		return -EPERM;
 
 	/* released below */
@@ -778,7 +778,7 @@ int aa_change_profile(const char *ns_nam
 	 * no_new_privs is set because this aways results in a reduction
 	 * of permissions.
 	 */
-	if (current->no_new_privs && !unconfined(profile)) {
+	if (task_no_new_privs(current) && !unconfined(profile)) {
 		put_cred(cred);
 		return -EPERM;
 	}