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

[Vivek Goyal <vgoyal@redhat.com>]

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@redhat.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