From: Thomas Gleixner <[email protected]>

The thread clock permissions are restricted to tasks of the same thread
group, but that also prevents a ptracer from reading them. This is
inconsistent vs. the process restrictions and unnecessary strict.

Relax it to ptrace permissions in the same way as process permissions are
handled.

Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Frederic Weisbecker <[email protected]>
---
 kernel/time/posix-cpu-timers.c |   66 +++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 34 deletions(-)

--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -51,6 +51,7 @@ void update_rlimit_cpu(struct task_struc
 static struct task_struct *lookup_task(const pid_t pid, bool thread,
                                       bool gettime)
 {
+       unsigned int mode = PTRACE_MODE_ATTACH_REALCREDS;
        struct task_struct *p;
 
        /*
@@ -64,50 +65,47 @@ static struct task_struct *lookup_task(c
        if (!p)
                return p;
 
-       if (thread)
-               return same_thread_group(p, current) ? p : NULL;
-
        if (gettime) {
                /*
-                * For clock_gettime(PROCESS) the task does not need to be
-                * the actual group leader. tsk->sighand gives
-                * access to the group's clock.
-                *
-                * Timers need the group leader because they take a
-                * reference on it and store the task pointer until the
-                * timer is destroyed.
+                * For clock_gettime() the task does not need to be the
+                * actual group leader. tsk->sighand gives access to the
+                * group's clock.
                 *
-                * current can obviously access it's own process, so spare
-                * the ptrace check below.
+                * The trivial case is that p is current or in the same
+                * thread group, i.e. sharing p->signal. Spare the ptrace
+                * check in that case.
                 */
-               if (p == current)
+               if (same_thread_group(p, current))
                        return p;
 
-               if (!thread_group_leader(p))
-                       return NULL;
+               mode = PTRACE_MODE_READ_REALCREDS;
 
-               if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
-                       return NULL;
-               return p;
-       }
+       } else if (thread) {
+               /*
+                * Timer is going to be attached to a thread. If p is
+                * current or in the same thread group, granted.
+                */
+               if (same_thread_group(p, current))
+                       return p;
 
-       /*
-        * For processes require that p is group leader.
-        */
-       if (!has_group_leader_pid(p))
-               return NULL;
+       } else {
+               /*
+                * Process wide timers need the group leader because they
+                * take a reference on it and store the task pointer until
+                * the timer is destroyed.
+                */
+               if (!has_group_leader_pid(p))
+                       return NULL;
 
-       /*
-        * Avoid the ptrace overhead when this is current's process
-        */
-       if (same_thread_group(p, current))
-               return p;
+               /*
+                * Avoid the ptrace overhead when this is current's process
+                */
+               if (same_thread_group(p, current))
+                       return p;
+       }
 
-       /*
-        * Creating timers on processes which cannot be ptraced is not
-        * permitted:
-        */
-       return ptrace_may_access(p, PTRACE_MODE_ATTACH_REALCREDS) ? p : NULL;
+       /* Decide based on the ptrace permissions. */
+       return ptrace_may_access(p, mode) ? p : NULL;
 }
 
 static struct task_struct *__get_task_for_clock(const clockid_t clock,


Reply via email to