linux-kernel - Re: [RFC PATCH 1/2] lsm: introduce new hook security_vm

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] [day] [month] [year] [list]

Message-ID: <CAHC9VhTpTbVL4=tchR3Bpcfe0Hsijf5XJ6wsEvU7cu8eUy_iDA@mail.gmail.com>
Date: Tue, 19 Mar 2024 19:10:02 -0400
From: Paul Moore <paul@...l-moore.com>
To: Kees Cook <kees@...nel.org>
Cc: Christian Göttsche <cgzones@...glemail.com>, 
	linux-security-module@...r.kernel.org, Eric Biederman <ebiederm@...ssion.com>, 
	Kees Cook <keescook@...omium.org>, Alexander Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, James Morris <jmorris@...ei.org>, 
	"Serge E. Hallyn" <serge@...lyn.com>, Khadija Kamran <kamrankhadijadj@...il.com>, 
	Andrii Nakryiko <andrii@...nel.org>, Casey Schaufler <casey@...aufler-ca.com>, 
	Alexei Starovoitov <ast@...nel.org>, Ondrej Mosnacek <omosnace@...hat.com>, 
	Roberto Sassu <roberto.sassu@...wei.com>, Alfred Piccioni <alpic@...gle.com>, 
	John Johansen <john.johansen@...onical.com>, linux-mm@...ck.org, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/2] lsm: introduce new hook security_vm_execstack

On Fri, Mar 15, 2024 at 11:24 PM Kees Cook <kees@...nel.org> wrote:
> On March 15, 2024 1:22:39 PM PDT, Paul Moore <paul@...l-moore.com> wrote:
> >On Fri, Mar 15, 2024 at 2:10 PM Christian Göttsche
> ><cgzones@...glemail.com> wrote:
> >>
> >> Add a new hook guarding instantiations of programs with executable
> >> stack.  They are being warned about since commit 47a2ebb7f505 ("execve:
> >> warn if process starts with executable stack").  Lets give LSMs the
> >> ability to control their presence on a per application basis.
> >>
> >> Signed-off-by: Christian Göttsche <cgzones@...glemail.com>
> >> ---
> >>  fs/exec.c                     |  4 ++++
> >>  include/linux/lsm_hook_defs.h |  1 +
> >>  include/linux/security.h      |  6 ++++++
> >>  security/security.c           | 13 +++++++++++++
> >>  4 files changed, 24 insertions(+)
> >
> >Looking at the commit referenced above, I'm guessing the existing
> >security_file_mprotect() hook doesn't catch this?
> >
> >> diff --git a/fs/exec.c b/fs/exec.c
> >> index 8cdd5b2dd09c..e6f9e980c6b1 100644
> >> --- a/fs/exec.c
> >> +++ b/fs/exec.c
> >> @@ -829,6 +829,10 @@ int setup_arg_pages(struct linux_binprm *bprm,
> >>         BUG_ON(prev != vma);
> >>
> >>         if (unlikely(vm_flags & VM_EXEC)) {
> >> +               ret = security_vm_execstack();
> >> +               if (ret)
> >> +                       goto out_unlock;
> >> +
> >>                 pr_warn_once("process '%pD4' started with executable stack\n",
> >>                              bprm->file);
> >>         }
> >
> >Instead of creating a new LSM hook, have you considered calling the
> >existing security_file_mprotect() hook?  The existing LSM controls
> >there may not be a great fit in this case, but I'd like to hear if
> >you've tried that, and if you have, what made you decide a new hook
> >was the better option?
>
> Also, can't MDWE handle this already?
> https://git.kernel.org/linus/b507808ebce23561d4ff8c2aa1fb949fe402bc61

It looks like it, but that doesn't mean there isn't also value in an
associated LSM hook as the LSM hook would admins and security policy
developers/analysts to incorporate this as part of the system's
security policy.  It's great that we have all of these cool knobs that
we can play with independent of each other, but sometimes you really
want a single unified security policy that you can look at, analyze,
and reason about.

Regardless, my previous comments still stand, I'd like to hear
verification that the existing security_file_mprotect() hook is not
sufficient, and if its current placement is lacking, why calling it
from a second location wasn't practical and required the creation of a
new LSM hook.

-- 
paul-moore.com