linux-kernel - [PATCH] proc: /proc/<pid>/timerslack

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1455775171-8125-1-git-send-email-john.stultz@linaro.org>
Date:	Wed, 17 Feb 2016 21:59:31 -0800
From:	John Stultz <john.stultz@...aro.org>
To:	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Arjan van de Ven <arjan@...ux.intel.com>
Cc:	lkml <linux-kernel@...r.kernel.org>,
	John Stultz <john.stultz@...aro.org>,
	Oren Laadan <orenl@...lrox.com>,
	Ruchi Kandoi <kandoiruchi@...gle.com>,
	Rom Lemarchand <romlem@...roid.com>,
	Kees Cook <keescook@...omium.org>,
	Android Kernel Team <kernel-team@...roid.com>
Subject: [PATCH] proc: /proc/<pid>/timerslack_ns permissions fixes

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@...ux.intel.com>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Oren Laadan <orenl@...lrox.com>
Cc: Ruchi Kandoi <kandoiruchi@...gle.com>
Cc: Rom Lemarchand <romlem@...roid.com>
Cc: Kees Cook <keescook@...omium.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Android Kernel Team <kernel-team@...roid.com>
Signed-off-by: John Stultz <john.stultz@...aro.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