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
| ||
|
Date: Thu, 18 Feb 2016 09:52:20 -0800 From: Kees Cook <keescook@...omium.org> To: John Stultz <john.stultz@...aro.org> Cc: Andrew Morton <akpm@...ux-foundation.org>, Thomas Gleixner <tglx@...utronix.de>, Arjan van de Ven <arjan@...ux.intel.com>, lkml <linux-kernel@...r.kernel.org>, Oren Laadan <orenl@...lrox.com>, Ruchi Kandoi <kandoiruchi@...gle.com>, Rom Lemarchand <romlem@...roid.com>, Android Kernel Team <kernel-team@...roid.com> Subject: Re: [PATCH] proc: /proc/<pid>/timerslack_ns permissions fixes On Wed, Feb 17, 2016 at 9:59 PM, John Stultz <john.stultz@...aro.org> wrote: > 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> Acked-by: Kees Cook <keescook@...omium.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. Probably best to fold them together. -Kees > > 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 > -- Kees Cook Chrome OS & Brillo Security
Powered by blists - more mailing lists