* Re: [PATCH v5 04/12] S.A.R.A.: generic DFA for string matching
From: Jann Horn @ 2019-07-06 18:32 UTC (permalink / raw)
To: Salvatore Mesoraca
Cc: kernel list, Kernel Hardening, Linux-MM, linux-security-module,
Alexander Viro, Brad Spengler, Casey Schaufler, Christoph Hellwig,
James Morris, Kees Cook, PaX Team, Serge E. Hallyn,
Thomas Gleixner
In-Reply-To: <1562410493-8661-5-git-send-email-s.mesoraca16@gmail.com>
On Sat, Jul 6, 2019 at 12:55 PM Salvatore Mesoraca
<s.mesoraca16@gmail.com> wrote:
> Creation of a generic Discrete Finite Automata implementation
> for string matching. The transition tables have to be produced
> in user-space.
> This allows us to possibly support advanced string matching
> patterns like regular expressions, but they need to be supported
> by user-space tools.
AppArmor already has a DFA implementation that takes a DFA machine
from userspace and runs it against file paths; see e.g.
aa_dfa_match(). Did you look into whether you could move their DFA to
some place like lib/ and reuse it instead of adding yet another
generic rule interface to the kernel?
[...]
> +++ b/security/sara/dfa.c
> @@ -0,0 +1,335 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * S.A.R.A. Linux Security Module
> + *
> + * Copyright (C) 2017 Salvatore Mesoraca <s.mesoraca16@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
Throughout the series, you are adding files that both add an SPDX
identifier and have a description of the license in the comment block
at the top. The SPDX identifier already identifies the license.
^ permalink raw reply
* Re: [PATCH v5 06/12] S.A.R.A.: WX protection
From: Al Viro @ 2019-07-06 19:28 UTC (permalink / raw)
To: Salvatore Mesoraca
Cc: linux-kernel, kernel-hardening, linux-mm, linux-security-module,
Brad Spengler, Casey Schaufler, Christoph Hellwig, James Morris,
Jann Horn, Kees Cook, PaX Team, Serge E. Hallyn, Thomas Gleixner
In-Reply-To: <1562410493-8661-7-git-send-email-s.mesoraca16@gmail.com>
On Sat, Jul 06, 2019 at 12:54:47PM +0200, Salvatore Mesoraca wrote:
> +#define sara_warn_or_return(err, msg) do { \
> + if ((sara_wxp_flags & SARA_WXP_VERBOSE)) \
> + pr_wxp(msg); \
> + if (!(sara_wxp_flags & SARA_WXP_COMPLAIN)) \
> + return -err; \
> +} while (0)
> +
> +#define sara_warn_or_goto(label, msg) do { \
> + if ((sara_wxp_flags & SARA_WXP_VERBOSE)) \
> + pr_wxp(msg); \
> + if (!(sara_wxp_flags & SARA_WXP_COMPLAIN)) \
> + goto label; \
> +} while (0)
No. This kind of "style" has no place in the kernel.
Don't hide control flow. It's nasty enough to reviewers,
but it's pure hell on anyone who strays into your code while
chasing a bug or doing general code audit. In effect, you
are creating your oh-so-private C dialect and assuming that
everyone who ever looks at your code will start with learning
that *AND* incorporating it into their mental C parser.
I'm sorry, but you are not that important.
If it looks like a function call, a casual reader will assume
that this is exactly what it is. And when one is scanning
through a function (e.g. to tell if handling of some kind
of refcounts is correct, with twentieth grep through the
tree having brought something in your code into the view),
the last thing one wants is to switch between the area-specific
C dialects. Simply because looking at yours is sandwiched
between digging through some crap in drivers/target/ and that
weird thing in kernel/tracing/, hopefully staying limited
to 20 seconds of glancing through several functions in your
code.
Don't Do That. Really.
^ permalink raw reply
* Re: [PATCH v5 00/12] S.A.R.A. a new stacked LSM
From: James Morris @ 2019-07-07 1:16 UTC (permalink / raw)
To: Salvatore Mesoraca
Cc: linux-kernel, kernel-hardening, linux-mm, linux-security-module,
Alexander Viro, Brad Spengler, Casey Schaufler, Christoph Hellwig,
Jann Horn, Kees Cook, PaX Team, Serge E. Hallyn, Thomas Gleixner
In-Reply-To: <1562410493-8661-1-git-send-email-s.mesoraca16@gmail.com>
On Sat, 6 Jul 2019, Salvatore Mesoraca wrote:
> S.A.R.A. (S.A.R.A. is Another Recursive Acronym) is a stacked Linux
Please make this just SARA. Nobody wants to read or type S.A.R.A.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v2] tomoyo: Don't check open/getattr permission on sockets.
From: James Morris @ 2019-07-07 2:44 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: linux-security-module
In-Reply-To: <bc146372-764d-93a9-af27-666d73ed3af5@i-love.sakura.ne.jp>
On Thu, 4 Jul 2019, Tetsuo Handa wrote:
> Hello.
>
> Since it seems that Al has no comments, I'd like to send this patch to
> linux-next.git . What should I do? Do I need to set up a git tree?
Yes, you can create one at github or similar.
>
> On 2019/06/22 13:45, Tetsuo Handa wrote:
> > From c63c4074300921d6d1c33c3b8dc9c84ebfededf5 Mon Sep 17 00:00:00 2001
> > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > Date: Sat, 22 Jun 2019 13:14:26 +0900
> > Subject: [PATCH v2] tomoyo: Don't check open/getattr permission on sockets.
> >
> > syzbot is reporting that use of SOCKET_I()->sk from open() can result in
> > use after free problem [1], for socket's inode is still reachable via
> > /proc/pid/fd/n despite destruction of SOCKET_I()->sk already completed.
> >
> > But there is no point with calling security_file_open() on sockets
> > because open("/proc/pid/fd/n", !O_PATH) on sockets fails with -ENXIO.
> >
> > There is some point with calling security_inode_getattr() on sockets
> > because stat("/proc/pid/fd/n") and fstat(open("/proc/pid/fd/n", O_PATH))
> > are valid. If we want to access "struct sock"->sk_{family,type,protocol}
> > fields, we will need to use security_socket_post_create() hook and
> > security_inode_free() hook in order to remember these fields because
> > security_sk_free() hook is called before the inode is destructed. But
> > since information which can be protected by checking
> > security_inode_getattr() on sockets is trivial, let's not be bothered by
> > "struct inode"->i_security management.
> >
> > There is point with calling security_file_ioctl() on sockets. Since
> > ioctl(open("/proc/pid/fd/n", O_PATH)) is invalid, security_file_ioctl()
> > on sockets should remain safe.
> >
> > [1] https://syzkaller.appspot.com/bug?id=73d590010454403d55164cca23bd0565b1eb3b74
> >
> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > Reported-by: syzbot <syzbot+0341f6a4d729d4e0acf1@syzkaller.appspotmail.com>
> > ---
> > security/tomoyo/tomoyo.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> > index 716c92e..8ea3f5d 100644
> > --- a/security/tomoyo/tomoyo.c
> > +++ b/security/tomoyo/tomoyo.c
> > @@ -126,6 +126,9 @@ static int tomoyo_bprm_check_security(struct linux_binprm *bprm)
> > */
> > static int tomoyo_inode_getattr(const struct path *path)
> > {
> > + /* It is not safe to call tomoyo_get_socket_name(). */
> > + if (S_ISSOCK(d_inode(path->dentry)->i_mode))
> > + return 0;
> > return tomoyo_path_perm(TOMOYO_TYPE_GETATTR, path, NULL);
> > }
> >
> > @@ -316,6 +319,9 @@ static int tomoyo_file_open(struct file *f)
> > /* Don't check read permission here if called from do_execve(). */
> > if (current->in_execve)
> > return 0;
> > + /* Sockets can't be opened by open(). */
> > + if (S_ISSOCK(file_inode(f)->i_mode))
> > + return 0;
> > return tomoyo_check_open_permission(tomoyo_domain(), &f->f_path,
> > f->f_flags);
> > }
> >
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v2] tomoyo: Don't check open/getattr permission on sockets.
From: James Morris @ 2019-07-07 2:50 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: linux-security-module
In-Reply-To: <alpine.LRH.2.21.1907061944050.2662@namei.org>
On Sat, 6 Jul 2019, James Morris wrote:
> On Thu, 4 Jul 2019, Tetsuo Handa wrote:
>
> > Hello.
> >
> > Since it seems that Al has no comments, I'd like to send this patch to
> > linux-next.git . What should I do? Do I need to set up a git tree?
>
> Yes, you can create one at github or similar.
Also notify Stephen Rothwell of the location of your -next branch, so it
gets pulled into his tree.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH] security/commoncap: Use xattr security prefix len
From: James Morris @ 2019-07-07 3:02 UTC (permalink / raw)
To: Carmeli Tamir; +Cc: serge, linux-security-module, linux-kernel
In-Reply-To: <20190706150738.4619-1-carmeli.tamir@gmail.com>
On Sat, 6 Jul 2019, Carmeli Tamir wrote:
> Using the existing defined XATTR_SECURITY_PREFIX_LEN instead of
> sizeof(XATTR_SECURITY_PREFIX) - 1. Pretty simple cleanup.
>
> Signed-off-by: Carmeli Tamir <carmeli.tamir@gmail.com>
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-lsm
Thanks!
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks
From: Dr. Greg @ 2019-07-07 13:30 UTC (permalink / raw)
To: Casey Schaufler
Cc: Xing, Cedric, Stephen Smalley, linux-sgx@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
Schaufler, Casey, jmorris@namei.org, luto@kernel.org,
jethro@fortanix.com, sds@tycho.nsa.gov,
jarkko.sakkinen@linux.intel.com, Christopherson, Sean J
In-Reply-To: <012fc47d-4e9d-3398-0d9d-d9298a758c8d@schaufler-ca.com>
On Wed, Jul 03, 2019 at 08:32:10AM -0700, Casey Schaufler wrote:
Good morning, I hope the weekend has been enjoyable for everyone.
> >> On 7/2/2019 12:42 AM, Xing, Cedric wrote:
> >>> ...
> >>> Guess this discussion will never end if we don't get into
> >>> code. Guess it'd be more productive to talk over phone then come back
> >>> to this thread with a conclusion. Will that be ok with you?
> >> I don't think that a phone call is going to help. Talking code
> >> issues tends to muddle them in my brain. If you can give me a few
> >> days I will propose a rough version of how I think your code should
> >> be integrated into the LSM environment. I'm spending more time
> >> trying (unsuccessfully :( ) to discribe the issues in English than
> >> it will probably take in C.
> > While Casey is off writing his rosetta stone,
> I'd hardly call it that. More of an effort to round the corners on
> the square peg. And Cedric has some ideas on how to approach that.
Should we infer from this comment that, of the two competing
strategies, Cedric's is the favored architecture?
> > let me suggest that the
> > most important thing we need to do is to take a little time, step back
> > and look at the big picture with respect to what we are trying to
> > accomplish and if we are going about it in a way that makes any sense
> > from an engineering perspective.
> >
> > This conversation shouldn't be about SGX, it should be about the best
> > way for the kernel/LSM to discipline a Trusted Execution Environment
> > (TEE). As I have noted previously, a TEE is a 'blackbox' that, by
> > design, is intended to allow execution of code and processing of data
> > in a manner that is resistant to manipulation or inspection by
> > untrusted userspace, the kernel and/or the hardware itself.
> >
> > Given that fact, if we are to be intellectually honest, we need to ask
> > ourselves how effective we believe we can be in controlling any TEE
> > with kernel based mechanisms. This is particularly the case if the
> > author of any code running in the TEE has adversarial intent.
> >
> > Here is the list of controls that we believe an LSM can, effectively,
> > implement against a TEE:
> >
> > 1.) Code provenance and origin.
> >
> > 2.) Cryptographic verification of dynamically executable content.
> >
> > 2.) The ability of a TEE to implement anonymous executable content.
> >
> > If people are in agreement with this concept, it is difficult to
> > understand why we should be implementing complex state machines and
> > the like, whether it is in the driver or the LSM. Security code has
> > to be measured with a metric of effectiveness, otherwise we are
> > engaging in security theater.
> >
> > I believe that if we were using this lens, we would already have a
> > mainline SGX driver, since we seem to have most of the needed LSM
> > infrastructure and any additional functionality would be a straight
> > forward implementation. Most importantly, the infrastructure would
> > not be SGX specific, which would seem to be a desirable political
> > concept.
> Generality introduced in the absence of multiple instances
> often results in unnecessary complexity, unused interfaces
> and feature compromise. Guessing what other TEE systems might
> do, and constraining SGX to those models (or the other way around)
> is a well established road to ruin. The LSM infrastructure is
> a fine example. For the first ten years the "general" mechanism
> had a single user. I'd say to hold off on the general until there
> is more experience with the specific. It's easier to construct
> a general mechanism around things that work than to fit things
> that need to work into some preconceived notion of generality.
All well taken points from an implementation perspective, but they
elide the point I was trying to make. Which is the fact that without
any semblance of a discussion regarding the requirements needed to
implement a security architecture around the concept of a TEE, this
entire process, despite Cedric's well intentioned efforts, amounts to
pounding a square solution into the round hole of a security problem.
Which, as I noted in my e-mail, is tantamount to security theater.
Everyone wants to see this driver upstream. If we would have had a
reasoned discussion regarding what it means to implement proper
controls around a TEE, when we started to bring these issues forward
last November, we could have possibly been on the road to having a
driver with reasoned security controls and one that actually delivers
the security guarantees the hardware was designed to deliver.
Best wishes for a productive week to everyone.
Dr. Greg
As always,
Dr. G.W. Wettstein, Ph.D. Enjellic Systems Development, LLC.
4206 N. 19th Ave. Specializing in information infra-structure
Fargo, ND 58102 development.
PH: 701-281-1686 EMAIL: greg@enjellic.com
------------------------------------------------------------------------------
"Any intelligent fool can make things bigger and more complex... It
takes a touch of genius - and a lot of courage to move in the opposite
direction."
-- Albert Einstein
^ permalink raw reply
* Re: [PATCH v5 00/12] S.A.R.A. a new stacked LSM
From: Salvatore Mesoraca @ 2019-07-07 15:40 UTC (permalink / raw)
To: James Morris
Cc: linux-kernel, Kernel Hardening, linux-mm, linux-security-module,
Alexander Viro, Brad Spengler, Casey Schaufler, Christoph Hellwig,
Jann Horn, Kees Cook, PaX Team, Serge E. Hallyn, Thomas Gleixner
In-Reply-To: <alpine.LRH.2.21.1907061814390.24897@namei.org>
James Morris <jmorris@namei.org> wrote:
>
> On Sat, 6 Jul 2019, Salvatore Mesoraca wrote:
>
> > S.A.R.A. (S.A.R.A. is Another Recursive Acronym) is a stacked Linux
>
> Please make this just SARA. Nobody wants to read or type S.A.R.A.
Agreed.
Thank you for your suggestion.
^ permalink raw reply
* Re: [PATCH v5 06/12] S.A.R.A.: WX protection
From: Salvatore Mesoraca @ 2019-07-07 15:49 UTC (permalink / raw)
To: Al Viro
Cc: linux-kernel, Kernel Hardening, linux-mm, linux-security-module,
Brad Spengler, Casey Schaufler, Christoph Hellwig, Jann Horn,
Kees Cook, PaX Team, Serge E. Hallyn, Thomas Gleixner,
James Morris
In-Reply-To: <20190706192852.GO17978@ZenIV.linux.org.uk>
Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Sat, Jul 06, 2019 at 12:54:47PM +0200, Salvatore Mesoraca wrote:
>
> > +#define sara_warn_or_return(err, msg) do { \
> > + if ((sara_wxp_flags & SARA_WXP_VERBOSE)) \
> > + pr_wxp(msg); \
> > + if (!(sara_wxp_flags & SARA_WXP_COMPLAIN)) \
> > + return -err; \
> > +} while (0)
> > +
> > +#define sara_warn_or_goto(label, msg) do { \
> > + if ((sara_wxp_flags & SARA_WXP_VERBOSE)) \
> > + pr_wxp(msg); \
> > + if (!(sara_wxp_flags & SARA_WXP_COMPLAIN)) \
> > + goto label; \
> > +} while (0)
>
> No. This kind of "style" has no place in the kernel.
>
> Don't hide control flow. It's nasty enough to reviewers,
> but it's pure hell on anyone who strays into your code while
> chasing a bug or doing general code audit. In effect, you
> are creating your oh-so-private C dialect and assuming that
> everyone who ever looks at your code will start with learning
> that *AND* incorporating it into their mental C parser.
> I'm sorry, but you are not that important.
>
> If it looks like a function call, a casual reader will assume
> that this is exactly what it is. And when one is scanning
> through a function (e.g. to tell if handling of some kind
> of refcounts is correct, with twentieth grep through the
> tree having brought something in your code into the view),
> the last thing one wants is to switch between the area-specific
> C dialects. Simply because looking at yours is sandwiched
> between digging through some crap in drivers/target/ and that
> weird thing in kernel/tracing/, hopefully staying limited
> to 20 seconds of glancing through several functions in your
> code.
>
> Don't Do That. Really.
I understand your concerns.
The first version of SARA didn't use these macros,
they were added because I was asked[1] to do so.
I have absolutely no problems in reverting this change.
I just want to make sure that there is agreement on this matter.
Maybe Kees can clarify his stance.
Thank you for your suggestions.
[1] https://lkml.kernel.org/r/CAGXu5jJuQx2qOt_aDqDQDcqGOZ5kmr5rQ9Zjv=MRRCJ65ERfGw@mail.gmail.com
^ permalink raw reply
* Re: [PATCH v5 04/12] S.A.R.A.: generic DFA for string matching
From: Salvatore Mesoraca @ 2019-07-07 16:01 UTC (permalink / raw)
To: Jann Horn
Cc: kernel list, Kernel Hardening, Linux-MM, linux-security-module,
Alexander Viro, Brad Spengler, Casey Schaufler, Christoph Hellwig,
Kees Cook, PaX Team, Serge E. Hallyn, Thomas Gleixner,
James Morris, John Johansen
In-Reply-To: <CAG48ez35oJhey5WNzMQR14ko6RPJUJp+nCuAHVUJqX7EPPPokA@mail.gmail.com>
Jann Horn <jannh@google.com> wrote:
>
> On Sat, Jul 6, 2019 at 12:55 PM Salvatore Mesoraca
> <s.mesoraca16@gmail.com> wrote:
> > Creation of a generic Discrete Finite Automata implementation
> > for string matching. The transition tables have to be produced
> > in user-space.
> > This allows us to possibly support advanced string matching
> > patterns like regular expressions, but they need to be supported
> > by user-space tools.
>
> AppArmor already has a DFA implementation that takes a DFA machine
> from userspace and runs it against file paths; see e.g.
> aa_dfa_match(). Did you look into whether you could move their DFA to
> some place like lib/ and reuse it instead of adding yet another
> generic rule interface to the kernel?
Yes, using AppArmor DFA cloud be a possibility.
Though, I didn't know how AppArmor's maintainers feel about this.
I thought that was easier to just implement my own.
Anyway I understand that re-using that code would be the optimal solution.
I'm adding in CC AppArmor's maintainers, let's see what they think about this.
> > +++ b/security/sara/dfa.c
> > @@ -0,0 +1,335 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * S.A.R.A. Linux Security Module
> > + *
> > + * Copyright (C) 2017 Salvatore Mesoraca <s.mesoraca16@gmail.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2, as
> > + * published by the Free Software Foundation.
>
> Throughout the series, you are adding files that both add an SPDX
> identifier and have a description of the license in the comment block
> at the top. The SPDX identifier already identifies the license.
I added the license description because I thought it was required anyway.
IANAL, if you tell me that SPDX it's enough I'll remove the description.
Thank you for your comments.
^ permalink raw reply
* Re: [PATCH v5 11/12] S.A.R.A.: /proc/*/mem write limitation
From: Salvatore Mesoraca @ 2019-07-07 16:15 UTC (permalink / raw)
To: Jann Horn
Cc: kernel list, Kernel Hardening, Linux-MM, linux-security-module,
Alexander Viro, Brad Spengler, Casey Schaufler, Christoph Hellwig,
Kees Cook, PaX Team, Serge E. Hallyn, Thomas Gleixner,
James Morris
In-Reply-To: <CAG48ez0uFX4AniOk1W0Vs6j=7Q5QfSFQTrBBzC2qL2bpWn_yCg@mail.gmail.com>
Jann Horn <jannh@google.com> wrote:
>
> On Sat, Jul 6, 2019 at 12:55 PM Salvatore Mesoraca
> <s.mesoraca16@gmail.com> wrote:
> > Prevent a task from opening, in "write" mode, any /proc/*/mem
> > file that operates on the task's mm.
> > A process could use it to overwrite read-only memory, bypassing
> > S.A.R.A. restrictions.
> [...]
> > +static void sara_task_to_inode(struct task_struct *t, struct inode *i)
> > +{
> > + get_sara_inode_task(i) = t;
>
> This looks bogus. Nothing is actually holding a reference to `t` here, right?
I think you are right, I should probably store the PID here.
> > +}
> > +
> > static struct security_hook_list data_hooks[] __lsm_ro_after_init = {
> > LSM_HOOK_INIT(cred_prepare, sara_cred_prepare),
> > LSM_HOOK_INIT(cred_transfer, sara_cred_transfer),
> > LSM_HOOK_INIT(shm_alloc_security, sara_shm_alloc_security),
> > + LSM_HOOK_INIT(task_to_inode, sara_task_to_inode),
> > };
> [...]
> > +static int sara_file_open(struct file *file)
> > +{
> > + struct task_struct *t;
> > + struct mm_struct *mm;
> > + u16 sara_wxp_flags = get_current_sara_wxp_flags();
> > +
> > + /*
> > + * Prevent write access to /proc/.../mem
> > + * if it operates on the mm_struct of the
> > + * current process: it could be used to
> > + * bypass W^X.
> > + */
> > +
> > + if (!sara_enabled ||
> > + !wxprot_enabled ||
> > + !(sara_wxp_flags & SARA_WXP_WXORX) ||
> > + !(file->f_mode & FMODE_WRITE))
> > + return 0;
> > +
> > + t = get_sara_inode_task(file_inode(file));
> > + if (unlikely(t != NULL &&
> > + strcmp(file->f_path.dentry->d_name.name,
> > + "mem") == 0)) {
>
> This should probably at least have a READ_ONCE() somewhere in case the
> file concurrently gets renamed?
My understanding here is that /proc/$pid/mem files cannot be renamed.
t != NULL implies we are in procfs.
Under these assumptions I think that that code is fine.
Am I wrong?
> > + get_task_struct(t);
> > + mm = get_task_mm(t);
> > + put_task_struct(t);
>
> Getting and dropping a reference to the task_struct here is completely
> useless. Either you have a reference, in which case you don't need to
> take another one, or you don't have a reference, in which case you
> also can't take one.
Absolutely agree.
> > + if (unlikely(mm == current->mm))
> > + sara_warn_or_goto(error,
> > + "write access to /proc/*/mem");
>
> Why is the current process so special that it must be protected more
> than other processes? Is the idea here to rely on other protections to
> protect all other tasks? This should probably come with a comment that
> explains this choice.
Yes, I should have spent some more words here.
Access to /proc/$pid/mem from other processes is already regulated by
security_ptrace_access_check (i.e. Yama).
Unfortunately, that hook is ignored when "mm == current->mm".
It seems that there is some user-space software that relies on /proc/self/mem
being writable (cfr. commit f511c0b17b08).
Thank you for your suggestions.
^ permalink raw reply
* Re: [RFC PATCH v4 07/12] LSM: x86/sgx: Introduce ->enclave_map() hook for Intel SGX
From: Sean Christopherson @ 2019-07-07 18:01 UTC (permalink / raw)
To: Xing, Cedric
Cc: Stephen Smalley, Jarkko Sakkinen, linux-sgx@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
Roberts, William C, Schaufler, Casey, James Morris, Hansen, Dave,
Andy Lutomirski, Jethro Beekman, Dr . Greg Wettstein
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F6551B85C@ORSMSX116.amr.corp.intel.com>
On Thu, Jun 27, 2019 at 01:29:39PM -0700, Xing, Cedric wrote:
> > From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> > owner@vger.kernel.org] On Behalf Of Stephen Smalley
> > Sent: Tuesday, June 25, 2019 1:48 PM
> >
> > On 6/21/19 12:54 PM, Xing, Cedric wrote:
> > >> From: Christopherson, Sean J
> > >> Sent: Wednesday, June 19, 2019 3:24 PM
> > >>
> > >> diff --git a/security/security.c b/security/security.c index
> > >> 613a5c00e602..03951e08bdfc 100644
> > >> --- a/security/security.c
> > >> +++ b/security/security.c
> > >> @@ -2359,3 +2359,10 @@ void security_bpf_prog_free(struct
> > bpf_prog_aux *aux)
> > >> call_void_hook(bpf_prog_free_security, aux);
> > >> }
> > >> #endif /* CONFIG_BPF_SYSCALL */
> > >> +
> > >> +#ifdef CONFIG_INTEL_SGX
> > >> +int security_enclave_map(unsigned long prot) {
> > >> + return call_int_hook(enclave_map, 0, prot); } #endif /*
> > >> +CONFIG_INTEL_SGX */
> > >
> > > Why is this new security_enclave_map() necessary while
> > security_mmap_file() will also be invoked?
> >
> > security_mmap_file() doesn't know about enclaves. It will just end up
> > checking FILE__READ, FILE__WRITE, and FILE__EXECUTE to /dev/sgx/enclave.
> > This was noted in the patch description.
>
> Surely I understand all those. As I mentioned in my other email,
> enclave_load() could indicate to LSM that a file is an enclave. Of course
> mmap() could be invoked before any pages are loaded so LSM wouldn't know at
> the first mmap(), but that doesn't matter as an empty enclave wouldn't post
> any threats anyway.
security_mmap_file() is invoked before the final address is known, and
MAP_FIXED isn't technically required.
^ permalink raw reply
* Re: [RFC PATCH v4 10/12] security/selinux: Add enclave_load() implementation
From: Sean Christopherson @ 2019-07-07 18:46 UTC (permalink / raw)
To: Xing, Cedric
Cc: Andy Lutomirski, Stephen Smalley, Jarkko Sakkinen,
linux-sgx@vger.kernel.org, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, Roberts, William C, Schaufler, Casey,
James Morris, Hansen, Dave, Jethro Beekman, Dr . Greg Wettstein
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F6551D77E@ORSMSX116.amr.corp.intel.com>
On Mon, Jul 01, 2019 at 01:03:51PM -0700, Xing, Cedric wrote:
> > From: Andy Lutomirski [mailto:luto@kernel.org]
> > Sent: Monday, July 01, 2019 12:33 PM
> >
> > It does make sense, but I'm not sure it's correct to assume that any LSM
> > policy will always allow execution on enclave source pages if it would
> > allow execution inside the enclave. As an example, here is a policy
> > that seems reasonable:
> >
> > Task A cannot execute dynamic non-enclave code (no execmod, no execmem,
> > etc -- only approved unmodified file pages can be executed).
> > But task A can execute an enclave with MRENCLAVE == such-and-such, and
> > that enclave may be loaded from regular anonymous memory -- the
> > MRENCLAVE is considered enough verification.
>
> You are right. That's a reasonable policy. But I still can't see the need for
> SGX_EXECUNMR, as MRENCLAVE is considered enough verification.
That assumes the enclave/loader developer will never make a mistake, and
that policy owners are going to do a deep dive on the EEXTEND values for
an enclave (and will never make a mistake).
User errors aside, EXECUNMR would also be useful in conjunction with
MRSIGNER, e.g. allow all enclaves signed by X, but disallow unmeasured
code.
^ permalink raw reply
* Re: [RFC PATCH v4 05/12] x86/sgx: Enforce noexec filesystem restriction for enclaves
From: Sean Christopherson @ 2019-07-07 19:03 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-sgx, linux-security-module, selinux, Bill Roberts,
Casey Schaufler, James Morris, Dave Hansen, Cedric Xing,
Andy Lutomirski, Jethro Beekman, Dr . Greg Wettstein,
Stephen Smalley
In-Reply-To: <20190621012654.GI20474@linux.intel.com>
On Fri, Jun 21, 2019 at 04:26:54AM +0300, Jarkko Sakkinen wrote:
> On Wed, Jun 19, 2019 at 03:23:54PM -0700, Sean Christopherson wrote:
> > Do not allow an enclave page to be mapped with PROT_EXEC if the source
> > vma does not have VM_MAYEXEC. This effectively enforces noexec as
> > do_mmap() clears VM_MAYEXEC if the vma is being loaded from a noexec
> > path, i.e. prevents executing a file by loading it into an enclave.
> >
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> > arch/x86/kernel/cpu/sgx/driver/ioctl.c | 42 +++++++++++++++++++++++---
> > 1 file changed, 37 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> > index e18d2afd2aad..1fca70a36ce3 100644
> > --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> > +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> > @@ -564,6 +564,39 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr,
> > return ret;
> > }
> >
> > +static int sgx_encl_page_copy(void *dst, unsigned long src, unsigned long prot)
>
> I will probably forget the context with this name after this has been
> merged :-) So many functions dealing with enclave pages. Two
> alternatives that come up to my mind:
>
> 1. sgx_encl_page_user_import()
> 2. sgx_encl_page_user_copy_from()
What about sgx_encl_page_copy_from_user() to align with copy_from_user()?
> Not saying that they are beatiful names but at least you immediately
> know the context.
>
> > +{
> > + struct vm_area_struct *vma;
> > + int ret;
> > +
> > + /* Hold mmap_sem across copy_from_user() to avoid a TOCTOU race. */
> > + down_read(¤t->mm->mmap_sem);
> > +
> > + /* Query vma's VM_MAYEXEC as an indirect path_noexec() check. */
> > + if (prot & PROT_EXEC) {
> > + vma = find_vma(current->mm, src);
> > + if (!vma) {
> > + ret = -EFAULT;
> > + goto out;
>
> Should this be -EINVAL instead?
copy_from_user() failure is handled via -EFAULT, this is effectively an
equivalent check.
^ permalink raw reply
* [RFC PATCH v3 0/4] security/x86/sgx: SGX specific LSM hooks
From: Cedric Xing @ 2019-07-07 23:41 UTC (permalink / raw)
To: linux-sgx, linux-security-module, selinux
Cc: Cedric Xing, casey.schaufler, jmorris, luto, jethro, greg, sds,
jarkko.sakkinen, sean.j.christopherson
In-Reply-To: <cover.1561588012.git.cedric.xing@intel.com>
This series intends to make the new SGX subsystem to work with the existing LSM
architecture smoothly so that, say, SGX cannot be abused to work around
restrictions set forth by LSM modules/policies.
This patch is based on and could be applied cleanly on top of Jarkko Sakkinen’s
SGX patch series v20 (https://patchwork.kernel.org/cover/10905141/).
For those who haven’t followed closely, the whole discussion started from the
primary question of how to prevent creating an executable enclave page from a
regular memory page that is NOT executable as prohibited by LSM
modules/policies. And that can be translated into 2 relating questions in
practice, i.e. 1) how to determine the allowed initial protections of enclave
pages when they are being loaded and 2) how to determine the allowed
protections of enclave pages at runtime. Those who are familiar with LSM may
notice that, for regular files, #1 is determined by security_mmap_file() while
#2 is covered by security_file_mprotect(). Those 2 hooks however are
insufficient for enclaves due to the distinct composition and lifespan of
enclave pages. Specifically, security_mmap_file() only passes in the file but
is not specific on which portion of the file being mmap()’ed, with the
assumption that all pages of the same file shall have the same set of
allowed/disallowed protections. But that assumption is no longer true for
enclaves for 2 reasons: a) pages of an enclave may be loaded from different
image files with different attributes and b) enclave pages retain contents
across munmap()/mmap(), therefore, say, if a policy prohibits execution of
modified pages, then pages flagged modified have to stay modified across
munmap()/mmap() so that the policy cannot be circumvented by remapping (i.e.
munmap() followed by mmap() on the same range). But the lack of range
information in security_mmap_file()’s arguments simply blocks LSM modules from
tracking enclave pages properly.
A rational solution would always involve tracking the correspondence between
enclave pages and their origin (e.g. files from which they were loaded), which
is similar to tracking regular memory pages and their origin via vm_file of
struct vm_area_struct. But given the longer lifespan of enclave pages (than
VMAs they are mapped into), such correspondence has to be stored in a separate
data structure outside of VMAs. In theory, the correspondence could be stored
either in LSM or in the SGX subsystem. This series has picked the former
because firstly, such information is useful only within LSM so it makes more
sense to keep it as “LSM internal” and secondly, keeping the data structure
inside LSM would allow additional information to be cached in LSM modules
without affecting the rest of the kernel, while lastly, those data structures
would be gone when LSM is disabled hence would not impose any unnecessary
overhead.
Those who are familiar with this topic and related discussions may also notice
that, Sean Christopherson has sent out an RFC patch recently to address the
same problem as this series. He adopted the other approach of tracking
page/origin correspondence inside the SGX subsystem. However, to reduce memory
overhead in practice, he cached the FSM (Finite State Machine) instead of
page/origin correspondences. By “FSM”, I mean policy FSM defined as sets of
states and events that may trigger state transitions. Generally speaking, any
LSM module has its own definition of FSM and usually uses attributes attached
to files to argument the FSM, then it advances the FSM as events are observed
and gives out decision based on the current FSM state. Sean’s implementation
attempts to move the FSM into the SGX subsystem, and by caching the arguments
returned by LSM it tries to monitor events and reach the same decisions by
itself. So from architecture perspective, that model has to face tough
challenges in reality, such as how to support multiple LSM modules that employ
different FSMs to govern page protection transitions. Implementation wise, his
model also imposes unwanted restrictions specifically to SGX2, such as:
· Complicated/Restricted UAPI – Enclave loaders are required to provide
“maximal protection” at page load time, but such information is NOT always
available. For example, Graphene containers may run different applications
comprised of different set of executables and/or shared objects. Some of
them may contain self-modifying code (or text relocation) while others
don’t. The generic enclave loader usually doesn’t have such information so
wouldn’t be able to provide it ahead of time.
· Inefficient Auditing – Audit logs are supposed to help system
administrators to determine the set of minimally needed permissions and to
detect abnormal behaviors. But consider the “maximal protection” model, if
“maximal protection” is set to be too permissive, then audit log wouldn’t
be able to detect anomalies; or if “maximal protection” is too restrictive,
then audit log cannot identify the file violating the policy. In either
case the audit log cannot fulfill its purposes.
· Inability to support #PF driven EPC allocation in SGX2 – For those
unfamiliar with SGX2 software flows, an SGX2 enclave requests a page by
issuing EACCEPT on the address that a new page is wanted, and the resulted
#PF is expected to be handled by the kernel by EAUG’ing an EPC page at the
fault address, and then the enclave would be resumed and the faulting
EACCEPT retried, and succeed. The key requirement is to allow mmap()’ing
non-existing enclave pages so that the SGX module/subsystem could respond
to #PFs by EAUG’ing new pages. Sean’s implementation doesn’t allow
mmap()’ing non-existing pages for variety of reasons and therefore blocks
this major SGX2 usage.
Please note that this series has only been compile-tested.
History:
· This is version 3 of this patch series, with the following changes over
version 2 per comments/requests from the community.
- Per Casey Schaufler, moved EMA from the LSM infrastructure into a new LSM
module, named “ema”, which is responsible for maintaining EMA maps for
enclaves and offers APIs for other LSM modules to query/update EMA nodes.
- Per Stephen Smalley, removed kernel command line option
“lsm.ema.cache_decisions”. Enclave page origins will always be tracked
and audit logs will always be accurate.
- Per Andy Lutomirski, a new PROCESS2__ENCLAVE_EXECANON permission has been
added to SELinux to allow EADD’ing executable pages from non-executable
anonymous source pages.
- Revised permission checks for enclave pages in SELinux. See
selinux_enclave_load() and enclave_mprotect() functions in
security/selinux/hooks.c for details.
· v2 – https://patchwork.kernel.org/cover/11020301/
· v1 – https://patchwork.kernel.org/cover/10984127/
Cedric Xing (4):
x86/sgx: Add SGX specific LSM hooks
x86/64: Call LSM hooks from SGX subsystem/module
X86/sgx: Introduce EMA as a new LSM module
x86/sgx: Implement SGX specific hooks in SELinux
arch/x86/kernel/cpu/sgx/driver/ioctl.c | 80 ++++++-
arch/x86/kernel/cpu/sgx/driver/main.c | 16 +-
include/linux/lsm_ema.h | 97 +++++++++
include/linux/lsm_hooks.h | 27 +++
include/linux/security.h | 23 ++
security/Makefile | 1 +
security/commonema.c | 277 +++++++++++++++++++++++++
security/security.c | 17 ++
security/selinux/hooks.c | 236 ++++++++++++++++++++-
security/selinux/include/classmap.h | 3 +-
security/selinux/include/objsec.h | 7 +
11 files changed, 770 insertions(+), 14 deletions(-)
create mode 100644 include/linux/lsm_ema.h
create mode 100644 security/commonema.c
--
2.17.1
^ permalink raw reply
* [RFC PATCH v3 1/4] x86/sgx: Add SGX specific LSM hooks
From: Cedric Xing @ 2019-07-07 23:41 UTC (permalink / raw)
To: linux-sgx, linux-security-module, selinux, cedric.xing
In-Reply-To: <cover.1562542383.git.cedric.xing@intel.com>
SGX enclaves are loaded from pages in regular memory. Given the ability to
create executable pages, the newly added SGX subsystem may present a backdoor
for adversaries to circumvent LSM policies, such as creating an executable
enclave page from a modified regular page that would otherwise not be made
executable as prohibited by LSM. Therefore arises the primary question of
whether an enclave page should be allowed to be created from a given source
page in regular memory.
A related question is whether to grant/deny a mprotect() request on a given
enclave page/range. mprotect() is traditionally covered by
security_file_mprotect() hook, however, enclave pages have a different lifespan
than either MAP_PRIVATE or MAP_SHARED. Particularly, MAP_PRIVATE pages have the
same lifespan as the VMA while MAP_SHARED pages have the same lifespan as the
backing file (on disk), but enclave pages have the lifespan of the enclave’s
file descriptor. For example, enclave pages could be munmap()’ed then mmap()’ed
again without losing contents (like MAP_SHARED), but all enclave pages will be
lost once its file descriptor has been closed (like MAP_PRIVATE). That said,
LSM modules need some new data structure for tracking protections of enclave
pages/ranges so that they can make proper decisions at mmap()/mprotect()
syscalls.
The last question, which is orthogonal to the 2 above, is whether or not to
allow a given enclave to launch/run. Enclave pages are not visible to the rest
of the system, so to some extent offer a better place for malicious software to
hide. Thus, it is sometimes desirable to whitelist/blacklist enclaves by their
measurements, signing public keys, or image files.
To address the questions above, 2 new LSM hooks are added for enclaves.
· security_enclave_load() – This hook allows LSM to decide whether or not to
allow instantiation of a range of enclave pages using the specified VMA. It
is invoked when a range of enclave pages is about to be loaded. It serves 3
purposes: 1) indicate to LSM that the file struct in subject is an enclave;
2) allow LSM to decide whether or not to instantiate those pages and 3)
allow LSM to initialize internal data structures for tracking
origins/protections of those pages.
· security_enclave_init() – This hook allows whitelisting/blacklisting or
performing whatever checks deemed appropriate before an enclave is allowed
to run. An LSM module may opt to use the file backing the SIGSTRUCT as a
proxy to dictate allowed protections for anonymous pages.
mprotect() of enclave pages continue to be governed by
security_file_mprotect(), with the expectation that LSM is able to distinguish
between regular and enclave pages inside the hook. For mmap(), the SGX
subsystem is expected to invoke security_file_mprotect() explicitly to check
protections against the requested protections for existing enclave pages.
Signed-off-by: Cedric Xing <cedric.xing@intel.com>
---
include/linux/lsm_hooks.h | 27 +++++++++++++++++++++++++++
include/linux/security.h | 23 +++++++++++++++++++++++
security/security.c | 17 +++++++++++++++++
3 files changed, 67 insertions(+)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 47f58cfb6a19..9d9e44200683 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1446,6 +1446,22 @@
* @bpf_prog_free_security:
* Clean up the security information stored inside bpf prog.
*
+ * @enclave_load:
+ * Decide if a range of pages shall be allowed to be loaded into an
+ * enclave
+ *
+ * @encl points to the file identifying the target enclave
+ * @start target range starting address
+ * @end target range ending address
+ * @flags contains protections being requested for the target range
+ * @source points to the VMA containing the source pages to be loaded
+ *
+ * @enclave_init:
+ * Decide if an enclave shall be allowed to launch
+ *
+ * @encl points to the file identifying the target enclave being launched
+ * @sigstruct contains a copy of the SIGSTRUCT in kernel memory
+ * @source points to the VMA backing SIGSTRUCT in user memory
*/
union security_list_options {
int (*binder_set_context_mgr)(struct task_struct *mgr);
@@ -1807,6 +1823,13 @@ union security_list_options {
int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux);
void (*bpf_prog_free_security)(struct bpf_prog_aux *aux);
#endif /* CONFIG_BPF_SYSCALL */
+
+#ifdef CONFIG_INTEL_SGX
+ int (*enclave_load)(struct file *encl, size_t start, size_t end,
+ size_t flags, struct vm_area_struct *source);
+ int (*enclave_init)(struct file *encl, struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *source);
+#endif
};
struct security_hook_heads {
@@ -2046,6 +2069,10 @@ struct security_hook_heads {
struct hlist_head bpf_prog_alloc_security;
struct hlist_head bpf_prog_free_security;
#endif /* CONFIG_BPF_SYSCALL */
+#ifdef CONFIG_INTEL_SGX
+ struct hlist_head enclave_load;
+ struct hlist_head enclave_init;
+#endif
} __randomize_layout;
/*
diff --git a/include/linux/security.h b/include/linux/security.h
index 659071c2e57c..52c200810004 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1829,5 +1829,28 @@ static inline void security_bpf_prog_free(struct bpf_prog_aux *aux)
#endif /* CONFIG_SECURITY */
#endif /* CONFIG_BPF_SYSCALL */
+#ifdef CONFIG_INTEL_SGX
+struct sgx_sigstruct;
+#ifdef CONFIG_SECURITY
+int security_enclave_load(struct file *encl, size_t start, size_t end,
+ size_t flags, struct vm_area_struct *source);
+int security_enclave_init(struct file *encl, struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *source);
+#else
+static inline int security_enclave_load(struct file *encl, size_t start,
+ size_t end, struct vm_area_struct *src)
+{
+ return 0;
+}
+
+static inline int security_enclave_init(struct file *encl,
+ struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *src)
+{
+ return 0;
+}
+#endif /* CONFIG_SECURITY */
+#endif /* CONFIG_INTEL_SGX */
+
#endif /* ! __LINUX_SECURITY_H */
diff --git a/security/security.c b/security/security.c
index f493db0bf62a..72c10f5e4f95 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1420,6 +1420,7 @@ int security_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
{
return call_int_hook(file_mprotect, 0, vma, reqprot, prot);
}
+EXPORT_SYMBOL(security_file_mprotect);
int security_file_lock(struct file *file, unsigned int cmd)
{
@@ -2355,3 +2356,19 @@ void security_bpf_prog_free(struct bpf_prog_aux *aux)
call_void_hook(bpf_prog_free_security, aux);
}
#endif /* CONFIG_BPF_SYSCALL */
+
+#ifdef CONFIG_INTEL_SGX
+int security_enclave_load(struct file *encl, size_t start, size_t end,
+ size_t flags, struct vm_area_struct *src)
+{
+ return call_int_hook(enclave_load, 0, encl, start, end, flags, src);
+}
+EXPORT_SYMBOL(security_enclave_load);
+
+int security_enclave_init(struct file *encl, struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *src)
+{
+ return call_int_hook(enclave_init, 0, encl, sigstruct, src);
+}
+EXPORT_SYMBOL(security_enclave_init);
+#endif /* CONFIG_INTEL_SGX */
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v3 2/4] x86/64: Call LSM hooks from SGX subsystem/module
From: Cedric Xing @ 2019-07-07 23:41 UTC (permalink / raw)
To: linux-sgx, linux-security-module, selinux, cedric.xing
In-Reply-To: <cover.1562542383.git.cedric.xing@intel.com>
It’s straightforward to call new LSM hooks from the SGX subsystem/module. There
are three places where LSM hooks are invoked.
1) sgx_mmap() invokes security_file_mprotect() to validate requested
protection. It is necessary because security_mmap_file() invoked by mmap()
syscall only validates protections against /dev/sgx/enclave file, but not
against those files from which the pages were loaded from.
2) security_enclave_load() is invoked upon loading of every enclave page by
the EADD ioctl. Please note that if pages are EADD’ed in batch, the SGX
subsystem/module is responsible for dividing pages in trunks so that each
trunk is loaded from a single VMA.
3) security_enclave_init() is invoked before initializing (EINIT) every
enclave.
Signed-off-by: Cedric Xing <cedric.xing@intel.com>
---
arch/x86/kernel/cpu/sgx/driver/ioctl.c | 80 +++++++++++++++++++++++---
arch/x86/kernel/cpu/sgx/driver/main.c | 16 +++++-
2 files changed, 85 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
index b186fb7b48d5..4f5abf9819a7 100644
--- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
// Copyright(c) 2016-19 Intel Corporation.
-#include <asm/mman.h>
+#include <linux/mman.h>
#include <linux/delay.h>
#include <linux/file.h>
#include <linux/hashtable.h>
@@ -11,6 +11,7 @@
#include <linux/shmem_fs.h>
#include <linux/slab.h>
#include <linux/suspend.h>
+#include <linux/security.h>
#include "driver.h"
struct sgx_add_page_req {
@@ -575,6 +576,46 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr,
return ret;
}
+static int sgx_encl_prepare_page(struct file *filp, unsigned long dst,
+ unsigned long src, void *buf)
+{
+ struct vm_area_struct *vma;
+ unsigned long prot;
+ int rc;
+
+ if (dst & ~PAGE_SIZE)
+ return -EINVAL;
+
+ rc = down_read_killable(¤t->mm->mmap_sem);
+ if (rc)
+ return rc;
+
+ vma = find_vma(current->mm, dst);
+ if (vma && dst >= vma->vm_start)
+ prot = _calc_vm_trans(vma->vm_flags, VM_READ, PROT_READ) |
+ _calc_vm_trans(vma->vm_flags, VM_WRITE, PROT_WRITE) |
+ _calc_vm_trans(vma->vm_flags, VM_EXEC, PROT_EXEC);
+ else
+ prot = 0;
+
+ vma = find_vma(current->mm, src);
+ if (!vma || src < vma->vm_start || src + PAGE_SIZE > vma->vm_end)
+ rc = -EFAULT;
+
+ if (!rc && !(vma->vm_flags & VM_MAYEXEC))
+ rc = -EACCES;
+
+ if (!rc && copy_from_user(buf, (void __user *)src, PAGE_SIZE))
+ rc = -EFAULT;
+
+ if (!rc)
+ rc = security_enclave_load(filp, dst, PAGE_SIZE, prot, vma);
+
+ up_read(¤t->mm->mmap_sem);
+
+ return rc;
+}
+
/**
* sgx_ioc_enclave_add_page - handler for %SGX_IOC_ENCLAVE_ADD_PAGE
*
@@ -613,10 +654,9 @@ static long sgx_ioc_enclave_add_page(struct file *filep, unsigned int cmd,
data = kmap(data_page);
- if (copy_from_user((void *)data, (void __user *)addp->src, PAGE_SIZE)) {
- ret = -EFAULT;
+ ret = sgx_encl_prepare_page(filep, addp->addr, addp->src, data);
+ if (ret)
goto out;
- }
ret = sgx_encl_add_page(encl, addp->addr, data, &secinfo, addp->mrmask);
if (ret)
@@ -718,6 +758,31 @@ static int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
return ret;
}
+static int sgx_encl_prepare_sigstruct(struct file *filp, unsigned long src,
+ struct sgx_sigstruct *ss)
+{
+ struct vm_area_struct *vma;
+ int rc;
+
+ rc = down_read_killable(¤t->mm->mmap_sem);
+ if (rc)
+ return rc;
+
+ vma = find_vma(current->mm, src);
+ if (!vma || src < vma->vm_start || src + sizeof(*ss) > vma->vm_end)
+ rc = -EFAULT;
+
+ if (!rc && copy_from_user(ss, (void __user *)src, sizeof(*ss)))
+ rc = -EFAULT;
+
+ if (!rc)
+ rc = security_enclave_init(filp, ss, vma);
+
+ up_read(¤t->mm->mmap_sem);
+
+ return rc;
+}
+
/**
* sgx_ioc_enclave_init - handler for %SGX_IOC_ENCLAVE_INIT
*
@@ -753,12 +818,9 @@ static long sgx_ioc_enclave_init(struct file *filep, unsigned int cmd,
((unsigned long)sigstruct + PAGE_SIZE / 2);
memset(einittoken, 0, sizeof(*einittoken));
- if (copy_from_user(sigstruct, (void __user *)initp->sigstruct,
- sizeof(*sigstruct))) {
- ret = -EFAULT;
+ ret = sgx_encl_prepare_sigstruct(filep, initp->sigstruct, sigstruct);
+ if (ret)
goto out;
- }
-
ret = sgx_encl_init(encl, sigstruct, einittoken);
diff --git a/arch/x86/kernel/cpu/sgx/driver/main.c b/arch/x86/kernel/cpu/sgx/driver/main.c
index 58ba6153070b..8848711a55bd 100644
--- a/arch/x86/kernel/cpu/sgx/driver/main.c
+++ b/arch/x86/kernel/cpu/sgx/driver/main.c
@@ -63,14 +63,26 @@ static long sgx_compat_ioctl(struct file *filep, unsigned int cmd,
static int sgx_mmap(struct file *file, struct vm_area_struct *vma)
{
struct sgx_encl *encl = file->private_data;
+ unsigned long prot;
+ int rc;
vma->vm_ops = &sgx_vm_ops;
vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP | VM_IO;
vma->vm_private_data = encl;
- kref_get(&encl->refcount);
+ prot = vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
+ vma->vm_flags &= ~prot;
- return 0;
+ prot = _calc_vm_trans(prot, VM_READ, PROT_READ) |
+ _calc_vm_trans(prot, VM_WRITE, PROT_WRITE) |
+ _calc_vm_trans(prot, VM_EXEC, PROT_EXEC);
+ rc = security_file_mprotect(vma, prot, prot);
+ if (!rc) {
+ vma->vm_flags |= calc_vm_prot_bits(prot, 0);
+ kref_get(&encl->refcount);
+ }
+
+ return rc;
}
static unsigned long sgx_get_unmapped_area(struct file *file,
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v3 4/4] x86/sgx: Implement SGX specific hooks in SELinux
From: Cedric Xing @ 2019-07-07 23:41 UTC (permalink / raw)
To: linux-sgx, linux-security-module, selinux, cedric.xing
In-Reply-To: <cover.1562542383.git.cedric.xing@intel.com>
This patch governs enclave page protections in a similar way to how current
SELinux governs protections for regular memory pages. In summary:
· All pages are allowed PROT_READ/PROT_WRITE upon request.
· For pages that are EADD’ed, PROT_EXEC will be granted initially if
PROT_EXEC could also be granted to the VMA containing the source pages, or
if the calling process has ENCLAVE_EXECANON permission. Afterwards,
PROT_EXEC will be removed once PROT_WRITE is requested/granted, and could
be granted again if the backing file has EXECMOD or the calling process has
PROCMEM. For anonymous pages, backing file is considered to be the file
containing SIGSTRUCT.
· For pages that are EAUG’ed, they are considered modified initially so
PROT_EXEC will not be granted unless the file containing SIGSTRUCT has
EXECMOD, or the calling process has EXECMEM.
Besides, launch control is implemented as EXECUTE permission on the SIGSTRUCT
file. That is,
· SIGSTRUCT file has EXECUTE – Enclave is allowed to launch. But this is
granted only if the enclosing VMA has the same content as the disk file
(i.e. vma->anon_vma == NULL).
· SIGSTRUCT file has EXECMOD – All anonymous enclave pages are allowed
PROT_EXEC.
In all cases, simultaneous WX requires EXECMEM on the calling process.
Implementation wise, 2 bits are associated with every EMA by SELinux.
· sourced – Set if EMA is loaded from some memory page (i.e. EADD’ed),
cleared otherwise. When cleared, the backing file is considered to be the
file containing SIGSTRUCT.
· modified – Set if EMA has ever been mapped writable, as result of
mmap()/mprotect() syscalls. When set, FILE__EXECMOD is required on the
backing file for the range to be executable.
Both bits are initialized at selinux_enclave_load() and checked in
selinux_file_mprotect(). SGX subsystem is expected to invoke
security_file_mprotect() upon mmap() to not bypass the check. mmap() shall be
treated as mprotect() from PROT_NONE to the requested protection.
selinux_enclave_init() determines if an enclave is allowed to launch, using the
criteria described earlier. This implementation does NOT accept SIGSTRUCT in
anonymous memory. The backing file is also cached in struct
file_security_struct and will serve as the base for decisions for anonymous
pages.
There’s one new process permission – PROCESS2__ENCLAVE_EXECANON introduced by
this patch. It is equivalent to FILE__EXECUTE for all enclave pages loaded from
anonymous mappings.
Signed-off-by: Cedric Xing <cedric.xing@intel.com>
---
security/selinux/hooks.c | 236 +++++++++++++++++++++++++++-
security/selinux/include/classmap.h | 3 +-
security/selinux/include/objsec.h | 7 +
3 files changed, 243 insertions(+), 3 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 94de51628fdc..c7fe1d47654d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3499,6 +3499,13 @@ static int selinux_file_alloc_security(struct file *file)
return file_alloc_security(file);
}
+static void selinux_file_free_security(struct file *file)
+{
+ long f = atomic_long_read(&selinux_file(file)->encl_ss);
+ if (f)
+ fput((struct file *)f);
+}
+
/*
* Check whether a task has the ioctl permission and cmd
* operation to an inode.
@@ -3666,19 +3673,23 @@ static int selinux_mmap_file(struct file *file, unsigned long reqprot,
(flags & MAP_TYPE) == MAP_SHARED);
}
+#ifdef CONFIG_INTEL_SGX
+static int enclave_mprotect(struct vm_area_struct *, size_t);
+#endif
+
static int selinux_file_mprotect(struct vm_area_struct *vma,
unsigned long reqprot,
unsigned long prot)
{
const struct cred *cred = current_cred();
u32 sid = cred_sid(cred);
+ int rc = 0;
if (selinux_state.checkreqprot)
prot = reqprot;
if (default_noexec &&
(prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
- int rc = 0;
if (vma->vm_start >= vma->vm_mm->start_brk &&
vma->vm_end <= vma->vm_mm->brk) {
rc = avc_has_perm(&selinux_state,
@@ -3705,7 +3716,12 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
return rc;
}
- return file_map_prot_check(vma->vm_file, prot, vma->vm_flags&VM_SHARED);
+ rc = file_map_prot_check(vma->vm_file, prot, vma->vm_flags&VM_SHARED);
+#ifdef CONFIG_INTEL_SGX
+ if (!rc)
+ rc = enclave_mprotect(vma, prot);
+#endif
+ return rc;
}
static int selinux_file_lock(struct file *file, unsigned int cmd)
@@ -6740,6 +6756,213 @@ static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
}
#endif
+#ifdef CONFIG_INTEL_SGX
+static size_t ema__blob __lsm_ro_after_init;
+
+static inline struct ema_security_struct *selinux_ema(struct ema *ema)
+{
+ return ema_data(ema, ema__blob);
+}
+
+static int ema__chk_X_cb(struct ema *ema, void *a)
+{
+ struct file_security_struct *fsec = selinux_file(a);
+ struct ema_security_struct *esec = selinux_ema(ema);
+ struct file *ess = (struct file *)atomic_long_read(&fsec->encl_ss);
+ int rc;
+
+ if (!esec->sourced) {
+ /* EAUG'ed pages */
+ rc = file_has_perm(current_cred(), ess, FILE__EXECMOD);
+ } else if (!ema->source) {
+ /* EADD'ed anonymous pages */
+ u32 sid = current_sid();
+ rc = avc_has_perm(&selinux_state, sid, sid, SECCLASS_PROCESS2,
+ PROCESS2__ENCLAVE_EXECANON, NULL);
+ if (rc)
+ rc = avc_has_perm(&selinux_state, sid, sid,
+ SECCLASS_PROCESS, PROCESS__EXECMEM,
+ NULL);
+ if (!rc && esec->modified)
+ rc = file_has_perm(current_cred(), ess, FILE__EXECMOD);
+ } else {
+ /* EADD'ed pages from files */
+ u32 av = FILE__EXECUTE;
+ if (esec->modified)
+ av |= FILE__EXECMOD;
+ rc = file_has_perm(current_cred(), ema->source, av);
+ }
+
+ return rc;
+}
+
+static int ema__set_M_cb(struct ema *ema, void *a)
+{
+ selinux_ema(ema)->modified = 1;
+ return 0;
+}
+
+static int enclave_mprotect(struct vm_area_struct *vma, size_t prot)
+{
+ struct ema_map *m;
+ int rc;
+
+ /* is vma an enclave vma ? */
+ if (!vma->vm_file)
+ return 0;
+ m = ema_get_map(vma->vm_file);
+ if (!m)
+ return 0;
+
+ /* WX requires EXECMEM */
+ if ((prot && PROT_WRITE) && (prot & PROT_EXEC)) {
+ rc = avc_has_perm(&selinux_state, current_sid(), current_sid(),
+ SECCLASS_PROCESS, PROCESS__EXECMEM, NULL);
+ if (rc)
+ return rc;
+ }
+
+ rc = ema_lock_map(m);
+ if (rc)
+ return rc;
+
+ if ((prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC))
+ rc = ema_apply_to_range(m, vma->vm_start, vma->vm_end,
+ ema__chk_X_cb, vma->vm_file);
+ if (!rc && (prot & PROT_WRITE) && !(vma->vm_flags & VM_WRITE))
+ rc = ema_apply_to_range(m, vma->vm_start, vma->vm_end,
+ ema__set_M_cb, NULL);
+
+ ema_unlock_map(m);
+
+ return rc;
+}
+
+static int enclave_load_prot_check(struct file *encl, size_t prot,
+ struct vm_area_struct *vma)
+{
+ struct file_security_struct *fsec = selinux_file(encl);
+ struct file *ess;
+ const struct cred *cred = current_cred();
+ u32 sid = cred_sid(cred);
+ int rc;
+ int modified = 0;
+
+ /* R/W without X are always allowed */
+ if (!(prot & PROT_EXEC))
+ /* R/W always allowed */
+ return 0;
+
+ if (!vma) {
+ ess = (struct file *)atomic_long_read(&fsec->encl_ss);
+ WARN_ON(!ess);
+ if (unlikely(!ess))
+ return -EPERM;
+
+ /* For EAUG, X is considered self-modifying code */
+ rc = file_has_perm(cred, ess, FILE__EXECMOD);
+ } else if (!vma->vm_file || IS_PRIVATE(file_inode(vma->vm_file))) {
+ /* EADD from anonymous pages requires ENCLAVE_EXECANON */
+ if (!(prot & PROT_WRITE) &&
+ avc_has_perm(&selinux_state, sid, sid, SECCLASS_PROCESS2,
+ PROCESS2__ENCLAVE_EXECANON, NULL)) {
+ /* On failure, Trigger EXECMEM check at the end */
+ prot |= PROT_WRITE;
+ }
+ rc = 0;
+ } else {
+ /* EADD from file requires EXECUTE */
+ u32 av = FILE__EXECUTE;
+
+ /* EXECMOD required for modified private mapping */
+ if (vma->anon_vma) {
+ av |= FILE__EXECMOD;
+ modified = 1;
+ }
+
+ rc = file_has_perm(cred, vma->vm_file, av);
+ }
+
+ /* WX requires EXECMEM additionally */
+ if (!rc && (prot & PROT_WRITE))
+ rc = avc_has_perm(&selinux_state, sid, sid, SECCLASS_PROCESS,
+ PROCESS__EXECMEM, NULL);
+
+ return rc ? rc : modified;
+}
+
+static int ema__set_cb(struct ema *ema, void *a)
+{
+ struct ema_security_struct *esec = selinux_ema(ema);
+ struct ema_security_struct *s = a;
+
+ esec->modified = s->modified;
+ esec->sourced = s->sourced;
+ return 0;
+}
+
+static int selinux_enclave_load(struct file *encl, size_t start, size_t end,
+ size_t flags, struct vm_area_struct *src)
+{
+ struct ema_map *m;
+ size_t prot;
+ int rc;
+
+ m = ema_get_map(encl);
+ WARN_ON(!m);
+ if (unlikely(!m))
+ return -EPERM;
+
+ prot = flags & (PROT_READ | PROT_WRITE | PROT_EXEC);
+
+ /* check if @prot could be granted */
+ rc = enclave_load_prot_check(encl, prot, src);
+
+ /* initialize ema */
+ if (rc >= 0) {
+ struct ema_security_struct esec;
+
+ if ((prot & PROT_WRITE) || rc)
+ esec.modified = 1;
+ if (src)
+ esec.sourced = 1;
+
+ rc = ema_lock_apply_to_range(m, start, end,
+ ema__set_cb, &esec);
+ }
+
+ /* remove ema on error */
+ if (rc)
+ ema_remove_range(m, start, end);
+
+ return rc;
+}
+
+static int selinux_enclave_init(struct file *encl,
+ struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *src)
+{
+ struct file_security_struct *fsec = selinux_file(encl);
+ int rc;
+
+ /* Is @src mapped shared, or mapped privately and not modified? */
+ if (!src->vm_file || src->anon_vma)
+ return -EACCES;
+
+ /* EXECUTE grants enclaves permission to launch */
+ rc = file_has_perm(current_cred(), src->vm_file, FILE__EXECUTE);
+ if (rc)
+ return rc;
+
+ /* Store SIGSTRUCT file for future use */
+ if (atomic_long_cmpxchg(&fsec->encl_ss, 0, (long)src->vm_file))
+ return -EEXIST;
+
+ get_file(src->vm_file);
+ return 0;
+}
+#endif
+
struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
.lbs_cred = sizeof(struct task_security_struct),
.lbs_file = sizeof(struct file_security_struct),
@@ -6822,6 +7045,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(file_permission, selinux_file_permission),
LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
+ LSM_HOOK_INIT(file_free_security, selinux_file_free_security),
LSM_HOOK_INIT(file_ioctl, selinux_file_ioctl),
LSM_HOOK_INIT(mmap_file, selinux_mmap_file),
LSM_HOOK_INIT(mmap_addr, selinux_mmap_addr),
@@ -6982,6 +7206,11 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free),
LSM_HOOK_INIT(bpf_prog_free_security, selinux_bpf_prog_free),
#endif
+
+#ifdef CONFIG_INTEL_SGX
+ LSM_HOOK_INIT(enclave_load, selinux_enclave_load),
+ LSM_HOOK_INIT(enclave_init, selinux_enclave_init),
+#endif
};
static __init int selinux_init(void)
@@ -7007,6 +7236,9 @@ static __init int selinux_init(void)
hashtab_cache_init();
+#ifdef CONFIG_INTEL_SGX
+ ema__blob = ema_request_blob(sizeof(struct ema_security_struct));
+#endif
security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 201f7e588a29..0d3161a52577 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -51,7 +51,8 @@ struct security_class_mapping secclass_map[] = {
"execmem", "execstack", "execheap", "setkeycreate",
"setsockcreate", "getrlimit", NULL } },
{ "process2",
- { "nnp_transition", "nosuid_transition", NULL } },
+ { "nnp_transition", "nosuid_transition",
+ "enclave_execanon", NULL } },
{ "system",
{ "ipc_info", "syslog_read", "syslog_mod",
"syslog_console", "module_request", "module_load", NULL } },
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 91c5395dd20c..8d1ce9c6d6fa 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -23,6 +23,7 @@
#include <linux/in.h>
#include <linux/spinlock.h>
#include <linux/lsm_hooks.h>
+#include <linux/lsm_ema.h>
#include <linux/msg.h>
#include <net/net_namespace.h>
#include "flask.h"
@@ -68,6 +69,7 @@ struct file_security_struct {
u32 fown_sid; /* SID of file owner (for SIGIO) */
u32 isid; /* SID of inode at the time of file open */
u32 pseqno; /* Policy seqno at the time of file open */
+ atomic_long_t encl_ss; /* Enclave sigstruct file */
};
struct superblock_security_struct {
@@ -154,6 +156,11 @@ struct bpf_security_struct {
u32 sid; /*SID of bpf obj creater*/
};
+struct ema_security_struct {
+ int modified:1; /* Set when W is granted */
+ int sourced:1; /* Set if loaded from source in regular memory */
+};
+
extern struct lsm_blob_sizes selinux_blob_sizes;
static inline struct task_security_struct *selinux_cred(const struct cred *cred)
{
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v3 3/4] X86/sgx: Introduce EMA as a new LSM module
From: Cedric Xing @ 2019-07-07 23:41 UTC (permalink / raw)
To: linux-sgx, linux-security-module, selinux, cedric.xing
In-Reply-To: <cover.1562542383.git.cedric.xing@intel.com>
As enclave pages have different lifespan than the existing MAP_PRIVATE and
MAP_SHARED pages, a new data structure is required outside of VMA to track
their protections and/or origins. Enclave Memory Area (or EMA for short) has
been introduced to address the need. EMAs are maintained by a new LSM module
named “ema”, which is similar to the idea of the “capability” LSM module.
This new “ema” module has LSM_ORDER_FIRST so will always be dispatched before
other LSM_ORDER_MUTABLE modules (e.g. selinux, apparmor, etc.). It is
responsible for initializing EMA maps, and inserting and freeing EMA nodes, and
offers APIs for other LSM modules to query/update EMAs. Details could be found
in include/linux/lsm_ema.h and security/commonema.c.
Signed-off-by: Cedric Xing <cedric.xing@intel.com>
---
include/linux/lsm_ema.h | 97 ++++++++++++++
security/Makefile | 1 +
security/commonema.c | 277 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 375 insertions(+)
create mode 100644 include/linux/lsm_ema.h
create mode 100644 security/commonema.c
diff --git a/include/linux/lsm_ema.h b/include/linux/lsm_ema.h
new file mode 100644
index 000000000000..59fc4ea6fa78
--- /dev/null
+++ b/include/linux/lsm_ema.h
@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/**
+ * Enclave Memory Area interface for LSM modules
+ *
+ * Copyright(c) 2016-19 Intel Corporation.
+ */
+
+#ifndef _LSM_EMA_H_
+#define _LSM_EMA_H_
+
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+
+/**
+ * ema - Enclave Memory Area structure for LSM modules
+ *
+ * Data structure to track origins of enclave pages
+ *
+ * @link:
+ * Link to adjacent EMAs. EMAs are sorted by their addresses in ascending
+ * order
+ * @start:
+ * Starting address
+ * @end:
+ * Ending address
+ * @source:
+ * File from which this range was loaded from, or NULL if not loaded from
+ * any files
+ */
+struct ema {
+ struct list_head link;
+ size_t start;
+ size_t end;
+ struct file *source;
+};
+
+#define ema_data(ema, offset) \
+ ((void *)((char *)((struct ema *)(ema) + 1) + offset))
+
+/**
+ * ema_map - LSM Enclave Memory Map structure for LSM modules
+ *
+ * Container for EMAs of an enclave
+ *
+ * @list:
+ * Head of a list of sorted EMAs
+ * @lock:
+ * Acquire before querying/updateing the list EMAs
+ */
+struct ema_map {
+ struct list_head list;
+ struct mutex lock;
+};
+
+size_t __init ema_request_blob(size_t blob_size);
+struct ema_map *ema_get_map(struct file *encl);
+int ema_apply_to_range(struct ema_map *map, size_t start, size_t end,
+ int (*cb)(struct ema *ema, void *arg), void *arg);
+void ema_remove_range(struct ema_map *map, size_t start, size_t end);
+
+static inline int __must_check ema_lock_map(struct ema_map *map)
+{
+ return mutex_lock_interruptible(&map->lock);
+}
+
+static inline void ema_unlock_map(struct ema_map *map)
+{
+ mutex_unlock(&map->lock);
+}
+
+static inline int ema_lock_apply_to_range(struct ema_map *map,
+ size_t start, size_t end,
+ int (*cb)(struct ema *, void *),
+ void *arg)
+{
+ int rc = ema_lock_map(map);
+ if (!rc) {
+ rc = ema_apply_to_range(map, start, end, cb, arg);
+ ema_unlock_map(map);
+ }
+ return rc;
+}
+
+static inline int ema_lock_remove_range(struct ema_map *map,
+ size_t start, size_t end)
+{
+ int rc = ema_lock_map(map);
+ if (!rc) {
+ ema_remove_range(map, start, end);
+ ema_unlock_map(map);
+ }
+ return rc;
+}
+
+#endif /* _LSM_EMA_H_ */
diff --git a/security/Makefile b/security/Makefile
index c598b904938f..b66d03a94853 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_SECURITY_YAMA) += yama/
obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/
obj-$(CONFIG_SECURITY_SAFESETID) += safesetid/
obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
+obj-$(CONFIG_INTEL_SGX) += commonema.o
# Object integrity file lists
subdir-$(CONFIG_INTEGRITY) += integrity
diff --git a/security/commonema.c b/security/commonema.c
new file mode 100644
index 000000000000..c5b0bdfdc013
--- /dev/null
+++ b/security/commonema.c
@@ -0,0 +1,277 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#include <linux/lsm_ema.h>
+#include <linux/lsm_hooks.h>
+#include <linux/slab.h>
+
+static struct kmem_cache *_map_cache;
+static struct kmem_cache *_node_cache;
+static size_t _data_size __lsm_ro_after_init;
+
+static struct lsm_blob_sizes ema_blob_sizes __lsm_ro_after_init = {
+ .lbs_file = sizeof(atomic_long_t),
+};
+
+static atomic_long_t *_map_file(struct file *encl)
+{
+ return (void *)((char *)(encl->f_security) + ema_blob_sizes.lbs_file);
+}
+
+static struct ema_map *_alloc_map(void)
+{
+ struct ema_map *m;
+
+ m = kmem_cache_zalloc(_map_cache, GFP_KERNEL);
+ if (likely(m)) {
+ INIT_LIST_HEAD(&m->list);
+ mutex_init(&m->lock);
+ }
+ return m;
+}
+
+static struct ema *_new_ema(size_t start, size_t end, struct file *src)
+{
+ struct ema *ema;
+
+ if (unlikely(!_node_cache)) {
+ struct kmem_cache *c;
+
+ c = kmem_cache_create("lsm-ema", sizeof(*ema) + _data_size,
+ __alignof__(typeof(*ema)), SLAB_PANIC,
+ NULL);
+ if (atomic_long_cmpxchg((atomic_long_t *)&_node_cache, 0,
+ (long)c))
+ kmem_cache_destroy(c);
+ }
+
+ ema = kmem_cache_zalloc(_node_cache, GFP_KERNEL);
+ if (likely(ema)) {
+ INIT_LIST_HEAD(&ema->link);
+ ema->start = start;
+ ema->end = end;
+ if (src)
+ ema->source = get_file(src);
+ }
+ return ema;
+}
+
+static void _free_ema(struct ema *ema)
+{
+ if (ema->source)
+ fput(ema->source);
+ kmem_cache_free(_node_cache, ema);
+}
+
+static void _free_map(struct ema_map *map)
+{
+ struct ema *p, *n;
+
+ WARN_ON(mutex_is_locked(&map->lock));
+ list_for_each_entry_safe(p, n, &map->list, link)
+ _free_ema(p);
+ kmem_cache_free(_map_cache, map);
+}
+
+static struct ema_map *_init_map(struct file *encl)
+{
+ struct ema_map *m = ema_get_map(encl);
+ if (!m) {
+ m = _alloc_map();
+ if (atomic_long_cmpxchg(_map_file(encl), 0, (long)m)) {
+ _free_map(m);
+ m = ema_get_map(encl);
+ }
+ }
+ return m;
+}
+
+static inline struct ema *_next_ema(struct ema *p, struct ema_map *map)
+{
+ p = list_next_entry(p, link);
+ return &p->link == &map->list ? NULL : p;
+}
+
+static inline struct ema *_find_ema(struct ema_map *map, size_t a)
+{
+ struct ema *p;
+
+ WARN_ON(!mutex_is_locked(&map->lock));
+
+ list_for_each_entry(p, &map->list, link)
+ if (a < p->end)
+ break;
+ return &p->link == &map->list ? NULL : p;
+}
+
+static struct ema *_split_ema(struct ema *p, size_t at)
+{
+ typeof(p) n;
+
+ if (at <= p->start || at >= p->end)
+ return p;
+
+ n = _new_ema(p->start, at, p->source);
+ if (likely(n)) {
+ memcpy(n + 1, p + 1, _data_size);
+ p->start = at;
+ list_add_tail(&n->link, &p->link);
+ }
+ return n;
+}
+
+static int _merge_ema(struct ema *p, struct ema_map *map)
+{
+ typeof(p) prev = list_prev_entry(p, link);
+
+ WARN_ON(!mutex_is_locked(&map->lock));
+
+ if (&prev->link == &map->list || prev->end != p->start ||
+ prev->source != p->source || memcmp(prev + 1, p + 1, _data_size))
+ return 0;
+
+ p->start = prev->start;
+ fput(prev->source);
+ _free_ema(prev);
+ return 1;
+}
+
+static inline int _insert_ema(struct ema_map *map, struct ema *n)
+{
+ typeof(n) p = _find_ema(map, n->start);
+
+ if (!p)
+ list_add_tail(&n->link, &map->list);
+ else if (n->end <= p->start)
+ list_add_tail(&n->link, &p->link);
+ else
+ return -EEXIST;
+
+ _merge_ema(n, map);
+ if (p)
+ _merge_ema(p, map);
+ return 0;
+}
+
+static void ema_file_free_security(struct file *encl)
+{
+ struct ema_map *m = ema_get_map(encl);
+ if (m)
+ _free_map(m);
+}
+
+static int ema_enclave_load(struct file *encl, size_t start, size_t end,
+ size_t flags, struct vm_area_struct *vma)
+{
+ struct ema_map *m;
+ struct ema *ema;
+ int rc;
+
+ m = _init_map(encl);
+ if (unlikely(!m))
+ return -ENOMEM;
+
+ ema = _new_ema(start, end, vma ? vma->vm_file : NULL);
+ if (unlikely(!ema))
+ return -ENOMEM;
+
+ rc = ema_lock_map(m);
+ if (!rc) {
+ rc = _insert_ema(m, ema);
+ ema_unlock_map(m);
+ }
+ if (rc)
+ _free_ema(ema);
+ return rc;
+}
+
+static int ema_enclave_init(struct file *encl, struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *vma)
+{
+ if (unlikely(!_init_map(encl)))
+ return -ENOMEM;
+ return 0;
+}
+
+static struct security_hook_list ema_hooks[] __lsm_ro_after_init = {
+ LSM_HOOK_INIT(file_free_security, ema_file_free_security),
+ LSM_HOOK_INIT(enclave_load, ema_enclave_load),
+ LSM_HOOK_INIT(enclave_init, ema_enclave_init),
+};
+
+static int __init ema_init(void)
+{
+ _map_cache = kmem_cache_create("lsm-ema_map", sizeof(struct ema_map),
+ __alignof__(struct ema_map), SLAB_PANIC,
+ NULL);
+ security_add_hooks(ema_hooks, ARRAY_SIZE(ema_hooks), "ema");
+ return 0;
+}
+
+DEFINE_LSM(ema) = {
+ .name = "ema",
+ .order = LSM_ORDER_FIRST,
+ .init = ema_init,
+ .blobs = &ema_blob_sizes,
+};
+
+/* ema_request_blob shall only be called from LSM module init function */
+size_t __init ema_request_blob(size_t size)
+{
+ typeof(_data_size) offset = _data_size;
+ _data_size += size;
+ return offset;
+}
+
+struct ema_map *ema_get_map(struct file *encl)
+{
+ return (struct ema_map *)atomic_long_read(_map_file(encl));
+}
+
+/**
+ * Invoke a callback function on every EMA falls within range, split EMAs as
+ * needed
+ */
+int ema_apply_to_range(struct ema_map *map, size_t start, size_t end,
+ int (*cb)(struct ema *, void *), void *arg)
+{
+ struct ema *ema;
+ int rc;
+
+ ema = _find_ema(map, start);
+ while (ema && end > ema->start) {
+ if (start > ema->start)
+ _split_ema(ema, start);
+ if (end < ema->end)
+ ema = _split_ema(ema, end);
+
+ rc = (*cb)(ema, arg);
+ _merge_ema(ema, map);
+ if (rc)
+ return rc;
+
+ ema = _next_ema(ema, map);
+ }
+
+ if (ema)
+ _merge_ema(ema, map);
+ return 0;
+}
+
+/* Remove all EMAs falling within range, split EMAs as needed */
+void ema_remove_range(struct ema_map *map, size_t start, size_t end)
+{
+ struct ema *ema, *n;
+
+ ema = _find_ema(map, start);
+ while (ema && end > ema->start) {
+ if (start > ema->start)
+ _split_ema(ema, start);
+ if (end < ema->end)
+ ema = _split_ema(ema, end);
+
+ n = _next_ema(ema, map);
+ _free_ema(ema);
+ ema = n;
+ }
+}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around
From: Tetsuo Handa @ 2019-07-08 12:02 UTC (permalink / raw)
To: David Howells
Cc: viro, linux-api, linux-fsdevel, torvalds, ebiederm,
linux-security-module
In-Reply-To: <155059611887.17079.12991580316407924257.stgit@warthog.procyon.org.uk>
Hello, David Howells.
I realized via https://lwn.net/Articles/792622/ that a new set of
system calls for filesystem mounting has been added to Linux 5.2. But
I feel that LSM modules are not ready to support these system calls.
An example is move_mount() added by this patch. This patch added
security_move_mount() LSM hook but none of in-tree LSM modules are
providing "LSM_HOOK_INIT(move_mount, ...)" entry. Therefore, currently
security_move_mount() is a no-op. At least for TOMOYO, I want to check
mount manipulations caused by system calls because allowing mounts on
arbitrary location is not acceptable for pathname based access control.
What happened? I want TOMOYO to perform similar checks like mount() does.
On 2019/02/20 2:08, David Howells wrote:
> Add a move_mount() system call that will move a mount from one place to
> another and, in the next commit, allow to attach an unattached mount tree.
>
> The new system call looks like the following:
>
> int move_mount(int from_dfd, const char *from_path,
> int to_dfd, const char *to_path,
> unsigned int flags);
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-api@vger.kernel.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
^ permalink raw reply
* Re: [RFC 0/7] Introduce TEE based Trusted Keys support
From: Sumit Garg @ 2019-07-08 12:41 UTC (permalink / raw)
To: Jens Wiklander
Cc: corbet, dhowells, jejb, Jarkko Sakkinen, Mimi Zohar, jmorris,
serge, Ard Biesheuvel, Daniel Thompson, linux-doc,
Linux Kernel Mailing List, tee-dev, keyrings,
linux-security-module, linux-integrity
In-Reply-To: <1560421833-27414-1-git-send-email-sumit.garg@linaro.org>
Hi Jens,
On Thu, 13 Jun 2019 at 16:01, Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Add support for TEE based trusted keys where TEE provides the functionality
> to seal and unseal trusted keys using hardware unique key. Also, this is
> an alternative in case platform doesn't possess a TPM device.
>
> This series also adds some TEE features like:
>
> Patch #1, #2 enables support for registered kernel shared memory with TEE.
>
Would you like to pick up Patch #1, #2 separately? I think both these
patches add independent functionality and also got reviewed-by tags
too.
-Sumit
> Patch #3 enables support for private kernel login method required for
> cases like trusted keys where we don't wan't user-space to directly access
> TEE service to retrieve trusted key contents.
>
> Rest of the patches from #4 to #7 adds support for TEE based trusted keys.
>
> This patch-set has been tested with OP-TEE based pseudo TA which can be
> found here [1].
>
> Looking forward to your valuable feedback/suggestions.
>
> [1] https://github.com/OP-TEE/optee_os/pull/3082
>
> Sumit Garg (7):
> tee: optee: allow kernel pages to register as shm
> tee: enable support to register kernel memory
> tee: add private login method for kernel clients
> KEYS: trusted: Introduce TEE based Trusted Keys
> KEYS: encrypted: Allow TEE based trusted master keys
> doc: keys: Document usage of TEE based Trusted Keys
> MAINTAINERS: Add entry for TEE based Trusted Keys
>
> Documentation/security/keys/tee-trusted.rst | 93 +++++
> MAINTAINERS | 9 +
> drivers/tee/optee/call.c | 7 +
> drivers/tee/tee_core.c | 6 +
> drivers/tee/tee_shm.c | 16 +-
> include/keys/tee_trusted.h | 84 ++++
> include/keys/trusted-type.h | 1 +
> include/linux/tee_drv.h | 1 +
> include/uapi/linux/tee.h | 2 +
> security/keys/Kconfig | 3 +
> security/keys/Makefile | 3 +
> security/keys/encrypted-keys/masterkey_trusted.c | 10 +-
> security/keys/tee_trusted.c | 506 +++++++++++++++++++++++
> 13 files changed, 737 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/security/keys/tee-trusted.rst
> create mode 100644 include/keys/tee_trusted.h
> create mode 100644 security/keys/tee_trusted.c
>
> --
> 2.7.4
>
^ permalink raw reply
* RE: [PATCH v5 06/12] S.A.R.A.: WX protection
From: David Laight @ 2019-07-08 12:42 UTC (permalink / raw)
To: 'Salvatore Mesoraca', linux-kernel@vger.kernel.org
Cc: kernel-hardening@lists.openwall.com, linux-mm@kvack.org,
linux-security-module@vger.kernel.org, Alexander Viro,
Brad Spengler, Casey Schaufler, Christoph Hellwig, James Morris,
Jann Horn, Kees Cook, PaX Team, Serge E. Hallyn, Thomas Gleixner
In-Reply-To: <1562410493-8661-7-git-send-email-s.mesoraca16@gmail.com>
From: Salvatore Mesoraca
> Sent: 06 July 2019 11:55
...
> Executable MMAP prevention works by preventing any new executable
> allocation after the dynamic libraries have been loaded. It works under the
> assumption that, when the dynamic libraries have been finished loading, the
> RELRO section will be marked read only.
What about writing to the file of a dynamic library after it is loaded
but before it is faulted it (or after evicting it from the I$).
...
> +#define find_relro_section(ELFH, ELFP, FILE, RELRO, FOUND) do { \
> + unsigned long i; \
> + int _tmp; \
> + loff_t _pos = 0; \
> + if (ELFH.e_type == ET_DYN || ELFH.e_type == ET_EXEC) { \
> + for (i = 0; i < ELFH.e_phnum; ++i) { \
> + _pos = ELFH.e_phoff + i*sizeof(ELFP); \
> + _tmp = kernel_read(FILE, &ELFP, sizeof(ELFP), \
> + &_pos); \
> + if (_tmp != sizeof(ELFP)) \
> + break; \
> + if (ELFP.p_type == PT_GNU_RELRO) { \
> + RELRO = ELFP.p_offset >> PAGE_SHIFT; \
> + FOUND = true; \
> + break; \
> + } \
> + } \
> + } \
> +} while (0)
This is big for a #define.
Since it contains kernel_read() it can't really matter if it is
a real function.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* [GIT PULL] integrity subsystem updates for v5.3
From: Mimi Zohar @ 2019-07-08 12:58 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module, linux-integrity, linux-kernel
Hi Linus,
Included in this pull request are bug fixes, code clean up, and new
features. Listed below are the main changes.
- IMA policy rules can be defined in terms of LSM labels, making the
IMA policy dependent on LSM policy label changes, in particular LSM
label deletions. The new environment, in which IMA-appraisal is being
used, frequently updates the LSM policy and permits LSM label
deletions.
- Prevent an mmap'ed shared file opened for write from also being
mmap'ed execute. In the long term, making this and other similar
changes at the VFS layer would be preferable.
- The IMA per policy rule template format support is needed for a
couple of new/proposed features (eg. kexec boot command line
measurement, appended signatures, and VFS provided file hashes).
- Other than the "boot-aggregate" record in the IMA measuremeent list,
all other measurements are of file data. Measuring and storing the
kexec boot command line in the IMA measurement list is the first
buffer based measurement included in the measurement list.
(Stephen is carrying a patch to address a merge conflict with David's
"Keys: Set 4 - Key ACLs for 5.3").
thanks,
Mimi
The following changes since commit 8cdc23a3d9ec0944000ad43bad588e36afdc38cd:
ima: show rules with IMA_INMASK correctly (2019-05-29 23:18:25 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
for you to fetch changes up to 650b29dbdf2caf7db27cdc8bfa8fc009b28a6ce3:
integrity: Introduce struct evm_xattr (2019-06-30 17:54:41 -0400)
----------------------------------------------------------------
Geert Uytterhoeven (1):
integrity: Fix __integrity_init_keyring() section mismatch
Janne Karhunen (2):
LSM: switch to blocking policy update notifiers
ima: use the lsm policy update notifier
Matthew Garrett (1):
IMA: support for per policy rule template formats
Mimi Zohar (2):
x86/ima: check EFI SetupMode too
ima: prevent a file already mmap'ed write to be mmap'ed execute
Nayna Jain (1):
x86/ima: fix the Kconfig dependency for IMA_ARCH_POLICY
Prakhar Srivastava (3):
IMA: Define a new hook to measure the kexec boot command line arguments
IMA: Define a new template field buf
KEXEC: Call ima_kexec_cmdline to measure the boot command line args
Thiago Jung Bauermann (3):
ima: Use designated initializers for struct ima_event_data
ima: Update MAX_TEMPLATE_NAME_LEN to fit largest reasonable definition
integrity: Introduce struct evm_xattr
YueHaibing (1):
ima: Make arch_policy_entry static
Documentation/ABI/testing/ima_policy | 6 +-
Documentation/security/IMA-templates.rst | 7 +-
arch/x86/kernel/ima_arch.c | 12 ++-
drivers/infiniband/core/device.c | 6 +-
include/linux/ima.h | 2 +
include/linux/security.h | 12 +--
kernel/kexec_file.c | 9 +-
security/integrity/digsig.c | 5 +-
security/integrity/evm/evm_main.c | 8 +-
security/integrity/ima/Kconfig | 3 +-
security/integrity/ima/ima.h | 21 +++-
security/integrity/ima/ima_api.c | 38 +++++--
security/integrity/ima/ima_appraise.c | 9 +-
security/integrity/ima/ima_init.c | 6 +-
security/integrity/ima/ima_main.c | 123 ++++++++++++++++++++--
security/integrity/ima/ima_policy.c | 163 +++++++++++++++++++++++++-----
security/integrity/ima/ima_template.c | 23 +++--
security/integrity/ima/ima_template_lib.c | 21 ++++
security/integrity/ima/ima_template_lib.h | 4 +
security/integrity/integrity.h | 6 ++
security/security.c | 23 +++--
security/selinux/hooks.c | 2 +-
security/selinux/selinuxfs.c | 2 +-
23 files changed, 413 insertions(+), 98 deletions(-)
^ permalink raw reply
* Re: [PATCH 02/10] vfs: syscall: Add move_mount(2) to move mounts around
From: Al Viro @ 2019-07-08 13:18 UTC (permalink / raw)
To: Tetsuo Handa
Cc: David Howells, linux-api, linux-fsdevel, torvalds, ebiederm,
linux-security-module
In-Reply-To: <c5b901ca-c243-bf80-91be-a794c4433415@I-love.SAKURA.ne.jp>
On Mon, Jul 08, 2019 at 09:02:10PM +0900, Tetsuo Handa wrote:
> Hello, David Howells.
>
> I realized via https://lwn.net/Articles/792622/ that a new set of
> system calls for filesystem mounting has been added to Linux 5.2. But
> I feel that LSM modules are not ready to support these system calls.
>
> An example is move_mount() added by this patch. This patch added
> security_move_mount() LSM hook but none of in-tree LSM modules are
> providing "LSM_HOOK_INIT(move_mount, ...)" entry. Therefore, currently
> security_move_mount() is a no-op. At least for TOMOYO, I want to check
> mount manipulations caused by system calls because allowing mounts on
> arbitrary location is not acceptable for pathname based access control.
> What happened? I want TOMOYO to perform similar checks like mount() does.
That would be like tomoyo_check_mount_acl(), right?
if (need_dev) {
/* Get mount point or device file. */
if (!dev_name || kern_path(dev_name, LOOKUP_FOLLOW, &path)) {
error = -ENOENT;
goto out;
}
obj.path1 = path;
requested_dev_name = tomoyo_realpath_from_path(&path);
if (!requested_dev_name) {
error = -ENOENT;
goto out;
}
} else {
is an obvious crap for *ALL* cases. You are doing pathname resolution,
followed by normalization and checks. Then the result of said pathname
resolution is thrown out and it's redone (usually by something in fs/super.c).
Results of _that_ get used.
Could you spell TOCTOU? And yes, exploiting that takes a lot less than
being able to do mount(2) in the first place - just pass it
/proc/self/fd/69/<some acceptable path>/. with descriptor refering to
opened root directory. With ~/<some acceptable path> being a symlink
to whatever you actually want to hit. And descriptor 42 being your
opened homedir. Now have that call of mount(2) overlap with dup2(42, 69)
from another thread sharing your descriptor table. It doesn't take much
to get the timing right, especially if you can arrange for some other
activity frequently hitting namespace_sem at least shared (e.g. reading
/proc/mounts in a loop from another process); that's likely to stall
mount(2) at the point of lock_mount(), which comes *AFTER* the point
where LSM hook is stuck into.
Again, *ANY* checks on "dev_name" in ->sb_mount() instances are so much
snake oil. Always had been.
^ permalink raw reply
* Re: [RFC PATCH v4 12/12] LSM: x86/sgx: Show line of sight to LSM support SGX2's EAUG
From: Sean Christopherson @ 2019-07-08 14:34 UTC (permalink / raw)
To: Xing, Cedric
Cc: Jarkko Sakkinen, linux-sgx@vger.kernel.org,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
Roberts, William C, Schaufler, Casey, James Morris, Hansen, Dave,
Andy Lutomirski, Jethro Beekman, Dr . Greg Wettstein,
Stephen Smalley
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F655184EC@ORSMSX116.amr.corp.intel.com>
On Fri, Jun 21, 2019 at 10:18:55AM -0700, Xing, Cedric wrote:
> > From: Christopherson, Sean J
> > Sent: Wednesday, June 19, 2019 3:24 PM
> >
> > diff --git a/arch/x86/kernel/cpu/sgx/driver/main.c b/arch/x86/kernel/cpu/sgx/driver/main.c
> > index 4379a2fb1f82..b478c0f45279 100644
> > --- a/arch/x86/kernel/cpu/sgx/driver/main.c
> > +++ b/arch/x86/kernel/cpu/sgx/driver/main.c
> > @@ -99,7 +99,8 @@ static long sgx_compat_ioctl(struct file *filep, unsigned int cmd,
> > * page is considered to have no RWX permissions, i.e. is inaccessible.
> > */
> > static unsigned long sgx_allowed_rwx(struct sgx_encl *encl,
> > - struct vm_area_struct *vma)
> > + struct vm_area_struct *vma,
> > + bool *eaug)
> > {
> > unsigned long allowed_rwx = VM_READ | VM_WRITE | VM_EXEC;
> > unsigned long idx, idx_start, idx_end; @@ -123,6 +124,8 @@ static unsigned long
> > sgx_allowed_rwx(struct sgx_encl *encl,
> > allowed_rwx = 0;
> > else
> > allowed_rwx &= page->vm_prot_bits;
> > + if (page->vm_prot_bits & SGX_VM_EAUG)
> > + *eaug = true;
> > if (!allowed_rwx)
> > break;
> > }
> > @@ -134,16 +137,17 @@ static int sgx_mmap(struct file *file, struct vm_area_struct *vma)
> > {
> > struct sgx_encl *encl = file->private_data;
> > unsigned long allowed_rwx, prot;
> > + bool eaug = false;
> > int ret;
> >
> > - allowed_rwx = sgx_allowed_rwx(encl, vma);
> > + allowed_rwx = sgx_allowed_rwx(encl, vma, &eaug);
> > if (vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC) & ~allowed_rwx)
> > return -EACCES;
>
> IIUC, "eaug range" has to be mapped PROT_NONE, then vm_ops->fault() won't be
> invoked. Am I correct? Then how to EAUG on #PF?
Pages tagged SGX_VM_EAUG also have maximal permissions and can be mapped
PROT_{READ,WRITE,EXEC} accordingly.
>
> >
> > prot = _calc_vm_trans(vma->vm_flags, VM_READ, PROT_READ) |
> > _calc_vm_trans(vma->vm_flags, VM_WRITE, PROT_WRITE) |
> > _calc_vm_trans(vma->vm_flags, VM_EXEC, PROT_EXEC);
> > - ret = security_enclave_map(prot);
> > + ret = security_enclave_map(prot, eaug);
> > if (ret)
> > return ret;
> >
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox