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

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

Reply via email to