From patchwork Thu Jul 21 20:24:26 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 72587 Delivered-To: patches@linaro.org Received: by 10.140.29.52 with SMTP id a49csp639663qga; Thu, 21 Jul 2016 13:24:31 -0700 (PDT) X-Received: by 10.66.155.129 with SMTP id vw1mr223845pab.148.1469132671520; Thu, 21 Jul 2016 13:24:31 -0700 (PDT) Return-Path: Received: from mail-pa0-x231.google.com (mail-pa0-x231.google.com. [2607:f8b0:400e:c03::231]) by mx.google.com with ESMTPS id v88si11483433pfj.110.2016.07.21.13.24.31 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 21 Jul 2016 13:24:31 -0700 (PDT) Received-SPF: pass (google.com: domain of john.stultz@linaro.org designates 2607:f8b0:400e:c03::231 as permitted sender) client-ip=2607:f8b0:400e:c03::231; 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:c03::231 as permitted sender) smtp.mailfrom=john.stultz@linaro.org; dmarc=pass (p=NONE dis=NONE) header.from=linaro.org Received: by mail-pa0-x231.google.com with SMTP id iw10so32092677pac.2 for ; Thu, 21 Jul 2016 13:24:31 -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=5Bf460f25mknctP44CD1falLw3plHFCVsxPpknaGyMA=; b=K0ZMY4zu1xOpUmI0FKB70+QB4KAXqzJxZRF++dIn9h4KeHkC2JzJ91eSOxguSPg9TN I+vvU2bfkgXB5Q+J5JfyvHR9WHWy19qtHKdPtHNHS/em8mrL1/y9kZRxN1d6wTh+L7mL TSm09TpCozplHaaCBLZ5btK4wNiGYHQXBzrkI= 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=5Bf460f25mknctP44CD1falLw3plHFCVsxPpknaGyMA=; b=aCvZS+FqEhtxbwyj69fL6CrERbtrkFmQ1UTwk49QWqd0uMq5mDf2J+2nbrvo5yGgFK smCgPSFIpySs9MHYsmnZAVWeTJpqbct22rzSJ77DjLvKSI7KTtoVGSbPJ5xtrdNjc02Q TmES1dhBqvePxunDJqRrj85c10Rr6fn3VF6z6gkVxdXqfjwqbBJUAeCbDojsPXHyG3ju TRMHwts/TDJuZXfcwXnXh7DJ2Ld7/D6/mm3JmWXsBsIVhZ6fSLupgdcOIio835zYCVuK xnXPw6BHE/LqikugLvPmEkVdsBbB5h7NsHe7XIfWlEVOddvmj50KdhsV0iIQClZes/il KRZQ== X-Gm-Message-State: AEkoouux956MwAIrcaFgULohdczawpHxxxqAP3kKuo4fn21V+NbYzjmVA1ajOsgV1EV0dbGSczA= X-Received: by 10.66.51.137 with SMTP id k9mr213349pao.49.1469132670899; Thu, 21 Jul 2016 13:24:30 -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 d185sm14277920pfd.80.2016.07.21.13.24.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 21 Jul 2016 13:24:29 -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: [PATCH 1/2 v4] proc: Relax /proc//timerslack_ns capability requirements Date: Thu, 21 Jul 2016 13:24:26 -0700 Message-Id: <1469132667-17377-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 Reviewed-by: Nick Kralevich Acked-by: Serge Hallyn Acked-by: Kees Cook Signed-off-by: John Stultz --- v2: Removed CAP_SYS_PTRACE check and simplified code flow v3: Tweaked where CAP_SYS_NICE check is made, suggeded by NickK v4: No changes fs/proc/base.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) -- 1.9.1 diff --git a/fs/proc/base.c b/fs/proc/base.c index a11eb71..c94abae 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2281,16 +2281,19 @@ static ssize_t timerslack_ns_write(struct file *file, const char __user *buf, 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 + if (!capable(CAP_SYS_NICE)) { count = -EPERM; + goto out; + } + + 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); +out: put_task_struct(p); return count; @@ -2300,19 +2303,22 @@ static int timerslack_ns_show(struct seq_file *m, void *v) { struct inode *inode = m->private; struct task_struct *p; - int err = 0; + int err = 0; 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 + if (!capable(CAP_SYS_NICE)) { err = -EPERM; + goto out; + } + task_lock(p); + seq_printf(m, "%llu\n", p->timer_slack_ns); + task_unlock(p); + +out: put_task_struct(p); return err;