linux-kernel - [PATCH 11/16] ptrace: Do not allow ptrace() from unsigned process to signed one

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: <1378849471-10521-12-git-send-email-vgoyal@redhat.com>
Date:	Tue, 10 Sep 2013 17:44:26 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org, kexec@...ts.infradead.org
Cc:	akpm@...ux-foundation.org, zohar@...ux.vnet.ibm.com,
	d.kasatkin@...sung.com, ebiederm@...ssion.com, hpa@...or.com,
	matthew.garrett@...ula.com, vgoyal@...hat.com
Subject: [PATCH 11/16] ptrace: Do not allow ptrace() from unsigned process to signed one

Do not allow unsigned processes to ptrace() signed ones otherwise they can
modify the address space of signed processes and whole purpose of signature
verification is defeated.

Signed-off-by: Vivek Goyal <vgoyal@...hat.com>
---
 fs/binfmt_elf.c      | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
 security/commoncap.c | 11 +++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 22a8272..8f2286e 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -568,6 +568,43 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
 #endif
 }
 
+#ifdef CONFIG_BINFMT_ELF_SIG
+/* check if current is being ptraced by tracer which is unsigned */
+static bool ptraced_by_unsafe_tracer(void)
+{
+	struct task_struct *child = current, *parent;
+	bool ret = false;
+	const struct cred *tcred;
+
+	/* Make sure parent does not change due to tracer ptrace detach */
+	read_lock(&tasklist_lock);
+
+	if (!child->ptrace) {
+		ret = false;
+		goto out;
+	}
+
+	parent = child->parent;
+	rcu_read_lock();
+	tcred = __task_cred(parent);
+	if (!tcred->proc_signed)
+		ret = true;
+	rcu_read_unlock();
+
+	/*
+	 * Make sure parent is memlocked too otherwise it might be signed
+	 * but still being swapped out and is open to address space
+	 * modifications.
+	 */
+	if (!test_bit(MMF_VM_LOCKED, &parent->mm->flags))
+		ret = true;
+
+out:
+	read_unlock(&tasklist_lock);
+	return ret;
+}
+#endif
+
 static int load_elf_binary(struct linux_binprm *bprm)
 {
 	struct file *interpreter = NULL; /* to shut gcc up */
@@ -951,8 +988,16 @@ static int load_elf_binary(struct linux_binprm *bprm)
 			send_sig(SIGKILL, current, 0);
 			goto out_free_dentry;
 		}
-		/* Signature verification successful */
-		bprm->cred->proc_signed = true;
+
+		/*
+		 * Signature verification successful. If this process is
+		 * is being ptraced at the time of exec() and tracer is
+		 * not signed, do not set proc_signed, otherwise unsigned
+		 * tracer could change signed tracee's address space,
+		 * effectively nullifying singature checking.
+		 */
+		if (!ptraced_by_unsafe_tracer())
+			bprm->cred->proc_signed = true;
 	}
 #endif
 
diff --git a/security/commoncap.c b/security/commoncap.c
index c44b6fe..4d7f90e 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -146,6 +146,12 @@ int cap_ptrace_access_check(struct task_struct *child, unsigned int mode)
 	rcu_read_lock();
 	cred = current_cred();
 	child_cred = __task_cred(child);
+
+	if (mode != PTRACE_MODE_READ && child_cred->proc_signed &&
+	    !cred->proc_signed) {
+		ret = -EPERM;
+		goto out;
+	}
 	if (cred->user_ns == child_cred->user_ns &&
 	    cap_issubset(child_cred->cap_permitted, cred->cap_permitted))
 		goto out;
@@ -178,6 +184,11 @@ int cap_ptrace_traceme(struct task_struct *parent)
 	rcu_read_lock();
 	cred = __task_cred(parent);
 	child_cred = current_cred();
+
+	if (child_cred->proc_signed && !cred->proc_signed) {
+		ret = -EPERM;
+		goto out;
+	}
 	if (cred->user_ns == child_cred->user_ns &&
 	    cap_issubset(child_cred->cap_permitted, cred->cap_permitted))
 		goto out;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/