From: Andy Lutomirski <luto@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>,
Djalal Harouni <tixxdz@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
"Serge E. Hallyn" <serge@hallyn.com>,
"kernel-hardening@lists.openwall.com"
<kernel-hardening@lists.openwall.com>,
LSM List <linux-security-module@vger.kernel.org>,
Linux API <linux-api@vger.kernel.org>,
Dongsu Park <dpark@posteo.net>,
Casey Schaufler <casey@schaufler-ca.com>,
James Morris <james.l.morris@oracle.com>,
Paul Moore <paul@paul-moore.com>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jonathan Corbet <corbet@lwn.net>, Jessica Yu <jeyu@redhat.com>,
Rusty Russell <rusty@rustcorp.com.au>,
Arnaldo Carvalho de Melo <acme@redhat.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Ingo Molnar <mingo@kernel.org>,
belakhdar abdeldjalil <zendyani@gmail.com>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH v3 2/2] modules:capabilities: add a per-task modules autoload restriction
Date: Wed, 19 Apr 2017 19:41:44 -0700 [thread overview]
Message-ID: <CALCETrUueOx1tqj+Ru93KGpy2HHR-A_GQ6DrAppiomkPTtX7Lw@mail.gmail.com> (raw)
In-Reply-To: <CAGXu5jJ5dm8yJJ34tEyOP78OxUr2awXmNhm6RaVP8jE4q+L5Ng@mail.gmail.com>
On Wed, Apr 19, 2017 at 4:43 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Apr 19, 2017 at 4:15 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> On Wed, Apr 19, 2017 at 3:20 PM, Djalal Harouni <tixxdz@gmail.com> wrote:
>>> +/* Sets task's modules_autoload */
>>> +static inline int task_set_modules_autoload(struct task_struct *task,
>>> + unsigned long value)
>>> +{
>>> + if (value > MODULES_AUTOLOAD_DISABLED)
>>> + return -EINVAL;
>>> + else if (task->modules_autoload > value)
>>> + return -EPERM;
>>> + else if (task->modules_autoload < value)
>>> + task->modules_autoload = value;
>>> +
>>> + return 0;
>>> +}
>>
>> This needs to be more locked down. Otherwise someone could set this
>> and then run a setuid program. Admittedly, it would be quite odd if
>> this particular thing causes a problem, but the issue exists
>> nonetheless.
>
> Eeeh, I don't agree this needs to be changed. APIs provided by modules
> are different than the existing privilege-manipulation syscalls this
> concern stems from. Applications are already forced to deal with
> things being missing like this in the face of it simply not being
> built into the kernel.
>
> Having to hide this behind nnp seems like it'd reduce its utility...
>
I think that adding an inherited boolean to task_struct that can be
set by unprivileged tasks and passed to privileged tasks is a terrible
precedent. Ideally someone would try to find all the existing things
like this and kill them off.
I agree that I don't see how one would exploit this particular
feature, but I still think I dislike the approach. This is a slippery
slope to adding a boolean for perf_event_open(), unshare(), etc, and
we should solve these for real rather than half-arsing them IMO.
WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>,
Djalal Harouni <tixxdz@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
"Serge E. Hallyn" <serge@hallyn.com>,
"kernel-hardening@lists.openwall.com"
<kernel-hardening@lists.openwall.com>,
LSM List <linux-security-module@vger.kernel.org>,
Linux API <linux-api@vger.kernel.org>,
Dongsu Park <dpark@posteo.net>,
Casey Schaufler <casey@schaufler-ca.com>,
James Morris <james.l.morris@oracle.com>,
Paul Moore <paul@paul-moore.com>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jonathan Corbet <corbet@lwn.net>, Jessica Yu <jeyu@redhat.com>,
Rusty Russell <rusty@rustcorp.com.au>,
Arnaldo Carvalho de Melo <acme@>
Subject: Re: [PATCH v3 2/2] modules:capabilities: add a per-task modules autoload restriction
Date: Wed, 19 Apr 2017 19:41:44 -0700 [thread overview]
Message-ID: <CALCETrUueOx1tqj+Ru93KGpy2HHR-A_GQ6DrAppiomkPTtX7Lw@mail.gmail.com> (raw)
In-Reply-To: <CAGXu5jJ5dm8yJJ34tEyOP78OxUr2awXmNhm6RaVP8jE4q+L5Ng@mail.gmail.com>
On Wed, Apr 19, 2017 at 4:43 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Apr 19, 2017 at 4:15 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> On Wed, Apr 19, 2017 at 3:20 PM, Djalal Harouni <tixxdz@gmail.com> wrote:
>>> +/* Sets task's modules_autoload */
>>> +static inline int task_set_modules_autoload(struct task_struct *task,
>>> + unsigned long value)
>>> +{
>>> + if (value > MODULES_AUTOLOAD_DISABLED)
>>> + return -EINVAL;
>>> + else if (task->modules_autoload > value)
>>> + return -EPERM;
>>> + else if (task->modules_autoload < value)
>>> + task->modules_autoload = value;
>>> +
>>> + return 0;
>>> +}
>>
>> This needs to be more locked down. Otherwise someone could set this
>> and then run a setuid program. Admittedly, it would be quite odd if
>> this particular thing causes a problem, but the issue exists
>> nonetheless.
>
> Eeeh, I don't agree this needs to be changed. APIs provided by modules
> are different than the existing privilege-manipulation syscalls this
> concern stems from. Applications are already forced to deal with
> things being missing like this in the face of it simply not being
> built into the kernel.
>
> Having to hide this behind nnp seems like it'd reduce its utility...
>
I think that adding an inherited boolean to task_struct that can be
set by unprivileged tasks and passed to privileged tasks is a terrible
precedent. Ideally someone would try to find all the existing things
like this and kill them off.
I agree that I don't see how one would exploit this particular
feature, but I still think I dislike the approach. This is a slippery
slope to adding a boolean for perf_event_open(), unshare(), etc, and
we should solve these for real rather than half-arsing them IMO.
WARNING: multiple messages have this Message-ID (diff)
From: luto@kernel.org (Andy Lutomirski)
To: linux-security-module@vger.kernel.org
Subject: [PATCH v3 2/2] modules:capabilities: add a per-task modules autoload restriction
Date: Wed, 19 Apr 2017 19:41:44 -0700 [thread overview]
Message-ID: <CALCETrUueOx1tqj+Ru93KGpy2HHR-A_GQ6DrAppiomkPTtX7Lw@mail.gmail.com> (raw)
In-Reply-To: <CAGXu5jJ5dm8yJJ34tEyOP78OxUr2awXmNhm6RaVP8jE4q+L5Ng@mail.gmail.com>
On Wed, Apr 19, 2017 at 4:43 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Apr 19, 2017 at 4:15 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> On Wed, Apr 19, 2017 at 3:20 PM, Djalal Harouni <tixxdz@gmail.com> wrote:
>>> +/* Sets task's modules_autoload */
>>> +static inline int task_set_modules_autoload(struct task_struct *task,
>>> + unsigned long value)
>>> +{
>>> + if (value > MODULES_AUTOLOAD_DISABLED)
>>> + return -EINVAL;
>>> + else if (task->modules_autoload > value)
>>> + return -EPERM;
>>> + else if (task->modules_autoload < value)
>>> + task->modules_autoload = value;
>>> +
>>> + return 0;
>>> +}
>>
>> This needs to be more locked down. Otherwise someone could set this
>> and then run a setuid program. Admittedly, it would be quite odd if
>> this particular thing causes a problem, but the issue exists
>> nonetheless.
>
> Eeeh, I don't agree this needs to be changed. APIs provided by modules
> are different than the existing privilege-manipulation syscalls this
> concern stems from. Applications are already forced to deal with
> things being missing like this in the face of it simply not being
> built into the kernel.
>
> Having to hide this behind nnp seems like it'd reduce its utility...
>
I think that adding an inherited boolean to task_struct that can be
set by unprivileged tasks and passed to privileged tasks is a terrible
precedent. Ideally someone would try to find all the existing things
like this and kill them off.
I agree that I don't see how one would exploit this particular
feature, but I still think I dislike the approach. This is a slippery
slope to adding a boolean for perf_event_open(), unshare(), etc, and
we should solve these for real rather than half-arsing them IMO.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>,
Djalal Harouni <tixxdz@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
"Serge E. Hallyn" <serge@hallyn.com>,
"kernel-hardening@lists.openwall.com"
<kernel-hardening@lists.openwall.com>,
LSM List <linux-security-module@vger.kernel.org>,
Linux API <linux-api@vger.kernel.org>,
Dongsu Park <dpark@posteo.net>,
Casey Schaufler <casey@schaufler-ca.com>,
James Morris <james.l.morris@oracle.com>,
Paul Moore <paul@paul-moore.com>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jonathan Corbet <corbet@lwn.net>, Jessica Yu <jeyu@redhat.com>,
Rusty Russell <rusty@rustcorp.com.au>,
Arnaldo Carvalho de Melo <acme@redhat.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Ingo Molnar <mingo@kernel.org>,
belakhdar abdeldjalil <zendyani@gmail.com>,
Peter Zijlstra <peterz@infradead.org>
Subject: [kernel-hardening] Re: [PATCH v3 2/2] modules:capabilities: add a per-task modules autoload restriction
Date: Wed, 19 Apr 2017 19:41:44 -0700 [thread overview]
Message-ID: <CALCETrUueOx1tqj+Ru93KGpy2HHR-A_GQ6DrAppiomkPTtX7Lw@mail.gmail.com> (raw)
In-Reply-To: <CAGXu5jJ5dm8yJJ34tEyOP78OxUr2awXmNhm6RaVP8jE4q+L5Ng@mail.gmail.com>
On Wed, Apr 19, 2017 at 4:43 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Apr 19, 2017 at 4:15 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> On Wed, Apr 19, 2017 at 3:20 PM, Djalal Harouni <tixxdz@gmail.com> wrote:
>>> +/* Sets task's modules_autoload */
>>> +static inline int task_set_modules_autoload(struct task_struct *task,
>>> + unsigned long value)
>>> +{
>>> + if (value > MODULES_AUTOLOAD_DISABLED)
>>> + return -EINVAL;
>>> + else if (task->modules_autoload > value)
>>> + return -EPERM;
>>> + else if (task->modules_autoload < value)
>>> + task->modules_autoload = value;
>>> +
>>> + return 0;
>>> +}
>>
>> This needs to be more locked down. Otherwise someone could set this
>> and then run a setuid program. Admittedly, it would be quite odd if
>> this particular thing causes a problem, but the issue exists
>> nonetheless.
>
> Eeeh, I don't agree this needs to be changed. APIs provided by modules
> are different than the existing privilege-manipulation syscalls this
> concern stems from. Applications are already forced to deal with
> things being missing like this in the face of it simply not being
> built into the kernel.
>
> Having to hide this behind nnp seems like it'd reduce its utility...
>
I think that adding an inherited boolean to task_struct that can be
set by unprivileged tasks and passed to privileged tasks is a terrible
precedent. Ideally someone would try to find all the existing things
like this and kill them off.
I agree that I don't see how one would exploit this particular
feature, but I still think I dislike the approach. This is a slippery
slope to adding a boolean for perf_event_open(), unshare(), etc, and
we should solve these for real rather than half-arsing them IMO.
next prev parent reply other threads:[~2017-04-20 2:42 UTC|newest]
Thread overview: 147+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-19 22:20 [PATCH v3 0/2] modules:capabilities: automatic module loading restrictions Djalal Harouni
2017-04-19 22:20 ` [kernel-hardening] " Djalal Harouni
2017-04-19 22:20 ` Djalal Harouni
2017-04-19 22:20 ` Djalal Harouni
2017-04-19 22:20 ` [PATCH v3 1/2] modules:capabilities: automatic module loading restriction Djalal Harouni
2017-04-19 22:20 ` [kernel-hardening] " Djalal Harouni
2017-04-19 22:20 ` Djalal Harouni
2017-04-19 22:20 ` Djalal Harouni
2017-04-19 23:16 ` Andy Lutomirski
2017-04-19 23:16 ` [kernel-hardening] " Andy Lutomirski
2017-04-19 23:16 ` Andy Lutomirski
2017-04-19 23:16 ` Andy Lutomirski
2017-04-20 2:22 ` Ben Hutchings
2017-04-20 2:22 ` [kernel-hardening] " Ben Hutchings
2017-04-20 2:22 ` Ben Hutchings
2017-04-20 12:44 ` [kernel-hardening] " Djalal Harouni
2017-04-20 12:44 ` Djalal Harouni
2017-04-20 12:44 ` Djalal Harouni
2017-04-20 15:02 ` Ben Hutchings
2017-04-20 15:02 ` Ben Hutchings
2017-04-20 15:02 ` Ben Hutchings
2017-04-20 20:39 ` [kernel-hardening] " Djalal Harouni
2017-04-20 20:39 ` Djalal Harouni
2017-04-20 20:39 ` Djalal Harouni
2017-04-20 21:28 ` Kees Cook
2017-04-20 21:28 ` Kees Cook
2017-04-20 21:28 ` Kees Cook
2017-04-20 21:28 ` Kees Cook
2017-04-19 22:20 ` [PATCH v3 2/2] modules:capabilities: add a per-task modules autoload restriction Djalal Harouni
2017-04-19 22:20 ` [kernel-hardening] " Djalal Harouni
2017-04-19 22:20 ` Djalal Harouni
2017-04-19 22:20 ` Djalal Harouni
2017-04-19 22:38 ` Djalal Harouni
2017-04-19 22:38 ` [kernel-hardening] " Djalal Harouni
2017-04-19 22:38 ` Djalal Harouni
2017-04-19 22:38 ` Djalal Harouni
2017-04-19 23:15 ` Andy Lutomirski
2017-04-19 23:15 ` [kernel-hardening] " Andy Lutomirski
2017-04-19 23:15 ` Andy Lutomirski
2017-04-19 23:15 ` Andy Lutomirski
2017-04-19 23:43 ` Kees Cook
2017-04-19 23:43 ` [kernel-hardening] " Kees Cook
2017-04-19 23:43 ` Kees Cook
2017-04-19 23:43 ` Kees Cook
2017-04-20 2:41 ` Andy Lutomirski [this message]
2017-04-20 2:41 ` [kernel-hardening] " Andy Lutomirski
2017-04-20 2:41 ` Andy Lutomirski
2017-04-20 2:41 ` Andy Lutomirski
2017-04-21 23:19 ` Kees Cook
2017-04-21 23:19 ` [kernel-hardening] " Kees Cook
2017-04-21 23:19 ` Kees Cook
2017-04-21 23:19 ` Kees Cook
2017-04-21 23:28 ` Andy Lutomirski
2017-04-21 23:28 ` [kernel-hardening] " Andy Lutomirski
2017-04-21 23:28 ` Andy Lutomirski
2017-04-21 23:28 ` Andy Lutomirski
2017-04-21 23:40 ` Kees Cook
2017-04-21 23:40 ` [kernel-hardening] " Kees Cook
2017-04-21 23:40 ` Kees Cook
2017-04-21 23:40 ` Kees Cook
2017-04-21 23:51 ` Andy Lutomirski
2017-04-21 23:51 ` [kernel-hardening] " Andy Lutomirski
2017-04-21 23:51 ` Andy Lutomirski
2017-04-21 23:51 ` Andy Lutomirski
2017-04-22 0:12 ` Djalal Harouni
2017-04-22 0:12 ` [kernel-hardening] " Djalal Harouni
2017-04-22 0:12 ` Djalal Harouni
2017-04-22 0:12 ` Djalal Harouni
2017-04-22 1:19 ` Djalal Harouni
2017-04-22 1:19 ` [kernel-hardening] " Djalal Harouni
2017-04-22 1:19 ` Djalal Harouni
2017-04-22 1:19 ` Djalal Harouni
2017-04-22 6:51 ` Andy Lutomirski
2017-04-22 6:51 ` [kernel-hardening] " Andy Lutomirski
2017-04-22 6:51 ` Andy Lutomirski
2017-04-22 6:51 ` Andy Lutomirski
2017-04-22 19:29 ` Kees Cook
2017-04-22 19:29 ` [kernel-hardening] " Kees Cook
2017-04-22 19:29 ` Kees Cook
2017-04-22 19:29 ` Kees Cook
2017-04-24 14:25 ` Djalal Harouni
2017-04-24 14:25 ` [kernel-hardening] " Djalal Harouni
2017-04-24 14:25 ` Djalal Harouni
2017-04-24 14:25 ` Djalal Harouni
2017-04-24 18:02 ` Kees Cook
2017-04-24 18:02 ` [kernel-hardening] " Kees Cook
2017-04-24 18:02 ` Kees Cook
2017-04-24 18:02 ` Kees Cook
2017-04-24 18:35 ` Djalal Harouni
2017-04-24 18:35 ` [kernel-hardening] " Djalal Harouni
2017-04-24 18:35 ` Djalal Harouni
2017-04-24 18:35 ` Djalal Harouni
2017-04-21 23:52 ` Casey Schaufler
2017-04-21 23:52 ` [kernel-hardening] " Casey Schaufler
2017-04-21 23:52 ` Casey Schaufler
2017-04-21 23:52 ` Casey Schaufler
2017-04-22 0:00 ` Andy Lutomirski
2017-04-22 0:00 ` [kernel-hardening] " Andy Lutomirski
2017-04-22 0:00 ` Andy Lutomirski
2017-04-22 0:00 ` Andy Lutomirski
2017-04-22 0:13 ` Casey Schaufler
2017-04-22 0:13 ` [kernel-hardening] " Casey Schaufler
2017-04-22 0:13 ` Casey Schaufler
2017-04-22 0:13 ` Casey Schaufler
2017-04-22 6:45 ` Andy Lutomirski
2017-04-22 6:45 ` [kernel-hardening] " Andy Lutomirski
2017-04-22 6:45 ` Andy Lutomirski
2017-04-22 6:45 ` Andy Lutomirski
2017-04-22 12:17 ` Djalal Harouni
2017-04-22 12:17 ` [kernel-hardening] " Djalal Harouni
2017-04-22 12:17 ` Djalal Harouni
2017-04-22 12:17 ` Djalal Harouni
2017-05-04 13:07 ` Djalal Harouni
2017-05-04 13:07 ` [kernel-hardening] " Djalal Harouni
2017-05-04 13:07 ` Djalal Harouni
2017-05-04 13:07 ` Djalal Harouni
2017-05-04 14:58 ` Serge E. Hallyn
2017-05-04 14:58 ` [kernel-hardening] " Serge E. Hallyn
2017-05-04 14:58 ` Serge E. Hallyn
2017-05-04 14:58 ` Serge E. Hallyn
2017-05-05 13:06 ` Djalal Harouni
2017-05-05 13:06 ` [kernel-hardening] " Djalal Harouni
2017-05-05 13:06 ` Djalal Harouni
2017-05-05 13:06 ` Djalal Harouni
2017-05-05 16:18 ` Andy Lutomirski
2017-05-05 16:18 ` [kernel-hardening] " Andy Lutomirski
2017-05-05 16:18 ` Andy Lutomirski
2017-05-05 16:18 ` Andy Lutomirski
2017-04-20 1:57 ` kbuild test robot
2017-04-20 1:57 ` [kernel-hardening] " kbuild test robot
2017-04-20 1:57 ` kbuild test robot
2017-04-20 1:57 ` kbuild test robot
2017-04-24 4:29 ` Rusty Russell
2017-04-24 4:29 ` [kernel-hardening] " Rusty Russell
2017-04-24 4:29 ` Rusty Russell
2017-04-24 4:29 ` Rusty Russell
2017-04-26 9:06 ` Djalal Harouni
2017-04-26 9:06 ` [kernel-hardening] " Djalal Harouni
2017-04-26 9:06 ` Djalal Harouni
2017-04-27 2:07 ` Rusty Russell
2017-04-27 2:07 ` [kernel-hardening] " Rusty Russell
2017-04-27 2:07 ` Rusty Russell
2017-04-27 2:07 ` Rusty Russell
2017-04-27 13:16 ` Djalal Harouni
2017-04-27 13:16 ` [kernel-hardening] " Djalal Harouni
2017-04-27 13:16 ` Djalal Harouni
2017-04-27 13:16 ` Djalal Harouni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALCETrUueOx1tqj+Ru93KGpy2HHR-A_GQ6DrAppiomkPTtX7Lw@mail.gmail.com \
--to=luto@kernel.org \
--cc=acme@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=casey@schaufler-ca.com \
--cc=corbet@lwn.net \
--cc=dpark@posteo.net \
--cc=gregkh@linuxfoundation.org \
--cc=james.l.morris@oracle.com \
--cc=jeyu@redhat.com \
--cc=keescook@chromium.org \
--cc=kernel-hardening@lists.openwall.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=mingo@kernel.org \
--cc=paul@paul-moore.com \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=peterz@infradead.org \
--cc=rusty@rustcorp.com.au \
--cc=serge@hallyn.com \
--cc=tixxdz@gmail.com \
--cc=zendyani@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.