Received-SPF: pass (google.com: domain of linux-kernel+bounces-104878-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240315181032.645161-1-cgzones@googlemail.com> <20240315181032.645161-2-cgzones@googlemail.com>
In-Reply-To: <20240315181032.645161-2-cgzones@googlemail.com>
From: Paul Moore <paul@paul-moore.com>
Date: Fri, 15 Mar 2024 16:22:39 -0400
Message-ID: <CAHC9VhRkawYWQN0UY2R68Qn4pRijpXgu97YOr6XPA7Ls0-zQcA@mail.gmail.com>
Subject: Re: [RFC PATCH 1/2] lsm: introduce new hook security_vm_execstack
To: =?UTF-8?Q?Christian_G=C3=B6ttsche?= <cgzones@googlemail.com>
Cc: linux-security-module@vger.kernel.org, 
	Eric Biederman <ebiederm@xmission.com>, Kees Cook <keescook@chromium.org>, 
	Alexander Viro <viro@zeniv.linux.org.uk>, Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>, 
	James Morris <jmorris@namei.org>, "Serge E. Hallyn" <serge@hallyn.com>, 
	Khadija Kamran <kamrankhadijadj@gmail.com>, Andrii Nakryiko <andrii@kernel.org>, 
	Casey Schaufler <casey@schaufler-ca.com>, Alexei Starovoitov <ast@kernel.org>, 
	Ondrej Mosnacek <omosnace@redhat.com>, Roberto Sassu <roberto.sassu@huawei.com>, 
	Alfred Piccioni <alpic@google.com>, John Johansen <john.johansen@canonical.com>, linux-mm@kvack.org, 
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 15, 2024 at 2:10=E2=80=AFPM Christian G=C3=B6ttsche
<cgzones@googlemail.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=C3=B6ttsche <cgzones@googlemail.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 !=3D vma);
>
>         if (unlikely(vm_flags & VM_EXEC)) {
> +               ret =3D security_vm_execstack();
> +               if (ret)
> +                       goto out_unlock;
> +
>                 pr_warn_once("process '%pD4' started with executable stac=
k\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?

--=20
paul-moore.com