linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arjan van de Ven <arjan@linux.intel.com>
Cc: lkml <linux-kernel@vger.kernel.org>,
	John Stultz <john.stultz@linaro.org>,
	Oren Laadan <orenl@cellrox.com>,
	Ruchi Kandoi <kandoiruchi@google.com>,
	Rom Lemarchand <romlem@android.com>,
	Kees Cook <keescook@chromium.org>,
	Android Kernel Team <kernel-team@android.com>
Subject: [PATCH] proc: /proc/<pid>/timerslack_ns permissions fixes
Date: Wed, 17 Feb 2016 21:59:31 -0800	[thread overview]
Message-ID: <1455775171-8125-1-git-send-email-john.stultz@linaro.org> (raw)
In-Reply-To: <1455671191-32105-3-git-send-email-john.stultz@linaro.org>

This patch adjusts the timerslack_ns file permissions to be
0666 but requires PTRACE_MODE_ATTACH_FSCREDS to read or write
the value.

This allows tasks with sufficient privledges (CAP_SYS_PTRACE)
to be able to modify a the timerslack for proccesses owned by
a different user.

This patch also fixes a return value from EINVAL to EPERM,
and does task locking consistently, given we're handling u64s
on 32bit systems. It also makes use of kstrtoull_from_user
which simplifies some code.

Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Oren Laadan <orenl@cellrox.com>
Cc: Ruchi Kandoi <kandoiruchi@google.com>
Cc: Rom Lemarchand <romlem@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
This patch applies on top of the previous two patches
which Andrew already added to -mm. It can be folded
down or kept separate as desired.

I've also wired up the Android userspace side to use
this interface, and tested it there, and things seem
to be working properly ( - with some selinux noise, I
still need to figure out the selinux policy changes,
but its working with permissive mode).

 fs/proc/base.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index d7c51ca..35f583a 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2262,18 +2262,10 @@ static ssize_t timerslack_ns_write(struct file *file, const char __user *buf,
 {
 	struct inode *inode = file_inode(file);
 	struct task_struct *p;
-	char buffer[PROC_NUMBUF];
 	u64 slack_ns;
 	int err;
 
-	memset(buffer, 0, sizeof(buffer));
-	if (count > sizeof(buffer) - 1)
-		count = sizeof(buffer) - 1;
-
-	if (copy_from_user(buffer, buf, count))
-		return -EFAULT;
-
-	err = kstrtoull(strstrip(buffer), 10, &slack_ns);
+	err = kstrtoull_from_user(buf, count, 10, &slack_ns);
 	if (err < 0)
 		return err;
 
@@ -2282,12 +2274,14 @@ static ssize_t timerslack_ns_write(struct file *file, const char __user *buf,
 		return -ESRCH;
 
 	if (ptrace_may_access(p, PTRACE_MODE_ATTACH_FSCREDS)) {
+		task_lock(p);
 		if (slack_ns == 0)
 			p->timer_slack_ns = p->default_timer_slack_ns;
 		else
 			p->timer_slack_ns = slack_ns;
+		task_unlock(p);
 	} else
-		count = -EINVAL;
+		count = -EPERM;
 
 	put_task_struct(p);
 
@@ -2298,18 +2292,22 @@ static int timerslack_ns_show(struct seq_file *m, void *v)
 {
 	struct inode *inode = m->private;
 	struct task_struct *p;
+	int err =  0;
 
 	p = get_proc_task(inode);
 	if (!p)
 		return -ESRCH;
 
-	task_lock(p);
-	seq_printf(m, "%llu\n", p->timer_slack_ns);
-	task_unlock(p);
+	if (ptrace_may_access(p, PTRACE_MODE_ATTACH_FSCREDS)) {
+		task_lock(p);
+		seq_printf(m, "%llu\n", p->timer_slack_ns);
+		task_unlock(p);
+	} else
+		err = -EPERM;
 
 	put_task_struct(p);
 
-	return 0;
+	return err;
 }
 
 static int timerslack_ns_open(struct inode *inode, struct file *filp)
@@ -2899,7 +2897,7 @@ static const struct pid_entry tgid_base_stuff[] = {
 #ifdef CONFIG_CHECKPOINT_RESTORE
 	REG("timers",	  S_IRUGO, proc_timers_operations),
 #endif
-	REG("timerslack_ns", S_IRUGO|S_IWUSR, proc_pid_set_timerslack_ns_operations),
+	REG("timerslack_ns", S_IRUGO|S_IWUGO, proc_pid_set_timerslack_ns_operations),
 };
 
 static int proc_tgid_base_readdir(struct file *file, struct dir_context *ctx)
-- 
1.9.1

  parent reply	other threads:[~2016-02-18  5:59 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-17  1:06 [PATCH 0/2] Extend timer_slack_ns to u64 on 32bit systems & add /proc/<pid>/timerslack_ns John Stultz
2016-02-17  1:06 ` [PATCH 1/2] timer: Convert timer_slack_ns from unsigned long to u64 John Stultz
2016-02-17  1:06 ` [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface John Stultz
2016-02-17 19:35   ` Andrew Morton
2016-02-17 20:09     ` Kees Cook
2016-02-17 20:18       ` Andrew Morton
2016-02-17 20:51         ` John Stultz
2016-02-17 21:07           ` Andrew Morton
2016-02-17 22:29         ` John Stultz
2016-02-17 22:45           ` Kees Cook
2016-02-17 22:51             ` John Stultz
2016-02-17 22:53           ` Andrew Morton
2016-02-17 20:49     ` John Stultz
2016-02-18  5:59   ` John Stultz [this message]
2016-02-18 17:52     ` [PATCH] proc: /proc/<pid>/timerslack_ns permissions fixes Kees Cook
2016-07-13 23:47   ` [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface John Stultz
2016-07-14  3:39     ` Kees Cook
2016-07-14  5:29       ` Arjan van de Ven
2016-07-14 12:48       ` Serge E. Hallyn
2016-07-14 13:42         ` Arjan van de Ven
2016-07-14 16:01           ` John Stultz
2016-07-14 16:09         ` John Stultz
2016-07-14 17:45           ` Kees Cook
2016-07-14 17:48             ` Arjan van de Ven
2016-07-14 17:49             ` Serge E. Hallyn
2016-07-14 17:56               ` Kees Cook
2016-07-14 20:21                 ` Serge E. Hallyn

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=1455775171-8125-1-git-send-email-john.stultz@linaro.org \
    --to=john.stultz@linaro.org \
    --cc=akpm@linux-foundation.org \
    --cc=arjan@linux.intel.com \
    --cc=kandoiruchi@google.com \
    --cc=keescook@chromium.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=orenl@cellrox.com \
    --cc=romlem@android.com \
    --cc=tglx@linutronix.de \
    /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).