linux-kernel - Re: [RFC][PATCH 1/2 v2] proc: Relax /proc/<tid>/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]

Date:	Fri, 15 Jul 2016 11:42:36 -0700
From:	John Stultz <john.stultz@...aro.org>
To:	Nick Kralevich <nnk@...gle.com>
Cc:	lkml <linux-kernel@...r.kernel.org>,
	Kees Cook <keescook@...omium.org>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Oren Laadan <orenl@...lrox.com>,
	Ruchi Kandoi <kandoiruchi@...gle.com>,
	Rom Lemarchand <romlem@...roid.com>,
	Todd Kjos <tkjos@...gle.com>, Colin Cross <ccross@...roid.com>,
	Dmitry Shmidt <dimitrysh@...gle.com>,
	Elliott Hughes <enh@...gle.com>,
	Android Kernel Team <kernel-team@...roid.com>,
	LSM List <linux-security-module@...r.kernel.org>,
	SELinux <selinux@...ho.nsa.gov>
Subject: Re: [RFC][PATCH 1/2 v2] proc: Relax /proc/<tid>/timerslack_ns
 capability requirements

On Fri, Jul 15, 2016 at 10:51 AM, Nick Kralevich <nnk@...gle.com> wrote:
> On Fri, Jul 15, 2016 at 10:24 AM, John Stultz <john.stultz@...aro.org> wrote:
>> +       if (!capable(CAP_SYS_NICE))
>> +               return -EPERM;
>> +
>
> Since you're going the LSM route (from your second patch of this

Well, you suggested it, so I sent out an RFC. I'm not married to it yet. :)


> series), the capability check above should be moved to the LSM hook in
> security/commoncap.c.  Only one security call to
> security_task_settimerslack is needed, which will cover the standard
> capabilities check as well as the SELinux check.

Huh. Ok. I was looking at the implementation of nice(), which does:

 if (increment < 0 && !can_nice(current, nice))
                return -EPERM;
retval = security_task_setnice(current, nice);
if (retval)
...

Which made it seem like standard checks are done first, then finer
grain lsm checks second.

(...and now you can guess where my accidental "current" usage in the
next patch came from :)


>
>>         p = get_proc_task(inode);
>>         if (!p)
>>                 return -ESRCH;
>>
>
> Per your patch #2, you'd call security_task_settimerslack here. This
> would call into the capability LSM hook you added in
> security/commoncap.c

Though I was hoping to keep the CAP_SYS_PTRACE -> CAP_SYS_NICE change
first, then add the LSM hooks, as it makes the needed ABI change more
obvious. I worry swapping it around with the LSM hook being added
first makes it significantly less obvious, as (at least for me) the
security_task_* functions get indirect and difficult to follow quickly
("wait, why are we checking SETSCHED for nice?").

A side curiosity: why does "git grep PROCESS__SETSCHED" miss the
definition? Is the av_permissions.h file somehow caught by .gitignore?

thanks
-john