linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@kernel.org>,
	Kees Cook <keescook@chromium.org>
Subject: [patch 2/6] posix-cpu-timers: Fix permission check regression
Date: Thu, 05 Sep 2019 14:03:41 +0200	[thread overview]
Message-ID: <20190905120539.797994508@linutronix.de> (raw)
In-Reply-To: 20190905120339.561100423@linutronix.de

The recent consolidation of the three permission checks introduced a subtle
regression. For timer_create() with a process wide timer it returns the
current task if the lookup through the PID which is encoded into the
clockid results in returning current.

That's broken because it does not validate whether the current task is the
group leader.

That was caused by the two different variants of permission checks:

  - posix_cpu_timer_get() allowed access to the process wide clock when the
    looked up task is current. That's not an issue because the process wide
    clock is in the shared sighand.

  - posix_cpu_timer_create() made sure that the looked up task is the group
    leader.

Restore the previous state.

Note, that these permission checks are more than questionable, but that's
subject to follow up changes.

Fixes: 6ae40e3fdcd3 ("posix-cpu-timers: Provide task validation functions")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/posix-cpu-timers.c |   40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -47,25 +47,42 @@ void update_rlimit_cpu(struct task_struc
 /*
  * Functions for validating access to tasks.
  */
-static struct task_struct *lookup_task(const pid_t pid, bool thread)
+static struct task_struct *lookup_task(const pid_t pid, bool thread,
+				       bool gettime)
 {
 	struct task_struct *p;
 
+	/*
+	 * If the encoded PID is 0, then the timer is targeted at current
+	 * or the process to which current belongs.
+	 */
 	if (!pid)
 		return thread ? current : current->group_leader;
 
 	p = find_task_by_vpid(pid);
-	if (!p || p == current)
+	if (!p)
 		return p;
+
 	if (thread)
 		return same_thread_group(p, current) ? p : NULL;
-	if (p == current)
-		return p;
+
+	if (gettime) {
+		/*
+		 * For clock_gettime() the task does not need to be the
+		 * actual group leader. tsk->sighand gives access to the
+		 * group's clock.
+		 */
+		return (p == current || thread_group_leader(p)) ? p : NULL;
+	}
+
+	/*
+	 * For processes require that p is group leader.
+	 */
 	return has_group_leader_pid(p) ? p : NULL;
 }
 
 static struct task_struct *__get_task_for_clock(const clockid_t clock,
-						bool getref)
+						bool getref, bool gettime)
 {
 	const bool thread = !!CPUCLOCK_PERTHREAD(clock);
 	const pid_t pid = CPUCLOCK_PID(clock);
@@ -75,7 +92,7 @@ static struct task_struct *__get_task_fo
 		return NULL;
 
 	rcu_read_lock();
-	p = lookup_task(pid, thread);
+	p = lookup_task(pid, thread, gettime);
 	if (p && getref)
 		get_task_struct(p);
 	rcu_read_unlock();
@@ -84,12 +101,17 @@ static struct task_struct *__get_task_fo
 
 static inline struct task_struct *get_task_for_clock(const clockid_t clock)
 {
-	return __get_task_for_clock(clock, true);
+	return __get_task_for_clock(clock, true, false);
+}
+
+static inline struct task_struct *get_task_for_clock_get(const clockid_t clock)
+{
+	return __get_task_for_clock(clock, true, true);
 }
 
 static inline int validate_clock_permissions(const clockid_t clock)
 {
-	return __get_task_for_clock(clock, false) ? 0 : -EINVAL;
+	return __get_task_for_clock(clock, false, false) ? 0 : -EINVAL;
 }
 
 /*
@@ -339,7 +361,7 @@ static int posix_cpu_clock_get(const clo
 	struct task_struct *tsk;
 	u64 t;
 
-	tsk = get_task_for_clock(clock);
+	tsk = get_task_for_clock_get(clock);
 	if (!tsk)
 		return -EINVAL;
 



  parent reply	other threads:[~2019-09-05 12:12 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-05 12:03 [patch 0/6] posix-cpu-timers: Fallout fixes and permission tightening Thomas Gleixner
2019-09-05 12:03 ` [patch 1/6] posix-cpu-timers: Always clear head pointer on dequeue Thomas Gleixner
2019-09-05 15:49   ` Frederic Weisbecker
2019-09-05 12:03 ` Thomas Gleixner [this message]
2019-09-05 17:31   ` [patch 2/6] posix-cpu-timers: Fix permission check regression Frederic Weisbecker
2019-09-05 18:55     ` Thomas Gleixner
2019-09-05 21:15       ` [patch V2 " Thomas Gleixner
2019-09-09 15:13         ` Frederic Weisbecker
2019-09-10 11:18         ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2019-09-05 12:03 ` [patch 3/6] posix-cpu-timers: Restrict timer_create() permissions Thomas Gleixner
2019-09-21  0:44   ` Frederic Weisbecker
2019-09-05 12:03 ` [patch 4/6] posix-cpu-timers: Restrict clock_gettime() permissions Thomas Gleixner
2019-09-23 13:39   ` Frederic Weisbecker
2019-09-05 12:03 ` [patch 5/6] posix-cpu-timers: Sanitize thread clock permissions Thomas Gleixner
2019-09-05 12:21   ` Peter Zijlstra
2019-09-05 14:11     ` Thomas Gleixner
2019-09-23 14:03   ` Frederic Weisbecker
2019-09-05 12:03 ` [patch 6/6] posix-cpu-timers: Make PID=0 and PID=self handling consistent Thomas Gleixner
2019-09-23 14:13   ` Frederic Weisbecker
2019-09-23 14:17     ` Thomas Gleixner
2019-09-05 14:48 ` [patch 0/6] posix-cpu-timers: Fallout fixes and permission tightening Frederic Weisbecker
2019-09-05 14:57   ` Thomas Gleixner
2019-09-05 15:21     ` Frederic Weisbecker
2019-09-05 20:32 ` Kees Cook
2019-09-05 21:13   ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190905120539.797994508@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=fweisbec@gmail.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).