From patchwork Fri Jul 15 17:24:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 72120 Delivered-To: patches@linaro.org Received: by 10.140.29.52 with SMTP id a49csp735118qga; Fri, 15 Jul 2016 10:24:27 -0700 (PDT) X-Received: by 10.66.216.202 with SMTP id os10mr33490263pac.91.1468603467719; Fri, 15 Jul 2016 10:24:27 -0700 (PDT) Return-Path: Received: from mail-pf0-x233.google.com (mail-pf0-x233.google.com. [2607:f8b0:400e:c00::233]) by mx.google.com with ESMTPS id sp8si10125900pab.2.2016.07.15.10.24.27 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 15 Jul 2016 10:24:27 -0700 (PDT) Received-SPF: pass (google.com: domain of john.stultz@linaro.org designates 2607:f8b0:400e:c00::233 as permitted sender) client-ip=2607:f8b0:400e:c00::233; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org; spf=pass (google.com: domain of john.stultz@linaro.org designates 2607:f8b0:400e:c00::233 as permitted sender) smtp.mailfrom=john.stultz@linaro.org; dmarc=pass (p=NONE dis=NONE) header.from=linaro.org Received: by mail-pf0-x233.google.com with SMTP id y134so17133342pfg.0 for ; Fri, 15 Jul 2016 10:24:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=KnVzQIsotrvO7juaVCvvgDhb4s0z8GpvSv8JMMJAwD8=; b=A/yBexmjSzfJVHqQkfT2PsHEQKyx7a1msMspIJJBpQRHcCaAcoGDTf/HsFhTFwJJGt 7R620pLlYpBwSrPRdViZd2DiJmMa/x5ZU1b+hs2plWfRtu4PeK+uh7urPdWoKfKZnlZW 9oavSM1oa4SsX6wIX1Ny/7CRPW8dngH8o60Oo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=KnVzQIsotrvO7juaVCvvgDhb4s0z8GpvSv8JMMJAwD8=; b=lr/vpkqSAa3Nf0uYWnMwC7PAg7beA3v8zd4PeMnpSyeBHYUT88E6diU/Ymv+wD/4vY 0OYrL8XSUMsVihtL00x+5iSH5HbDc1FdZm9cazFYnyAMgyqOyFEVd53+YG2JyUZkOvst arjS7vINSIbwTJk1uBQFBvzT0o4znYyFWopZ9AqUWiSa6zY9qwd2i3k1ca/hTr6uH3tr C338q76zoDumeWNPTbAQS121zzUmIBDQQjh//yQ6SavY7GRANXKoIczH1xh1WMMnfWE3 Cqdl9kfAkjykBxxjmyGbcobQpWuj3WytdqvSWWS2AKaQP3F6rairKQQTqkE/yh3CkYjY wWRw== X-Gm-Message-State: ALyK8tKkmdFNx2XEt1xtt75zDy4R7FBNkk71SR9fj8bGTfwsyVF5DY6yxP9UN4GlW/Eq9u6plfw= X-Received: by 10.98.84.197 with SMTP id i188mr23359528pfb.43.1468603467420; Fri, 15 Jul 2016 10:24:27 -0700 (PDT) Return-Path: Received: from localhost.localdomain (c-73-67-244-238.hsd1.or.comcast.net. [73.67.244.238]) by smtp.gmail.com with ESMTPSA id zk7sm6008954pac.41.2016.07.15.10.24.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 15 Jul 2016 10:24:26 -0700 (PDT) From: John Stultz To: lkml Cc: John Stultz , Kees Cook , "Serge E. Hallyn" , Andrew Morton , Thomas Gleixner , Arjan van de Ven , Oren Laadan , Ruchi Kandoi , Rom Lemarchand , Todd Kjos , Colin Cross , Nick Kralevich , Dmitry Shmidt , Elliott Hughes , Android Kernel Team Subject: [RFC][PATCH 1/2 v2] proc: Relax /proc//timerslack_ns capability requirements Date: Fri, 15 Jul 2016 10:24:19 -0700 Message-Id: <1468603460-17034-1-git-send-email-john.stultz@linaro.org> X-Mailer: git-send-email 1.9.1 When an interface to allow a task to change another tasks timerslack was first proposed, it was suggested that something greater then CAP_SYS_NICE would be needed, as a task could be delayed further then what normally could be done with nice adjustments. So CAP_SYS_PTRACE was adopted instead for what became the /proc//timerslack_ns interface. However, for Android (where this feature originates), giving the system_server CAP_SYS_PTRACE would allow it to observe and modify all tasks memory. This is considered too high a privilege level for only needing to change the timerslack. After some discussion, it was realized that a CAP_SYS_NICE process can set a task as SCHED_FIFO, so they could fork some spinning processes and set them all SCHED_FIFO 99, in effect delaying all other tasks for an infinite amount of time. So as a CAP_SYS_NICE task can already cause trouble for other tasks, using it as a required capability for accessing and modifying /proc//timerslack_ns seems sufficient. Thus, this patch loosens the capability requirements to CAP_SYS_NICE and removes CAP_SYS_PTRACE, simplifying some of the code flow as well. This is technically an ABI change, but as the feature just landed in 4.6, I suspect no one is yet using it. Cc: Kees Cook Cc: "Serge E. Hallyn" Cc: Andrew Morton Cc: Thomas Gleixner CC: Arjan van de Ven Cc: Oren Laadan Cc: Ruchi Kandoi Cc: Rom Lemarchand Cc: Todd Kjos Cc: Colin Cross Cc: Nick Kralevich Cc: Dmitry Shmidt Cc: Elliott Hughes Cc: Android Kernel Team Signed-off-by: John Stultz --- v2: Removed CAP_SYS_PTRACE check and simplified code flow fs/proc/base.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) -- 1.9.1 diff --git a/fs/proc/base.c b/fs/proc/base.c index a11eb71..8f4f8d7 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2277,19 +2277,19 @@ static ssize_t timerslack_ns_write(struct file *file, const char __user *buf, if (err < 0) return err; + if (!capable(CAP_SYS_NICE)) + return -EPERM; + p = get_proc_task(inode); if (!p) 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 = -EPERM; + 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); put_task_struct(p); @@ -2300,22 +2300,21 @@ static int timerslack_ns_show(struct seq_file *m, void *v) { struct inode *inode = m->private; struct task_struct *p; - int err = 0; + + if (!capable(CAP_SYS_NICE)) + return -EPERM; p = get_proc_task(inode); if (!p) return -ESRCH; - 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; + task_lock(p); + seq_printf(m, "%llu\n", p->timer_slack_ns); + task_unlock(p); put_task_struct(p); - return err; + return 0; } static int timerslack_ns_open(struct inode *inode, struct file *filp)