From patchwork Mon Jul 18 20:11:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 72256 Delivered-To: patches@linaro.org Received: by 10.140.29.52 with SMTP id a49csp299405qga; Mon, 18 Jul 2016 13:11:15 -0700 (PDT) X-Received: by 10.66.236.9 with SMTP id uq9mr50857605pac.145.1468872675419; Mon, 18 Jul 2016 13:11:15 -0700 (PDT) Return-Path: Received: from mail-pf0-x230.google.com (mail-pf0-x230.google.com. [2607:f8b0:400e:c00::230]) by mx.google.com with ESMTPS id t20si5930344pal.236.2016.07.18.13.11.15 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 18 Jul 2016 13:11:15 -0700 (PDT) Received-SPF: pass (google.com: domain of john.stultz@linaro.org designates 2607:f8b0:400e:c00::230 as permitted sender) client-ip=2607:f8b0:400e:c00::230; 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::230 as permitted sender) smtp.mailfrom=john.stultz@linaro.org; dmarc=pass (p=NONE dis=NONE) header.from=linaro.org Received: by mail-pf0-x230.google.com with SMTP id h186so17654791pfg.3 for ; Mon, 18 Jul 2016 13:11:15 -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=5qkgiWuiApUz70ilUivVht+1ImPSOdHanAH6RxN/xM8=; b=TAagKyn1qyCLcaT3/0gsc3S9T+qcJXMDg7/OX1VA4JZyjGi0nKr29yyvQD37o36jIg hXqztwUB71I/mamDQH1WXpoAqivN+EKsLDi6JNvzoBHL/Var8muiAUrwDbz39oh3mLQi K8qPlfRNPzODYrqjhoLuaZAKwwbHfaYXU9XBU= 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=5qkgiWuiApUz70ilUivVht+1ImPSOdHanAH6RxN/xM8=; b=ASeDmA4nGtsBp+fNrT9+AscBHSOCcM5Xk65NseP718KvP71BlARCAxMQ01+2uGbHy5 U3pb6qyrvjbVixF8r9ItbT2pvwC4Dry4qUgdV8yfV0i/R8a1KvrYrHC7uUouNFMoQc85 rh1X3yDuuEhhjm2g7wKKOOH6FKK7Tg/DRaZtH18kNzS4BbEmiJayrPwRCoqbpVIv4XmS sUFNF19XwEmMnfUZd0Ad7U2JXtezDW/4zPCAVi9JdPmg8KXQoPbKsstfQZ4XBAYFXVTZ uegtOPK85eVpjaIJALLdBQW1CfmXyuhc1ddDq9P7Xozv4NP9viMS21ejFDnYr9kLcjxM scSg== X-Gm-Message-State: ALyK8tLsHsswpETQpVWsuZbTbAB458Qk7QTZvKzadu2jJ4RqSLUDvVB7r5a3pzc6A0cE+WdZY6w= X-Received: by 10.98.152.6 with SMTP id q6mr48824346pfd.86.1468872675141; Mon, 18 Jul 2016 13:11:15 -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 132sm6970435pfu.6.2016.07.18.13.11.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 18 Jul 2016 13:11:14 -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 v3] proc: Relax /proc//timerslack_ns capability requirements Date: Mon, 18 Jul 2016 13:11:10 -0700 Message-Id: <1468872671-9002-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 v3: Tweaked where CAP_SYS_NICE check is made, suggeded by NickK 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;