lkml.org 
[lkml]   [2013]   [Sep]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 11/16] ptrace: Do not allow ptrace() from unsigned process to signed one
Date
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


\
 
 \ /
  Last update: 2013-09-11 00:21    [W:0.137 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site