* Re: [PATCH 00/58] LSM: Module stacking for AppArmor
From: Casey Schaufler @ 2019-06-04 19:58 UTC (permalink / raw)
To: Stephen Smalley, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul, casey
In-Reply-To: <2a9049a7-6259-5ae0-2790-0aaf337c51a4@tycho.nsa.gov>
On 6/4/2019 10:11 AM, Stephen Smalley wrote:
> On 6/4/19 12:14 PM, Casey Schaufler wrote:
>> On 6/4/2019 5:29 AM, Stephen Smalley wrote:
>>> On 6/2/19 12:50 PM, Casey Schaufler wrote:
>>>> This patchset provides the changes required for
>>>> the AppArmor security module to stack safely with any other.
>>>
>>> Please explain the motivation
>>
>> I'll add some explanation for the next revision.
>> It won't be anything that I haven't posted many times
>> before, but you're right that it belongs in the log.
>>
>>> - why do we want to allow AppArmor to stack with other modules,
>>
>> First, is there a reason not to? Sure, you can confuse
>> administrators by implementing complex security policies,
>> but there are lots of ways to do that already.
>
> There are costs to doing so, e.g.
> - greater complexity in the security framework,
Taking blob management out of the modules and
into the framework makes simplifies the modules.
> - possibly greater memory and runtime overheads,
Possibly reduced memory and runtime overheads, as well.
> - potential user confusion (which security module(s) caused a given failure?)
That's not new. I've seen countless cases where users blame
SELinux or Smack when the problem is with mode bits and/or
capabilities. Not to mention that a good 50% of current Linux
users don't understand any of the security mechanisms to
begin with.
> - potential distro maintainer burden
Selection of security modules and how they are configured
has always been a burden for distro developers and maintainers.
Nothing new here.
And, they knew the job was dangerous when they took it.
> (similar to above, but performing triage when any given permission denial can have multiple causes beyond just DAC + one module, weird interactions among modules, etc)
Yama has been widely accepted by distros, and civilization
has yet to have officially been declared ended.
> It isn't free so there should be a cost/benefit analysis.
Some benchmarking is definitely in order, but most
of what's you're calling out as downside is hypothetical
or based on assumption.
>
>>
>> AppArmor provides a different security model than SELinux,
>> TOMOYO or Smack. Smack is better at system component
>> separation, while AppArmor is better at application isolation.
>> It's a win to use each to its strength rather than trying to
>> stretch either to the edge of what it can do.
>>
>>> who would use it,
>>
>> Can't name names, but there have been multiple requests.
>>
>>> how would it be used,
>>
>> As mentioned above, Smack for system separation, AppArmor for
>> application isolation.
>
> Can you provide a concrete example of how combining the two yields a smaller, simpler configuration overall than using them individually?
Smack + AppArmor is a simpler, smaller model than the Smack policy
used in Tizen 2.
>
>>
>>> what does it provide that isn't already possible in the absence of it.
>>
>> It's not necessary that something be impossible to do any
>> other way. The question should be whether this provides for
>> a better way to achieve the goals, and this does that.
>> If I tried the come up with something that's impossible I
>> would expect the usual "you can do that with SELinux policy"
>> argument. We know we can do things. We want to have the tools
>> to do them better.
>>
>>> Also, Ubuntu fully upstreamed all of their changes to AppArmor, would this still suffice to enable stacking of AppArmor or do they rely on hooks that are not handled here?
>>
>> Some amount of merging will likely be required. But that's
>> always going to be true with parallel development tracks.
>> That's why we have git!
>>
>>> Please explain the cost of the change - what do we pay in terms of memory, runtime, or other overheads in order to support this change?
>>
>> Do you have particular benchmarks you want to see?
>> When I've supplied numbers in the past they have not
>> been remarked on.
>
> A combination of micro and macro benchmarks exercising multiple kernel subsystems would be good. Kernel build time isn't sufficient.
Do you have preferences, or better yet, facilities
for running them? I am, alas, running on finite resources
and benchmark contributions, especially in areas where you
have specific concerns, would be most welcome.
>
>>
>>>
>>>>
>>>> A new process attribute identifies which security module
>>>> information should be reported by SO_PEERSEC and the
>>>> /proc/.../attr/current interface. This is provided by
>>>> /proc/.../attr/display. Writing the name of the security
>>>> module desired to this interface will set which LSM hooks
>>>> will be called for this information. The first security
>>>> module providing the hooks will be used by default.
>>>
>>> Doesn't this effectively undo making the hooks read-only after init, at least for the subset involved? What are the security implications thereof?
>>
>> Any mechanism, be it a separate set of hooks, a name used to
>> do list look ups, or an sophisticated hash scheme will have that
>> impact for the processes that use it. This scheme has the best
>> performance profile of the mechanisms I experimented with and
>> avoids all sorts of special cases.
>>
>>>
>>>> The use of integer based security tokens (secids) is
>>>> generally (but not completely) replaced by a structure
>>>> lsm_export. The lsm_export structure can contain information
>>>> for each of the security modules that export information
>>>> outside the LSM layer.
>>>>
>>>> The LSM interfaces that provide "secctx" text strings
>>>> have been changed to use a structure "lsm_context"
>>>> instead of a pointer/length pair. In some cases the
>>>> interfaces used a "char *" pointer and in others a
>>>> "void *". This was necessary to ensure that the correct
>>>> release mechanism for the text is used. It also makes
>>>> many of the interfaces cleaner.
>>>>
>>>> https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
>>>>
>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>> ---
>>>> drivers/android/binder.c | 25 ++-
>>>> fs/kernfs/dir.c | 6 +-
>>>> fs/kernfs/inode.c | 31 ++-
>>>> fs/kernfs/kernfs-internal.h | 3 +-
>>>> fs/nfs/inode.c | 13 +-
>>>> fs/nfs/internal.h | 8 +-
>>>> fs/nfs/nfs4proc.c | 17 +-
>>>> fs/nfs/nfs4xdr.c | 16 +-
>>>> fs/nfsd/nfs4proc.c | 8 +-
>>>> fs/nfsd/nfs4xdr.c | 14 +-
>>>> fs/nfsd/vfs.c | 7 +-
>>>> fs/proc/base.c | 1 +
>>>> include/linux/cred.h | 3 +-
>>>> include/linux/lsm_hooks.h | 91 +++++----
>>>> include/linux/nfs4.h | 8 +-
>>>> include/linux/security.h | 133 +++++++++----
>>>> include/net/af_unix.h | 2 +-
>>>> include/net/netlabel.h | 10 +-
>>>> include/net/scm.h | 14 +-
>>>> kernel/audit.c | 43 ++--
>>>> kernel/audit.h | 9 +-
>>>> kernel/auditfilter.c | 6 +-
>>>> kernel/auditsc.c | 77 ++++----
>>>> kernel/cred.c | 15 +-
>>>> net/ipv4/cipso_ipv4.c | 13 +-
>>>> net/ipv4/ip_sockglue.c | 12 +-
>>>> net/netfilter/nf_conntrack_netlink.c | 29 ++-
>>>> net/netfilter/nf_conntrack_standalone.c | 16 +-
>>>> net/netfilter/nfnetlink_queue.c | 38 ++--
>>>> net/netfilter/nft_meta.c | 13 +-
>>>> net/netfilter/xt_SECMARK.c | 14 +-
>>>> net/netlabel/netlabel_kapi.c | 5 +-
>>>> net/netlabel/netlabel_unlabeled.c | 101 +++++-----
>>>> net/netlabel/netlabel_unlabeled.h | 2 +-
>>>> net/netlabel/netlabel_user.c | 13 +-
>>>> net/netlabel/netlabel_user.h | 2 +-
>>>> net/unix/af_unix.c | 6 +-
>>>> security/apparmor/audit.c | 4 +-
>>>> security/apparmor/include/audit.h | 2 +-
>>>> security/apparmor/include/net.h | 6 +-
>>>> security/apparmor/include/secid.h | 9 +-
>>>> security/apparmor/lsm.c | 64 +++---
>>>> security/apparmor/secid.c | 42 ++--
>>>> security/integrity/ima/ima.h | 14 +-
>>>> security/integrity/ima/ima_api.c | 9 +-
>>>> security/integrity/ima/ima_appraise.c | 6 +-
>>>> security/integrity/ima/ima_main.c | 34 ++--
>>>> security/integrity/ima/ima_policy.c | 19 +-
>>>> security/security.c | 338 +++++++++++++++++++++++++++-----
>>>> security/selinux/hooks.c | 259 ++++++++++++------------
>>>> security/selinux/include/audit.h | 5 +-
>>>> security/selinux/include/objsec.h | 42 +++-
>>>> security/selinux/netlabel.c | 25 +--
>>>> security/selinux/ss/services.c | 18 +-
>>>> security/smack/smack.h | 18 ++
>>>> security/smack/smack_lsm.c | 238 +++++++++++-----------
>>>> security/smack/smack_netfilter.c | 8 +-
>>>> security/smack/smackfs.c | 12 +-
>>>> 58 files changed, 1217 insertions(+), 779 deletions(-)
>>>>
>>>
>
^ permalink raw reply
* Re: possible deadlock in __do_page_fault (2)
From: syzbot @ 2019-06-04 20:07 UTC (permalink / raw)
To: adilger.kernel, hdanton, jmorris, linux-ext4, linux-kernel,
linux-security-module, serge, syzkaller-bugs, tytso, zohar
In-Reply-To: <000000000000fa91e1058a358cd5@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: 56b697c6 Add linux-next specific files for 20190604
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=12d210d4a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=4248d6bc70076f7d
dashboard link: https://syzkaller.appspot.com/bug?extid=606e524a3ca9617cf8c0
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17ddae0ea00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13e78f16a00000
The bug was bisected to:
commit 69d61f577d147b396be0991b2ac6f65057f7d445
Author: Mimi Zohar <zohar@linux.ibm.com>
Date: Wed Apr 3 21:47:46 2019 +0000
ima: verify mprotect change is consistent with mmap policy
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=16461c5aa00000
final crash: https://syzkaller.appspot.com/x/report.txt?x=15461c5aa00000
console output: https://syzkaller.appspot.com/x/log.txt?x=11461c5aa00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+606e524a3ca9617cf8c0@syzkaller.appspotmail.com
Fixes: 69d61f577d14 ("ima: verify mprotect change is consistent with mmap
policy")
======================================================
WARNING: possible circular locking dependency detected
5.2.0-rc3-next-20190604 #8 Not tainted
------------------------------------------------------
syz-executor361/10886 is trying to acquire lock:
00000000c4471342 (&mm->mmap_sem#2){++++}, at: do_user_addr_fault
arch/x86/mm/fault.c:1406 [inline]
00000000c4471342 (&mm->mmap_sem#2){++++}, at: __do_page_fault+0x9e9/0xda0
arch/x86/mm/fault.c:1521
but task is already holding lock:
000000009878d2b3 (&sb->s_type->i_mutex_key#10){+.+.}, at: inode_trylock
include/linux/fs.h:798 [inline]
000000009878d2b3 (&sb->s_type->i_mutex_key#10){+.+.}, at:
ext4_file_write_iter+0x246/0x1070 fs/ext4/file.c:232
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&sb->s_type->i_mutex_key#10){+.+.}:
down_write+0x38/0xa0 kernel/locking/rwsem.c:66
inode_lock include/linux/fs.h:778 [inline]
process_measurement+0x15ae/0x15e0
security/integrity/ima/ima_main.c:228
ima_file_mmap+0x11a/0x130 security/integrity/ima/ima_main.c:370
security_file_mprotect+0xd5/0x100 security/security.c:1426
do_mprotect_pkey+0x537/0xa30 mm/mprotect.c:550
__do_sys_mprotect mm/mprotect.c:582 [inline]
__se_sys_mprotect mm/mprotect.c:579 [inline]
__x64_sys_mprotect+0x78/0xb0 mm/mprotect.c:579
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
-> #0 (&mm->mmap_sem#2){++++}:
lock_acquire+0x16f/0x3f0 kernel/locking/lockdep.c:4300
down_read+0x3f/0x1e0 kernel/locking/rwsem.c:24
do_user_addr_fault arch/x86/mm/fault.c:1406 [inline]
__do_page_fault+0x9e9/0xda0 arch/x86/mm/fault.c:1521
do_page_fault+0x71/0x57d arch/x86/mm/fault.c:1552
page_fault+0x1e/0x30 arch/x86/entry/entry_64.S:1156
fault_in_pages_readable include/linux/pagemap.h:600 [inline]
iov_iter_fault_in_readable+0x377/0x450 lib/iov_iter.c:426
generic_perform_write+0x186/0x520 mm/filemap.c:3197
__generic_file_write_iter+0x25e/0x630 mm/filemap.c:3336
ext4_file_write_iter+0x332/0x1070 fs/ext4/file.c:266
call_write_iter include/linux/fs.h:1870 [inline]
new_sync_write+0x4d3/0x770 fs/read_write.c:483
__vfs_write+0xe1/0x110 fs/read_write.c:496
vfs_write+0x268/0x5d0 fs/read_write.c:558
ksys_write+0x14f/0x290 fs/read_write.c:611
__do_sys_write fs/read_write.c:623 [inline]
__se_sys_write fs/read_write.c:620 [inline]
__x64_sys_write+0x73/0xb0 fs/read_write.c:620
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&sb->s_type->i_mutex_key#10);
lock(&mm->mmap_sem#2);
lock(&sb->s_type->i_mutex_key#10);
lock(&mm->mmap_sem#2);
*** DEADLOCK ***
3 locks held by syz-executor361/10886:
#0: 00000000b8e64295 (&f->f_pos_lock){+.+.}, at: __fdget_pos+0xee/0x110
fs/file.c:801
#1: 00000000e032805f (sb_writers#3){.+.+}, at: file_start_write
include/linux/fs.h:2836 [inline]
#1: 00000000e032805f (sb_writers#3){.+.+}, at: vfs_write+0x485/0x5d0
fs/read_write.c:557
#2: 000000009878d2b3 (&sb->s_type->i_mutex_key#10){+.+.}, at:
inode_trylock include/linux/fs.h:798 [inline]
#2: 000000009878d2b3 (&sb->s_type->i_mutex_key#10){+.+.}, at:
ext4_file_write_iter+0x246/0x1070 fs/ext4/file.c:232
stack backtrace:
CPU: 0 PID: 10886 Comm: syz-executor361 Not tainted 5.2.0-rc3-next-20190604
#8
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
print_circular_bug.cold+0x1cc/0x28f kernel/locking/lockdep.c:1566
check_prev_add kernel/locking/lockdep.c:2311 [inline]
check_prevs_add kernel/locking/lockdep.c:2419 [inline]
validate_chain kernel/locking/lockdep.c:2801 [inline]
__lock_acquire+0x3755/0x5490 kernel/locking/lockdep.c:3790
lock_acquire+0x16f/0x3f0 kernel/locking/lockdep.c:4300
down_read+0x3f/0x1e0 kernel/locking/rwsem.c:24
do_user_addr_fault arch/x86/mm/fault.c:1406 [inline]
__do_page_fault+0x9e9/0xda0 arch/x86/mm/fault.c:1521
do_page_fault+0x71/0x57d arch/x86/mm/fault.c:1552
page_fault+0x1e/0x30 arch/x86/entry/entry_64.S:1156
RIP: 0010:fault_in_pages_readable include/linux/pagemap.h:600 [inline]
RIP: 0010:iov_iter_fault_in_readable+0x377/0x450 lib/iov_iter.c:426
Code: 89 f6 41 88 57 e0 e8 b8 dd 3c fe 45 85 f6 74 c1 e9 70 fe ff ff e8 29
dc 3c fe 0f 1f 00 0f ae e8 44 89 f0 48 8b 8d 68 ff ff ff <8a> 11 89 c3 0f
1f 00 41 88 57 d0 31 ff 89 de e8 85 dd 3c fe 85 db
RSP: 0018:ffff88808ca6f918 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00000000200011ff
RDX: 0000000000000000 RSI: ffffffff83341b37 RDI: 0000000000000007
RBP: ffff88808ca6f9b8 R08: ffff88809be68000 R09: ffff88809be688f8
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000001000
R13: 0000000000001000 R14: 0000000000000000 R15: ffff88808ca6f990
generic_perform_write+0x186/0x520 mm/filemap.c:3197
__generic_file_write_iter+0x25e/0x630 mm/filemap.c:3336
ext4_file_write_iter+0x332/0x1070 fs/ext4/file.c:266
call_write_iter include/linux/fs.h:1870 [inline]
new_sync_write+0x4d3/0x770 fs/read_write.c:483
__vfs_write+0xe1/0x110 fs/read_write.c:496
vfs_write+0x268/0x5d0 fs/read_write.c:558
ksys_write+0x14f/0x290 fs/read_write.c:611
__do_sys_write fs/read_write.c:623 [inline]
__se_sys_write fs/read_write.c:620 [inline]
__x64_sys_write+0x73/0xb0 fs/read_write.c:620
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x446939
Code: e8 4c b4 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 0b 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007effabbcdce8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00000000006dbc38 RCX: 0000000000446939
RDX: 000000000007fe00 RSI: 0000000020000200 RDI: 0000000000000006
RBP: 00000000006dbc30 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dbc3c
R13: 00007ffd5dd15ddf R14: 00007effabbce9c0 R15: 20c49ba5e353f7cf
^ permalink raw reply
* Re: possible deadlock in __do_page_fault (2)
From: Mimi Zohar @ 2019-06-04 20:08 UTC (permalink / raw)
To: syzbot, adilger.kernel, hdanton, jmorris, linux-ext4,
linux-kernel, linux-security-module, serge, syzkaller-bugs, tytso
In-Reply-To: <000000000000a7a51a058a728a6c@google.com>
On Mon, 2019-06-03 at 15:04 -0700, syzbot wrote:
> syzbot has bisected this bug to:
>
> commit 69d61f577d147b396be0991b2ac6f65057f7d445
> Author: Mimi Zohar <zohar@linux.ibm.com>
> Date: Wed Apr 3 21:47:46 2019 +0000
>
> ima: verify mprotect change is consistent with mmap policy
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=16461c5aa00000
> start commit: 3c09c195 Add linux-next specific files for 20190531
> git tree: linux-next
> final crash: https://syzkaller.appspot.com/x/report.txt?x=15461c5aa00000
> console output: https://syzkaller.appspot.com/x/log.txt?x=11461c5aa00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=6cfb24468280cd5c
> dashboard link: https://syzkaller.appspot.com/bug?extid=606e524a3ca9617cf8c0
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10572ca6a00000
>
> Reported-by: syzbot+606e524a3ca9617cf8c0@syzkaller.appspotmail.com
> Fixes: 69d61f577d14 ("ima: verify mprotect change is consistent with mmap
> policy")
>
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
Thank you for the report.
Mimi
^ permalink raw reply
* Re: [RFC PATCH 2/9] x86/sgx: Do not naturally align MAP_FIXED address
From: Andy Lutomirski @ 2019-06-04 20:16 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Sean Christopherson, Andy Lutomirski, Cedric Xing,
Stephen Smalley, James Morris, Serge E . Hallyn, LSM List,
Paul Moore, Eric Paris, selinux, Jethro Beekman, Dave Hansen,
Thomas Gleixner, Linus Torvalds, LKML, X86 ML, linux-sgx,
Andrew Morton, nhorman, npmccallum, Serge Ayoun, Shay Katz-zamir,
Haitao Huang, Andy Shevchenko, Kai Svahn, Borislav Petkov,
Josh Triplett, Kai Huang, David Rientjes, William Roberts,
Philip Tricca
In-Reply-To: <20190604114951.GC30594@linux.intel.com>
On Tue, Jun 4, 2019 at 4:50 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Fri, May 31, 2019 at 04:31:52PM -0700, Sean Christopherson wrote:
> > SGX enclaves have an associated Enclave Linear Range (ELRANGE) that is
> > tracked and enforced by the CPU using a base+mask approach, similar to
> > how hardware range registers such as the variable MTRRs. As a result,
> > the ELRANGE must be naturally sized and aligned.
> >
> > To reduce boilerplate code that would be needed in every userspace
> > enclave loader, the SGX driver naturally aligns the mmap() address and
> > also requires the range to be naturally sized. Unfortunately, SGX fails
> > to grant a waiver to the MAP_FIXED case, e.g. incorrectly rejects mmap()
> > if userspace is attempting to map a small slice of an existing enclave.
> >
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
>
> Why you want to allow mmap() to be called multiple times? mmap() could
> be allowed only once with PROT_NONE and denied afterwards. Is this for
> sending fd to another process that would map already existing enclave?
>
> I don't see any checks for whether the is enclave underneath. Also, I
> think that in all cases mmap() callback should allow only PROT_NONE
> as permissions for clarity even if it could called multiple times.
>
What's the advantage to only allowing PROT_NONE? The idea here is to
allow a PROT_NONE map followed by some replacemets that overlay it for
the individual segments. Admittedly, mprotect() can do the same
thing, but disallowing mmap() seems at least a bit surprising.
^ permalink raw reply
* Re: [RFC PATCH 3/9] x86/sgx: Allow userspace to add multiple pages in single ioctl()
From: Andy Lutomirski @ 2019-06-04 20:18 UTC (permalink / raw)
To: Sean Christopherson
Cc: Xing, Cedric, Jarkko Sakkinen, Andy Lutomirski, Stephen Smalley,
James Morris, Serge E . Hallyn, LSM List, Paul Moore, Eric Paris,
selinux@vger.kernel.org, Jethro Beekman, Hansen, Dave,
Thomas Gleixner, Linus Torvalds, LKML, X86 ML,
linux-sgx@vger.kernel.org, Andrew Morton, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, Andy Shevchenko, Svahn, Kai, Borislav Petkov,
Josh Triplett, Huang, Kai, David Rientjes, Roberts, William C,
Tricca, Philip B
In-Reply-To: <20190603203950.GJ13384@linux.intel.com>
On Mon, Jun 3, 2019 at 1:39 PM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Mon, Jun 03, 2019 at 01:08:04PM -0700, Sean Christopherson wrote:
> > On Sun, Jun 02, 2019 at 11:26:09PM -0700, Xing, Cedric wrote:
> > > > From: Christopherson, Sean J
> > > > Sent: Friday, May 31, 2019 4:32 PM
> > > >
> > > > +/**
> > > > + * sgx_ioc_enclave_add_pages - handler for %SGX_IOC_ENCLAVE_ADD_PAGES
> > > > + *
> > > > + * @filep: open file to /dev/sgx
> > > > + * @cmd: the command value
> > > > + * @arg: pointer to an &sgx_enclave_add_page instance
> > > > + *
> > > > + * Add a range of pages to an uninitialized enclave (EADD), and
> > > > +optionally
> > > > + * extend the enclave's measurement with the contents of the page (EEXTEND).
> > > > + * The range of pages must be virtually contiguous. The SECINFO and
> > > > + * measurement maskare applied to all pages, i.e. pages with different
> > > > + * properties must be added in separate calls.
> > > > + *
> > > > + * EADD and EEXTEND are done asynchronously via worker threads. A
> > > > +successful
> > > > + * sgx_ioc_enclave_add_page() only indicates the pages have been added
> > > > +to the
> > > > + * work queue, it does not guarantee adding the pages to the enclave
> > > > +will
> > > > + * succeed.
> > > > + *
> > > > + * Return:
> > > > + * 0 on success,
> > > > + * -errno otherwise
> > > > + */
> > > > +static long sgx_ioc_enclave_add_pages(struct file *filep, unsigned int cmd,
> > > > + unsigned long arg)
> > > > +{
> > > > + struct sgx_enclave_add_pages *addp = (void *)arg;
> > > > + struct sgx_encl *encl = filep->private_data;
> > > > + struct sgx_secinfo secinfo;
> > > > + unsigned int i;
> > > > + int ret;
> > > > +
> > > > + if (copy_from_user(&secinfo, (void __user *)addp->secinfo,
> > > > + sizeof(secinfo)))
> > > > + return -EFAULT;
> > > > +
> > > > + for (i = 0, ret = 0; i < addp->nr_pages && !ret; i++) {
> > > > + if (signal_pending(current))
> > > > + return -ERESTARTSYS;
> > >
> > > If interrupted, how would user mode code know how many pages have been EADD'ed?
> >
> > Hmm, updating nr_pages would be fairly simple and shouldn't confuse
> > userspace, e.g. as opposed to overloading the return value.
>
> Or maybe update @addr and @src as well? That would allow userspace to
> re-invoke the ioctl() without having to modify the struct.
If you're going to use -ERESTARTSYS, that's the way to go. -EINTR
would be an alternative. A benefit of -ERESTARTSYS is that, with
-EINTR, it wouldn't be that surprising for user code to simply fail to
handle it.
^ permalink raw reply
* Re: [RFC PATCH 6/9] x86/sgx: Require userspace to provide allowed prots to ADD_PAGES
From: Andy Lutomirski @ 2019-06-04 20:23 UTC (permalink / raw)
To: Sean Christopherson
Cc: Jarkko Sakkinen, Andy Lutomirski, Cedric Xing, Stephen Smalley,
James Morris, Serge E . Hallyn, LSM List, Paul Moore, Eric Paris,
selinux, Jethro Beekman, Dave Hansen, Thomas Gleixner,
Linus Torvalds, LKML, X86 ML, linux-sgx, Andrew Morton, nhorman,
npmccallum, Serge Ayoun, Shay Katz-zamir, Haitao Huang,
Andy Shevchenko, Kai Svahn, Borislav Petkov, Josh Triplett,
Kai Huang, David Rientjes, William Roberts, Philip Tricca
In-Reply-To: <20190531233159.30992-7-sean.j.christopherson@intel.com>
On Fri, May 31, 2019 at 4:32 PM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> ...to support (the equivalent) of existing Linux Security Module
> functionality.
>
> Because SGX manually manages EPC memory, all enclave VMAs are backed by
> the same vm_file, i.e. /dev/sgx/enclave, so that SGX can implement the
> necessary hooks to move pages in/out of the EPC. And because EPC pages
> for any given enclave are fundamentally shared between processes, i.e.
> CoW semantics are not possible with EPC pages, /dev/sgx/enclave must
> always be MAP_SHARED. Lastly, all real world enclaves will need read,
> write and execute permissions to EPC pages. As a result, SGX does not
> play nice with existing LSM behavior as it is impossible to apply
> policies to enclaves with any reasonable granularity, e.g. an LSM can
> deny access to EPC altogether, but can't deny potentially dangerous
> behavior such as mapping pages RW->RW or RWX.
>
> To give LSMs enough information to implement their policies without
> having to resort to ugly things, e.g. holding a reference to the vm_file
> of each enclave page, require userspace to explicitly state the allowed
> protections for each page (region), i.e. take ALLOW_{READ,WRITE,EXEC}
> in the ADD_PAGES ioctl.
>
> The ALLOW_* flags will be passed to LSMs so that they can make informed
> decisions when the enclave is being built, i.e. when the source vm_file
> is available. For example, SELinux's EXECMOD permission can be
> required if an enclave is requesting both ALLOW_WRITE and ALLOW_EXEC.
>
> Update the mmap()/mprotect() hooks to enforce the ALLOW_* protections,
> a la the standard VM_MAY{READ,WRITE,EXEC} flags.
>
> The ALLOW_EXEC flag also has a second (important) use in that it can
> be used to prevent loading an enclave from a noexec file system, on
> SGX2 hardware (regardless of kernel support for SGX2), userspace could
> EADD from a noexec path using read-only permissions and later mprotect()
> and ENCLU[EMODPE] the page to gain execute permissions. By requiring
> ALLOW_EXEC up front, SGX will be able to enforce noexec paths when
> building the enclave.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
> arch/x86/include/uapi/asm/sgx.h | 9 ++++++++-
> arch/x86/kernel/cpu/sgx/driver/ioctl.c | 23 +++++++++++++++++------
> arch/x86/kernel/cpu/sgx/encl.c | 2 +-
> arch/x86/kernel/cpu/sgx/encl.h | 1 +
> 4 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
> index 4a12d6abbcb7..4489e92fa0dc 100644
> --- a/arch/x86/include/uapi/asm/sgx.h
> +++ b/arch/x86/include/uapi/asm/sgx.h
> @@ -31,6 +31,11 @@ struct sgx_enclave_create {
> __u64 src;
> };
>
> +/* Supported flags for struct sgx_enclave_add_pages. */
> +#define SGX_ALLOW_READ VM_READ
> +#define SGX_ALLOW_WRITE VM_WRITE
> +#define SGX_ALLOW_EXEC VM_EXEC
> +
> /**
> * struct sgx_enclave_add_pages - parameter structure for the
> * %SGX_IOC_ENCLAVE_ADD_PAGES ioctl
> @@ -39,6 +44,7 @@ struct sgx_enclave_create {
> * @secinfo: address for the SECINFO data (common to all pages)
> * @nr_pages: number of pages (must be virtually contiguous)
> * @mrmask: bitmask for the measured 256 byte chunks (common to all pages)
> + * @flags: flags, e.g. SGX_ALLOW_{READ,WRITE,EXEC} (common to all pages)
> */
> struct sgx_enclave_add_pages {
> __u64 addr;
> @@ -46,7 +52,8 @@ struct sgx_enclave_add_pages {
> __u64 secinfo;
> __u32 nr_pages;
> __u16 mrmask;
> -} __attribute__((__packed__));
> + __u16 flags;
Minor nit: I would use at least u32 for flags. It's not like the size
saving is important.
> static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr,
> unsigned long src, struct sgx_secinfo *secinfo,
> - unsigned int mrmask)
> + unsigned int mrmask, unsigned int flags)
> {
> + unsigned long prot = secinfo->flags & (VM_READ | VM_WRITE | VM_EXEC);
> + unsigned long allowed_prot = flags & (VM_READ | VM_WRITE | VM_EXEC);
...
> + if (prot & ~allowed_prot)
> + return -EACCES;
This seems like a well-intentioned sanity check, but it doesn't really
accomplish anything with SGX2, so I tend to think it would be better
to omit it.
^ permalink raw reply
* Re: [RFC][PATCH 0/8] Mount, FS, Block and Keyrings notifications [ver #2]
From: Casey Schaufler @ 2019-06-04 20:31 UTC (permalink / raw)
To: Andy Lutomirski, David Howells
Cc: Al Viro, raven, Linux FS Devel, Linux API, linux-block, keyrings,
LSM List, LKML, casey
In-Reply-To: <CALCETrWzDR=Ap8NQ5-YrVhXCEBgr+hwpjw9fBn0m2NkZzZ7XLQ@mail.gmail.com>
n 6/4/2019 10:43 AM, Andy Lutomirski wrote:
> On Tue, Jun 4, 2019 at 9:35 AM David Howells <dhowells@redhat.com> wrote:
>>
>> Hi Al,
>>
>> Here's a set of patches to add a general variable-length notification queue
>> concept and to add sources of events for:
> I asked before and didn't see a response, so I'll ask again. Why are
> you paying any attention at all to the creds that generate an event?
> It seems like the resulting security model will be vary hard to
> understand and probably buggy. Can't you define a sensible model in
> which only the listener creds matter?
We've spent the last 18 months reeling from the implications
of what can happen when one process has the ability to snoop
on another. Introducing yet another mechanism that is trivial
to exploit is a very bad idea.
I will try to explain the problem once again. If process A
sends a signal (writes information) to process B the kernel
checks that either process A has the same UID as process B
or that process A has privilege to override that policy.
Process B is passive in this access control decision, while
process A is active. In the event delivery case, process A
does something (e.g. modifies a keyring) that generates an
event, which is then sent to process B's event buffer. Again,
A is active and B is passive. Process A must have write access
(defined by some policy) to process B's event buffer. To
implement such a policy requires A's credential, and some
information about the object (passive entity) to which the
event is being delivered. You can't just use the credential
from Process B because it is not the active entity, it is the
passive entity.
>
>> LSM support is included:
>>
>> (1) The creds of the process that did the fput() that reduced the refcount
>> to zero are cached in the file struct.
>>
>> (2) __fput() overrides the current creds with the creds from (1) whilst
>> doing the cleanup, thereby making sure that the creds seen by the
>> destruction notification generated by mntput() appears to come from
>> the last fputter.
> That looks like duct tape that is, at best, likely to be very buggy.
>
>> (3) security_post_notification() is called for each queue that we might
>> want to post a notification into, thereby allowing the LSM to prevent
>> covert communications.
> This seems like the wrong approach. If an LSM wants to prevent covert
> communication from, say, mount actions, then it shouldn't allow the
> watch to be set up in the first place.
^ permalink raw reply
* Re: [RFC PATCH 7/9] x86/sgx: Enforce noexec filesystem restriction for enclaves
From: Andy Lutomirski @ 2019-06-04 20:25 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Sean Christopherson, Andy Lutomirski, Cedric Xing,
Stephen Smalley, James Morris, Serge E . Hallyn, LSM List,
Paul Moore, Eric Paris, selinux, Jethro Beekman, Dave Hansen,
Thomas Gleixner, Linus Torvalds, LKML, X86 ML, linux-sgx,
Andrew Morton, nhorman, npmccallum, Serge Ayoun, Shay Katz-zamir,
Haitao Huang, Andy Shevchenko, Kai Svahn, Borislav Petkov,
Josh Triplett, Kai Huang, David Rientjes, William Roberts,
Philip Tricca
In-Reply-To: <20190604162555.GC3811@linux.intel.com>
On Tue, Jun 4, 2019 at 9:26 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Fri, May 31, 2019 at 04:31:57PM -0700, Sean Christopherson wrote:
> > Do not allow an enclave page to be mapped with PROT_EXEC if the source
> > page is backed by a file on a noexec file system.
> >
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
>
> Why don't you just check in sgx_encl_add_page() that whether the path
> comes from noexec and deny if SECINFO contains X?
>
SECINFO seems almost entirely useless for this kind of thing because
of SGX2. I'm thinking that SECINFO should be completely ignored for
anything other than its required architectural purpose.
^ permalink raw reply
* Re: [RFC PATCH 7/9] x86/sgx: Enforce noexec filesystem restriction for enclaves
From: Andy Lutomirski @ 2019-06-04 20:26 UTC (permalink / raw)
To: Xing, Cedric
Cc: Christopherson, Sean J, Jarkko Sakkinen, Andy Lutomirski,
Stephen Smalley, James Morris, Serge E . Hallyn, LSM List,
Paul Moore, Eric Paris, selinux@vger.kernel.org, Jethro Beekman,
Hansen, Dave, Thomas Gleixner, Linus Torvalds, LKML, X86 ML,
linux-sgx@vger.kernel.org, Andrew Morton, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, Andy Shevchenko, Svahn, Kai, Borislav Petkov,
Josh Triplett, Huang, Kai, David Rientjes, Roberts, William C,
Tricca, Philip B
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F654ECC53@ORSMSX116.amr.corp.intel.com>
On Sun, Jun 2, 2019 at 11:29 PM Xing, Cedric <cedric.xing@intel.com> wrote:
>
> > From: Christopherson, Sean J
> > Sent: Friday, May 31, 2019 4:32 PM
> >
> > Do not allow an enclave page to be mapped with PROT_EXEC if the source page is backed by a
> > file on a noexec file system.
> >
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> > arch/x86/kernel/cpu/sgx/driver/ioctl.c | 26 ++++++++++++++++++++++++--
> > 1 file changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> > b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> > index c30acd3fbbdd..5f71be7cbb01 100644
> > --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> > +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> > @@ -576,6 +576,27 @@ static int __sgx_encl_add_page(struct sgx_encl *encl, unsigned long
> > addr,
> > return ret;
> > }
> >
> > +static int sgx_encl_page_protect(unsigned long src, unsigned long prot,
> > + unsigned long *allowed_prot)
> > +{
> > + struct vm_area_struct *vma;
> > +
> > + if (!(*allowed_prot & VM_EXEC))
> > + goto do_check;
> > +
> > + down_read(¤t->mm->mmap_sem);
> > + vma = find_vma(current->mm, src);
> > + if (!vma || (vma->vm_file && path_noexec(&vma->vm_file->f_path)))
> > + *allowed_prot &= ~VM_EXEC;
>
> Testing (vma->vm_flags & VM_MAYEXEC) == 0 should be a better approach.
I think I agree, although that would need a comment explaining why it works.
^ permalink raw reply
* Re: [PATCH 00/58] LSM: Module stacking for AppArmor
From: Stephen Smalley @ 2019-06-04 20:34 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <a493956a-8a2f-6239-e5fe-09030640c397@schaufler-ca.com>
On 6/4/19 3:58 PM, Casey Schaufler wrote:
> On 6/4/2019 10:11 AM, Stephen Smalley wrote:
>> On 6/4/19 12:14 PM, Casey Schaufler wrote:
>>> On 6/4/2019 5:29 AM, Stephen Smalley wrote:
>>>> On 6/2/19 12:50 PM, Casey Schaufler wrote:
>>>>> This patchset provides the changes required for
>>>>> the AppArmor security module to stack safely with any other.
>>>>
>>>> Please explain the motivation
>>>
>>> I'll add some explanation for the next revision.
>>> It won't be anything that I haven't posted many times
>>> before, but you're right that it belongs in the log.
>>>
>>>> - why do we want to allow AppArmor to stack with other modules,
>>>
>>> First, is there a reason not to? Sure, you can confuse
>>> administrators by implementing complex security policies,
>>> but there are lots of ways to do that already.
>>
>> There are costs to doing so, e.g.
>> - greater complexity in the security framework,
>
> Taking blob management out of the modules and
> into the framework makes simplifies the modules.
>
>> - possibly greater memory and runtime overheads,
>
> Possibly reduced memory and runtime overheads, as well.
>
>> - potential user confusion (which security module(s) caused a given failure?)
>
> That's not new. I've seen countless cases where users blame
> SELinux or Smack when the problem is with mode bits and/or
> capabilities. Not to mention that a good 50% of current Linux
> users don't understand any of the security mechanisms to
> begin with.
>
>> - potential distro maintainer burden
>
> Selection of security modules and how they are configured
> has always been a burden for distro developers and maintainers.
> Nothing new here.
> And, they knew the job was dangerous when they took it.
>
>
>> (similar to above, but performing triage when any given permission denial can have multiple causes beyond just DAC + one module, weird interactions among modules, etc)
>
> Yama has been widely accepted by distros, and civilization
> has yet to have officially been declared ended.
>
>> It isn't free so there should be a cost/benefit analysis.
>
> Some benchmarking is definitely in order, but most
> of what's you're calling out as downside is hypothetical
> or based on assumption.
>
>>
>>>
>>> AppArmor provides a different security model than SELinux,
>>> TOMOYO or Smack. Smack is better at system component
>>> separation, while AppArmor is better at application isolation.
>>> It's a win to use each to its strength rather than trying to
>>> stretch either to the edge of what it can do.
>>>
>>>> who would use it,
>>>
>>> Can't name names, but there have been multiple requests.
>>>
>>>> how would it be used,
>>>
>>> As mentioned above, Smack for system separation, AppArmor for
>>> application isolation.
>>
>> Can you provide a concrete example of how combining the two yields a smaller, simpler configuration overall than using them individually?
>
> Smack + AppArmor is a simpler, smaller model than the Smack policy
> used in Tizen 2.
Not very compelling given that not even Tizen is using that policy
anymore. But even taking that as an example, have you actually done the
work to craft a working configuration of Smack + AppArmor that meets the
same security goals as the Tizen 2 policy and confirmed that it is in
fact smaller and simpler? Won't it just move the size/complexity from
Smack rules to AppArmor profiles? And those might be larger...
>
>>
>>>
>>>> what does it provide that isn't already possible in the absence of it.
>>>
>>> It's not necessary that something be impossible to do any
>>> other way. The question should be whether this provides for
>>> a better way to achieve the goals, and this does that.
>>> If I tried the come up with something that's impossible I
>>> would expect the usual "you can do that with SELinux policy"
>>> argument. We know we can do things. We want to have the tools
>>> to do them better.
>>>
>>>> Also, Ubuntu fully upstreamed all of their changes to AppArmor, would this still suffice to enable stacking of AppArmor or do they rely on hooks that are not handled here?
>>>
>>> Some amount of merging will likely be required. But that's
>>> always going to be true with parallel development tracks.
>>> That's why we have git!
>>>
>>>> Please explain the cost of the change - what do we pay in terms of memory, runtime, or other overheads in order to support this change?
>>>
>>> Do you have particular benchmarks you want to see?
>>> When I've supplied numbers in the past they have not
>>> been remarked on.
>>
>> A combination of micro and macro benchmarks exercising multiple kernel subsystems would be good. Kernel build time isn't sufficient.
>
> Do you have preferences, or better yet, facilities
> for running them? I am, alas, running on finite resources
> and benchmark contributions, especially in areas where you
> have specific concerns, would be most welcome.
>
>>
>>>
>>>>
>>>>>
>>>>> A new process attribute identifies which security module
>>>>> information should be reported by SO_PEERSEC and the
>>>>> /proc/.../attr/current interface. This is provided by
>>>>> /proc/.../attr/display. Writing the name of the security
>>>>> module desired to this interface will set which LSM hooks
>>>>> will be called for this information. The first security
>>>>> module providing the hooks will be used by default.
>>>>
>>>> Doesn't this effectively undo making the hooks read-only after init, at least for the subset involved? What are the security implications thereof?
>>>
>>> Any mechanism, be it a separate set of hooks, a name used to
>>> do list look ups, or an sophisticated hash scheme will have that
>>> impact for the processes that use it. This scheme has the best
>>> performance profile of the mechanisms I experimented with and
>>> avoids all sorts of special cases.
>>>
>>>>
>>>>> The use of integer based security tokens (secids) is
>>>>> generally (but not completely) replaced by a structure
>>>>> lsm_export. The lsm_export structure can contain information
>>>>> for each of the security modules that export information
>>>>> outside the LSM layer.
>>>>>
>>>>> The LSM interfaces that provide "secctx" text strings
>>>>> have been changed to use a structure "lsm_context"
>>>>> instead of a pointer/length pair. In some cases the
>>>>> interfaces used a "char *" pointer and in others a
>>>>> "void *". This was necessary to ensure that the correct
>>>>> release mechanism for the text is used. It also makes
>>>>> many of the interfaces cleaner.
>>>>>
>>>>> https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v1-apparmor
>>>>>
>>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>>> ---
>>>>> drivers/android/binder.c | 25 ++-
>>>>> fs/kernfs/dir.c | 6 +-
>>>>> fs/kernfs/inode.c | 31 ++-
>>>>> fs/kernfs/kernfs-internal.h | 3 +-
>>>>> fs/nfs/inode.c | 13 +-
>>>>> fs/nfs/internal.h | 8 +-
>>>>> fs/nfs/nfs4proc.c | 17 +-
>>>>> fs/nfs/nfs4xdr.c | 16 +-
>>>>> fs/nfsd/nfs4proc.c | 8 +-
>>>>> fs/nfsd/nfs4xdr.c | 14 +-
>>>>> fs/nfsd/vfs.c | 7 +-
>>>>> fs/proc/base.c | 1 +
>>>>> include/linux/cred.h | 3 +-
>>>>> include/linux/lsm_hooks.h | 91 +++++----
>>>>> include/linux/nfs4.h | 8 +-
>>>>> include/linux/security.h | 133 +++++++++----
>>>>> include/net/af_unix.h | 2 +-
>>>>> include/net/netlabel.h | 10 +-
>>>>> include/net/scm.h | 14 +-
>>>>> kernel/audit.c | 43 ++--
>>>>> kernel/audit.h | 9 +-
>>>>> kernel/auditfilter.c | 6 +-
>>>>> kernel/auditsc.c | 77 ++++----
>>>>> kernel/cred.c | 15 +-
>>>>> net/ipv4/cipso_ipv4.c | 13 +-
>>>>> net/ipv4/ip_sockglue.c | 12 +-
>>>>> net/netfilter/nf_conntrack_netlink.c | 29 ++-
>>>>> net/netfilter/nf_conntrack_standalone.c | 16 +-
>>>>> net/netfilter/nfnetlink_queue.c | 38 ++--
>>>>> net/netfilter/nft_meta.c | 13 +-
>>>>> net/netfilter/xt_SECMARK.c | 14 +-
>>>>> net/netlabel/netlabel_kapi.c | 5 +-
>>>>> net/netlabel/netlabel_unlabeled.c | 101 +++++-----
>>>>> net/netlabel/netlabel_unlabeled.h | 2 +-
>>>>> net/netlabel/netlabel_user.c | 13 +-
>>>>> net/netlabel/netlabel_user.h | 2 +-
>>>>> net/unix/af_unix.c | 6 +-
>>>>> security/apparmor/audit.c | 4 +-
>>>>> security/apparmor/include/audit.h | 2 +-
>>>>> security/apparmor/include/net.h | 6 +-
>>>>> security/apparmor/include/secid.h | 9 +-
>>>>> security/apparmor/lsm.c | 64 +++---
>>>>> security/apparmor/secid.c | 42 ++--
>>>>> security/integrity/ima/ima.h | 14 +-
>>>>> security/integrity/ima/ima_api.c | 9 +-
>>>>> security/integrity/ima/ima_appraise.c | 6 +-
>>>>> security/integrity/ima/ima_main.c | 34 ++--
>>>>> security/integrity/ima/ima_policy.c | 19 +-
>>>>> security/security.c | 338 +++++++++++++++++++++++++++-----
>>>>> security/selinux/hooks.c | 259 ++++++++++++------------
>>>>> security/selinux/include/audit.h | 5 +-
>>>>> security/selinux/include/objsec.h | 42 +++-
>>>>> security/selinux/netlabel.c | 25 +--
>>>>> security/selinux/ss/services.c | 18 +-
>>>>> security/smack/smack.h | 18 ++
>>>>> security/smack/smack_lsm.c | 238 +++++++++++-----------
>>>>> security/smack/smack_netfilter.c | 8 +-
>>>>> security/smack/smackfs.c | 12 +-
>>>>> 58 files changed, 1217 insertions(+), 779 deletions(-)
>>>>>
>>>>
>>
^ permalink raw reply
* Re: [RFC PATCH 7/9] x86/sgx: Enforce noexec filesystem restriction for enclaves
From: Sean Christopherson @ 2019-06-04 20:34 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Jarkko Sakkinen, Cedric Xing, Stephen Smalley, James Morris,
Serge E . Hallyn, LSM List, Paul Moore, Eric Paris, selinux,
Jethro Beekman, Dave Hansen, Thomas Gleixner, Linus Torvalds,
LKML, X86 ML, linux-sgx, Andrew Morton, nhorman, npmccallum,
Serge Ayoun, Shay Katz-zamir, Haitao Huang, Andy Shevchenko,
Kai Svahn, Borislav Petkov, Josh Triplett, Kai Huang,
David Rientjes, William Roberts, Philip Tricca
In-Reply-To: <CALCETrUqcQNbRvBe2UqQih8RHnuwn3KaC=xJU1cRsaEVsCQUgw@mail.gmail.com>
On Tue, Jun 04, 2019 at 01:25:10PM -0700, Andy Lutomirski wrote:
> On Tue, Jun 4, 2019 at 9:26 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Fri, May 31, 2019 at 04:31:57PM -0700, Sean Christopherson wrote:
> > > Do not allow an enclave page to be mapped with PROT_EXEC if the source
> > > page is backed by a file on a noexec file system.
> > >
> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> >
> > Why don't you just check in sgx_encl_add_page() that whether the path
> > comes from noexec and deny if SECINFO contains X?
> >
>
> SECINFO seems almost entirely useless for this kind of thing because
> of SGX2. I'm thinking that SECINFO should be completely ignored for
> anything other than its required architectural purpose.
Agreed.
I've already (somewhat unknowingly) reworked the SELinux patch such that
it ignores @prot (the SECINFO protections) and only looks at @allowed_prot
(the declared protections). If the kernel ignores SECINFO protections
entirely then the LSM hook can simply be:
int selinux_enclave_load(struct vm_area_struct *vma, unsigned long prot)
I.e. LSMs can be blissfully unaware that @prot isn't technically what's
going into the PTEs *now*.
^ permalink raw reply
* Re: [RFC PATCH 8/9] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX
From: Sean Christopherson @ 2019-06-04 20:36 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Jarkko Sakkinen, Cedric Xing, Stephen Smalley, James Morris,
Serge E . Hallyn, LSM List, Paul Moore, Eric Paris, selinux,
Jethro Beekman, Dave Hansen, Thomas Gleixner, Linus Torvalds,
LKML, X86 ML, linux-sgx, Andrew Morton, nhorman, npmccallum,
Serge Ayoun, Shay Katz-zamir, Haitao Huang, Andy Shevchenko,
Kai Svahn, Borislav Petkov, Josh Triplett, Kai Huang,
David Rientjes, William Roberts, Philip Tricca
In-Reply-To: <CALCETrXf3ujAn6uOwWMU8SRZOvBRb8ALvo_LQvwxc899mrakwQ@mail.gmail.com>
On Tue, Jun 04, 2019 at 01:29:10PM -0700, Andy Lutomirski wrote:
> On Fri, May 31, 2019 at 4:32 PM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> > static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr,
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 47f58cfb6a19..0562775424a0 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1446,6 +1446,14 @@
> > * @bpf_prog_free_security:
> > * Clean up the security information stored inside bpf prog.
> > *
> > + * Security hooks for Intel SGX enclaves.
> > + *
> > + * @enclave_load:
> > + * On success, returns 0 and optionally adjusts @allowed_prot
> > + * @vma: the source memory region of the enclave page being loaded.
> > + * @prot: the initial protection of the enclave page.
>
> What do you mean "initial"? The page is always mapped PROT_NONE when
> this is called, right? I feel like I must be missing something here.
Initial protection in the EPCM. Yet another reason to ignore SECINFO.
^ permalink raw reply
* Re: [RFC PATCH 8/9] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX
From: Andy Lutomirski @ 2019-06-04 20:29 UTC (permalink / raw)
To: Sean Christopherson
Cc: Jarkko Sakkinen, Andy Lutomirski, Cedric Xing, Stephen Smalley,
James Morris, Serge E . Hallyn, LSM List, Paul Moore, Eric Paris,
selinux, Jethro Beekman, Dave Hansen, Thomas Gleixner,
Linus Torvalds, LKML, X86 ML, linux-sgx, Andrew Morton, nhorman,
npmccallum, Serge Ayoun, Shay Katz-zamir, Haitao Huang,
Andy Shevchenko, Kai Svahn, Borislav Petkov, Josh Triplett,
Kai Huang, David Rientjes, William Roberts, Philip Tricca
In-Reply-To: <20190531233159.30992-9-sean.j.christopherson@intel.com>
On Fri, May 31, 2019 at 4:32 PM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> enclave_load() is roughly analogous to the existing file_mprotect().
>
> Due to the nature of SGX and its Enclave Page Cache (EPC), all enclave
> VMAs are backed by a single file, i.e. /dev/sgx/enclave, that must be
> MAP_SHARED. Furthermore, all enclaves need read, write and execute
> VMAs. As a result, file_mprotect() does not provide any meaningful
> security for enclaves since an LSM can only deny/grant access to the
> EPC as a whole.
>
> security_enclave_load() is called when SGX is first loading an enclave
> page, i.e. copying a page from normal memory into the EPC. The notable
> difference from file_mprotect() is the allowed_prot parameter, which
> is essentially an SGX-specific version of a VMA's MAY_{READ,WRITE,EXEC}
> flags. The purpose of allowed_prot is to enable checks such as
> SELinux's FILE__EXECMOD permission without having to track and update
> VMAs across multiple mm structs, i.e. SGX can ensure userspace doesn't
> overstep its bounds simply by restricting an enclave VMA's protections
> by vetting what is maximally allowed during build time.
>
> An alternative to the allowed_prot approach would be to use an enclave's
> SIGSTRUCT (a smallish structure that can uniquely identify an enclave)
> as a proxy for the enclave. For example, SGX could take and hold a
> reference to the file containing the SIGSTRUCT (if it's in a file) and
> call security_enclave_load() during mprotect(). While the SIGSTRUCT
> approach would provide better precision, the actual value added was
> deemed to be negligible. On the other hand, pinning a file for the
> lifetime of the enclave is ugly, and essentially caching LSM policies
> in each page's allowed_prot avoids having to make an extra LSM upcall
> during mprotect().
>
> Note, extensive discussion yielded no sane alternative to some form of
> SGX specific LSM hook[1].
>
> [1] https://lkml.kernel.org/r/CALCETrXf8mSK45h7sTK5Wf+pXLVn=Bjsc_RLpgO-h-qdzBRo5Q@mail.gmail.com
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
> arch/x86/kernel/cpu/sgx/driver/ioctl.c | 14 +++++++++-----
> include/linux/lsm_hooks.h | 16 ++++++++++++++++
> include/linux/security.h | 2 ++
> security/security.c | 8 ++++++++
> 4 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> index 5f71be7cbb01..260417ecbcff 100644
> --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
> @@ -8,6 +8,7 @@
> #include <linux/highmem.h>
> #include <linux/ratelimit.h>
> #include <linux/sched/signal.h>
> +#include <linux/security.h>
> #include <linux/shmem_fs.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
> @@ -580,21 +581,24 @@ static int sgx_encl_page_protect(unsigned long src, unsigned long prot,
> unsigned long *allowed_prot)
> {
> struct vm_area_struct *vma;
> + int ret = 0;
>
> - if (!(*allowed_prot & VM_EXEC))
> + if (!(*allowed_prot & VM_EXEC) && !IS_ENABLED(CONFIG_SECURITY))
> goto do_check;
>
> down_read(¤t->mm->mmap_sem);
> vma = find_vma(current->mm, src);
> if (!vma || (vma->vm_file && path_noexec(&vma->vm_file->f_path)))
> *allowed_prot &= ~VM_EXEC;
> +#ifdef CONFIG_SECURITY
> + ret = security_enclave_load(vma, prot, allowed_prot);
> +#endif
> up_read(¤t->mm->mmap_sem);
>
> do_check:
> - if (prot & ~*allowed_prot)
> - return -EACCES;
> -
> - return 0;
> + if (!ret && (prot & ~*allowed_prot))
> + ret = -EACCES;
> + return ret;
> }
>
> static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr,
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 47f58cfb6a19..0562775424a0 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1446,6 +1446,14 @@
> * @bpf_prog_free_security:
> * Clean up the security information stored inside bpf prog.
> *
> + * Security hooks for Intel SGX enclaves.
> + *
> + * @enclave_load:
> + * On success, returns 0 and optionally adjusts @allowed_prot
> + * @vma: the source memory region of the enclave page being loaded.
> + * @prot: the initial protection of the enclave page.
What do you mean "initial"? The page is always mapped PROT_NONE when
this is called, right? I feel like I must be missing something here.
^ permalink raw reply
* Re: [RFC][PATCH 0/8] Mount, FS, Block and Keyrings notifications [ver #2]
From: David Howells @ 2019-06-04 20:39 UTC (permalink / raw)
To: Andy Lutomirski
Cc: dhowells, Al Viro, Casey Schaufler, raven, Linux FS Devel,
Linux API, linux-block, keyrings, LSM List, LKML
In-Reply-To: <CALCETrWzDR=Ap8NQ5-YrVhXCEBgr+hwpjw9fBn0m2NkZzZ7XLQ@mail.gmail.com>
Andy Lutomirski <luto@kernel.org> wrote:
> > Here's a set of patches to add a general variable-length notification queue
> > concept and to add sources of events for:
>
> I asked before and didn't see a response, so I'll ask again. Why are you
> paying any attention at all to the creds that generate an event?
Casey responded to you. It's one of his requirements.
I'm not sure of the need, and I particularly don't like trying to make
indirect destruction events (mount destruction keyed on fput, for instance)
carry the creds of the triggerer. Indeed, the trigger can come from all sorts
of places - including af_unix queue destruction, someone poking around in
procfs, a variety of processes fputting simultaneously. Only one of them can
win, and the LSM needs to handle *all* the possibilities.
However, the LSMs (or at least SELinux) ignore f_cred and use current_cred()
when checking permissions. See selinux_revalidate_file_permission() for
example - it uses current_cred() not file->f_cred to re-evaluate the perms,
and the fd might be shared between a number of processes with different creds.
> This seems like the wrong approach. If an LSM wants to prevent covert
> communication from, say, mount actions, then it shouldn't allow the
> watch to be set up in the first place.
Yeah, I can agree to that. Casey?
David
^ permalink raw reply
* Re: [PATCH 00/58] LSM: Module stacking for AppArmor
From: James Morris @ 2019-06-04 20:42 UTC (permalink / raw)
To: Casey Schaufler
Cc: Stephen Smalley, casey.schaufler, linux-security-module, selinux,
keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <a493956a-8a2f-6239-e5fe-09030640c397@schaufler-ca.com>
On Tue, 4 Jun 2019, Casey Schaufler wrote:
> > It isn't free so there should be a cost/benefit analysis.
>
> Some benchmarking is definitely in order, but most
> of what's you're calling out as downside is hypothetical
> or based on assumption.
When you're proposing changes such as these, which make fundamental and
far-reaching changes, the burden is on you to present the cost/benefit
analysis.
You can't just say "Here are some changes and here are the benefits, and
any possible costs are merely hypothetical".
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC][PATCH 0/8] Mount, FS, Block and Keyrings notifications [ver #2]
From: Andy Lutomirski @ 2019-06-04 20:57 UTC (permalink / raw)
To: David Howells
Cc: Andy Lutomirski, Al Viro, Casey Schaufler, raven, Linux FS Devel,
Linux API, linux-block, keyrings, LSM List, LKML
In-Reply-To: <1207.1559680778@warthog.procyon.org.uk>
On Tue, Jun 4, 2019 at 1:39 PM David Howells <dhowells@redhat.com> wrote:
>
> Andy Lutomirski <luto@kernel.org> wrote:
>
> > > Here's a set of patches to add a general variable-length notification queue
> > > concept and to add sources of events for:
> >
> > I asked before and didn't see a response, so I'll ask again. Why are you
> > paying any attention at all to the creds that generate an event?
>
> Casey responded to you. It's one of his requirements.
>
It being a "requirement" doesn't make it okay.
> However, the LSMs (or at least SELinux) ignore f_cred and use current_cred()
> when checking permissions. See selinux_revalidate_file_permission() for
> example - it uses current_cred() not file->f_cred to re-evaluate the perms,
> and the fd might be shared between a number of processes with different creds.
That's a bug. It's arguably a rather severe bug. If I ever get
around to writing the patch I keep thinking of that will warn if we
use creds from invalid contexts, it will warn.
Let's please not repeat this.
^ permalink raw reply
* Re: [RFC][PATCH 0/8] Mount, FS, Block and Keyrings notifications [ver #2]
From: Andy Lutomirski @ 2019-06-04 21:05 UTC (permalink / raw)
To: Casey Schaufler
Cc: Andy Lutomirski, David Howells, Al Viro, raven, Linux FS Devel,
Linux API, linux-block, keyrings, LSM List, LKML
In-Reply-To: <50c2ea19-6ae8-1f42-97ef-ba5c95e40475@schaufler-ca.com>
On Tue, Jun 4, 2019 at 1:31 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> n 6/4/2019 10:43 AM, Andy Lutomirski wrote:
> > On Tue, Jun 4, 2019 at 9:35 AM David Howells <dhowells@redhat.com> wrote:
> >>
> >> Hi Al,
> >>
> >> Here's a set of patches to add a general variable-length notification queue
> >> concept and to add sources of events for:
> > I asked before and didn't see a response, so I'll ask again. Why are
> > you paying any attention at all to the creds that generate an event?
> > It seems like the resulting security model will be vary hard to
> > understand and probably buggy. Can't you define a sensible model in
> > which only the listener creds matter?
>
> We've spent the last 18 months reeling from the implications
> of what can happen when one process has the ability to snoop
> on another. Introducing yet another mechanism that is trivial
> to exploit is a very bad idea.
If you're talking about Spectre, etc, this is IMO entirely irrelevant.
Among other things, setting these watches can and should require some
degree of privilege.
>
> I will try to explain the problem once again. If process A
> sends a signal (writes information) to process B the kernel
> checks that either process A has the same UID as process B
> or that process A has privilege to override that policy.
> Process B is passive in this access control decision, while
> process A is active.
Are you stating what you see to be a requirement?
> Process A must have write access
> (defined by some policy) to process B's event buffer.
No, stop right here. Process B is monitoring some aspect of the
system. Process A is doing something. Process B should need
permission to monitor whatever it's monitoring, and process A should
have permission to do whatever it's doing. I don't think it makes
sense to try to ascribe an identity to the actor doing some action to
decide to omit it from the watch -- this has all kinds of correctness
issues.
If you're writing a policy and you don't like letting process B spy on
processes doing various things, then disallow that type of spying.
> To
> implement such a policy requires A's credential,
You may not design a new mechanism that looks at the credential in a
context where looking at a credential is invalid unless you have some
very strong justification for why all of the known reasons that it's a
bad idea don't apply to what you're doing.
So, without a much stronger justification, NAK.
^ permalink raw reply
* Re: [RFC][PATCH 0/8] Mount, FS, Block and Keyrings notifications [ver #2]
From: Casey Schaufler @ 2019-06-04 21:11 UTC (permalink / raw)
To: David Howells, Andy Lutomirski
Cc: Al Viro, raven, Linux FS Devel, Linux API, linux-block, keyrings,
LSM List, LKML, casey
In-Reply-To: <1207.1559680778@warthog.procyon.org.uk>
On 6/4/2019 1:39 PM, David Howells wrote:
> Andy Lutomirski <luto@kernel.org> wrote:
>
>>> Here's a set of patches to add a general variable-length notification queue
>>> concept and to add sources of events for:
>> I asked before and didn't see a response, so I'll ask again. Why are you
>> paying any attention at all to the creds that generate an event?
> Casey responded to you. It's one of his requirements.
Process A takes an action. As a result of that action,
an event is written to Process B's event buffer. This isn't
a covert channel, it's a direct access, just like sending
a signal. Process A is the subject and the event buffer,
which is part of Process B, is the object.
> I'm not sure of the need, and I particularly don't like trying to make
> indirect destruction events (mount destruction keyed on fput, for instance)
> carry the creds of the triggerer. Indeed, the trigger can come from all sorts
> of places - including af_unix queue destruction, someone poking around in
> procfs, a variety of processes fputting simultaneously. Only one of them can
> win, and the LSM needs to handle *all* the possibilities.
Yes, it's a hairy problem. It was a significant factor in the
demise of kdbus.
> However, the LSMs (or at least SELinux) ignore f_cred and use current_cred()
> when checking permissions. See selinux_revalidate_file_permission() for
> example - it uses current_cred() not file->f_cred to re-evaluate the perms,
> and the fd might be shared between a number of processes with different creds.
>
>> This seems like the wrong approach. If an LSM wants to prevent covert
>> communication from, say, mount actions, then it shouldn't allow the
>> watch to be set up in the first place.
> Yeah, I can agree to that. Casey?
Back to your earlier point, you don't know where the
event is coming from when you create the event watch.
If you enforce a watch time, what are you going to check?
Isn't this going to be considered too restrictive?
^ permalink raw reply
* Re: [PATCH 00/58] LSM: Module stacking for AppArmor
From: Casey Schaufler @ 2019-06-04 21:19 UTC (permalink / raw)
To: James Morris
Cc: Stephen Smalley, casey.schaufler, linux-security-module, selinux,
keescook, john.johansen, penguin-kernel, paul, casey
In-Reply-To: <alpine.LRH.2.21.1906050638550.31292@namei.org>
On 6/4/2019 1:42 PM, James Morris wrote:
> On Tue, 4 Jun 2019, Casey Schaufler wrote:
>
>>> It isn't free so there should be a cost/benefit analysis.
>> Some benchmarking is definitely in order, but most
>> of what's you're calling out as downside is hypothetical
>> or based on assumption.
> When you're proposing changes such as these, which make fundamental and
> far-reaching changes, the burden is on you to present the cost/benefit
> analysis.
Granted. There has been substantial conversation about it
over the years, but I have not done well including it in
this discussion.
> You can't just say "Here are some changes and here are the benefits, and
> any possible costs are merely hypothetical".
Of course. Nonetheless, no evidence for performance impact has
been provided, while it has been asserted.
^ permalink raw reply
* RE: [RFC PATCH 0/9] security: x86/sgx: SGX vs. LSM
From: Xing, Cedric @ 2019-06-04 21:38 UTC (permalink / raw)
To: Stephen Smalley, Christopherson, Sean J
Cc: Jarkko Sakkinen, Andy Lutomirski, James Morris, Serge E . Hallyn,
LSM List, Paul Moore, Eric Paris, selinux@vger.kernel.org,
Jethro Beekman, Hansen, Dave, Thomas Gleixner, Linus Torvalds,
LKML, X86 ML, linux-sgx@vger.kernel.org, Andrew Morton,
nhorman@redhat.com, npmccallum@redhat.com, Ayoun, Serge,
Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Svahn, Kai,
Borislav Petkov, Josh Triplett, Huang, Kai, David Rientjes,
Roberts, William C, Tricca, Philip B
In-Reply-To: <10a49f97-b3be-ed09-2821-68157f01aebe@tycho.nsa.gov>
Hi Stephen,
> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Stephen Smalley
> Sent: Tuesday, June 04, 2019 8:34 AM
>
> On 6/3/19 2:30 PM, Xing, Cedric wrote:
> >> From: Christopherson, Sean J
> >> Sent: Monday, June 03, 2019 10:16 AM
> >>
> >> On Sun, Jun 02, 2019 at 12:29:35AM -0700, Xing, Cedric wrote:
> >>> Hi Sean,
> >>>
> >>> Generally I agree with your direction but think ALLOW_* flags are
> >>> completely internal to LSM because they can be both produced and
> >>> consumed inside an LSM module. So spilling them into SGX driver and
> >>> also user mode code makes the solution ugly and in some cases
> >>> impractical because not every enclave host process has a priori
> >>> knowledge on whether or not an enclave page would be EMODPE'd at
> >> runtime.
> >>
> >> In this case, the host process should tag *all* pages it *might*
> convert
> >> to executable as ALLOW_EXEC. LSMs can (and should/will) be written
> in
> >> such a way that denying ALLOW_EXEC is fatal to the enclave if and
> only
> >> if the enclave actually attempts mprotect(PROT_EXEC).
> >
> > What if those pages contain self-modifying code but the host doesn't
> know ahead of time? Would it require ALLOW_WRITE|ALLOW_EXEC at EADD?
> Then would it prevent those pages to start with PROT_EXEC?
> >
> > Anyway, my point is that it is unnecessary even if it works.
> >
> >>
> >> Take the SELinux path for example. The only scenario in which
> >> PROT_WRITE is cleared from @allowed_prot is if the page *starts* with
> >> PROT_EXEC.
> >> If PROT_EXEC is denied on a page that starts RW, e.g. an EAUG'd page,
> >> then PROT_EXEC will be cleared from @allowed_prot.
> >>
> >> As Stephen pointed out, auditing the denials on @allowed_prot means
> the
> >> log will contain false positives of a sort. But this is more of a
> noise
> >> issue than true false positives. E.g. there are three possible
> outcomes
> >> for the enclave.
> >>
> >> - The enclave does not do EMODPE[PROT_EXEC] in any scenario, ever.
> >> Requesting ALLOW_EXEC is either a straightforward a userspace
> bug or
> >> a poorly written generic enclave loader.
> >>
> >> - The enclave conditionally performs EMODPE[PROT_EXEC]. In this
> case
> >> the denial is a true false positive.
> >>
> >> - The enclave does EMODPE[PROT_EXEC] and its host userspace then
> fails
> >> on mprotect(PROT_EXEC), i.e. the LSM denial is working as
> intended.
> >> The audit log will be noisy, but viewed as a whole the denials
> >> aren't
> >> false positives.
> >
> > What I was talking about was EMODPE[PROT_WRITE] on an RX page.
> >
> >>
> >> The potential for noisy audit logs and/or false positives is
> unfortunate,
> >> but it's (by far) the lesser of many evils.
> >>
> >>> Theoretically speaking, what you really need is a per page flag
> (let's
> >>> name it WRITTEN?) indicating whether a page has ever been written to
> >>> (or more precisely, granted PROT_WRITE), which will be used to
> decide
> >>> whether to grant PROT_EXEC when requested in future. Given the fact
> >>> that all mprotect() goes through LSM and mmap() is limited to
> >>> PROT_NONE, it's easy for LSM to capture that flag by itself instead
> of
> >> asking user mode code to provide it.
> >>>
> >>> That said, here is the summary of what I think is a better approach.
> >>> * In hook security_file_alloc(), if @file is an enclave, allocate
> some
> >> data
> >>> structure to store for every page, the WRITTEN flag as described
> >> above.
> >>> WRITTEN is cleared initially for all pages.
> >>
> >> This would effectively require *every* LSM to duplicate the SGX
> driver's
> >> functionality, e.g. track per-page metadata, implement locking to
> >> prevent races between multiple mm structs, etc...
> >
> > Architecturally we shouldn't dictate how LSM makes decisions. ALLOW_*
> are no difference than PROCESS__* or FILE__* flags, which are just
> artifacts to assist particular LSMs in decision making. They are never
> considered part of the LSM interface, even if other LSMs than SELinux
> may adopt the same/similar approach.
> >
> > If code duplication is what you are worrying about, you can put them
> in a library, or implement/export them in some new file (maybe
> security/enclave.c?) as utility functions. But spilling them into user
> mode is what I think is unacceptable.
> >
> >>
> >>> Open: Given a file of type struct file *, how to tell if it is an
> >> enclave (i.e. /dev/sgx/enclave)?
> >>> * In hook security_mmap_file(), if @file is an enclave, make sure
> >> @prot can
> >>> only be PROT_NONE. This is to force all protection changes to go
> >> through
> >>> security_file_mprotect().
> >>> * In the newly introduced hook security_enclave_load(), set WRITTEN
> >> for pages
> >>> that are requested PROT_WRITE.
> >>
> >> How would an LSM associate a page with a specific enclave? vma-
> >vm_file
> >> will point always point at /dev/sgx/enclave. vma->vm_mm is useless
> >> because we're allowing multiple processes to map a single enclave,
> not
> >> to mention that by mm would require holding a reference to the mm.
> >
> > Each open("/dev/sgx/enclave") syscall creates a *new* instance of
> struct file to uniquely identify one enclave instance. What I mean is
> @vma->vm_file, not @vma->vm_file->f_path or @vma->vm_file->f_inode.
> >
> >>
> >>> * In hook security_file_mprotect(), if @vma->vm_file is an enclave,
> >> look up
> >>> and use WRITTEN flags for all pages within @vma, along with other
> >> global
> >>> flags (e.g. PROCESS__EXECMEM/FILE__EXECMOD in the case of SELinux)
> >> to decide
> >>> on allowing/rejecting @prot.
> >>
> >> vma->vm_file will always be /dev/sgx/enclave at this point, which
> means
> >> LSMs don't have the necessary anchor back to the source file, e.g. to
> >> enforce FILE__EXECUTE. The noexec file system case is also
> unaddressed.
> >
> > vma->vm_file identifies an enclave instance uniquely. FILE__EXECUTE is
> checked by security_enclave_load() using @source_vma->vm_file. Once a
> page has been EADD'ed, whether to allow RW->RX depends on .sigstruct
> file (more precisely, the file backing SIGSTRUCT), whose FILE__*
> attributes could be cached in vma->vm_file->f_security by
> security_enclave_init().
>
> The RFC series seemed to dispense with the use of the sigstruct file and
> just used the source file throughout IIUC. That allowed for reuse of
> FILE__* permissions without ambiguity rather than introducing separate
> ENCLAVE__* permissions or using /dev/sgx/enclave inode as the target of
> all checks.
That's right. But that's not the distinction between Sean's patch and my proposal.
My point here is, from the perspective of LSM architecture, LSM hooks shall be defined to pass adequate information to allow *all* possible implementations to make reasonable decisions. In particular, all parameters to EADD (i.e. target linear address, SECINFO, source page) could affect (current and future) decisions but Sean's definition of security_enclave_load() passes only the source, which limits the possibility of other implementations. Another point I'm trying to make is, different LSM implementations may need different information at any given decision point, therefore it's *not* possible to always pass "right" information at "right" time. And that's why I think .f_security was added to struct file to allow stateful LSMs. Sean's patch however is trying the opposite, as ALLOW_* flags should otherwise be part of internal state of LSMs, but they are spilled into SGX driver and also userspace.
>
> Regardless, IIUC, your approach requires that we always check
> FILE__EXECMOD, and FILE__EXECUTE up front during security_enclave_load()
> irrespective of prot so that we can save the result in the f_security
> for later use by the mprotect hook. This may generate many spurious
> audit messages for cases where PROT_EXEC will never be requested, and
> users will be prone to just always allowing it since they cannot tell
> when it was actually needed.
Yes and no.
For those not following this discussion closely, here's the prototype of security_enclave_load() that I proposed in one of my earlier emails.
int security_enclave_load(struct file *enclave_fd, unsigned long linear_address, unsigned long nr_pages, int prot, struct vm_area_struct *source_vma);
@enclave_fd identifies the enclave to which new pages are being added.
@linear_address/@nr_pages specifies the linear range of pages being added.
@prot specifies the initial protection of those newly added pages. It is taken from the vma covering the target range.
@source_vma covers the source pages in the case of EADD. An LSM is expected to make sure security_file_mprotect(source_vma, prot, prot) would succeed before checking anything else, unless @source_vma is NULL, indicating pages are being EAUG'ed. In all cases, LSM is expected to "remember" @prot for all those pages to be checked in future security_file_mprotect() invocations.
Architecture wise, the idea here is for SGX driver to pass in all information relevant for an LSM's decision.
Implementation wise, LSM may allow PROT_EXEC depending on FILE__EXECUTE of the source file (@source_vma->vm_file), or the sigstruct file (will be provided to LSM at security_enclave_init), or /dev/sgx/enclave. It makes most sense to me to use the source file, hence the check would most likely be done here. For future security_file_mprotect(), the source file's FILE__EXECMOD could also be cached here, or it could use sigstruct file's FILE__EXECMOD instead. Given the fact that no EPC pages could be accessed before EINIT, the major purpose of security_enclave_load() is for LSM to cache whatever information deemed appropriate for the pages being EADD'ed (i.e. @source_vma != NULL) so that it has necessary information to make decisions in security_file_mprotect() in future. And in that regard the parameter @prot is unnecessary but I decided to have it here for 2 reasons: 1) Pages may be EADD'ed within an existing VMA (so no upcoming mprotect) so LSM's decision is needed right away and 2) @source_vma may not be able to mprotect(@prot) and in that case it'd be better to fail EADD instead of failing at mprotect() later.
>
> >
> > The noexec case should be addressed in IOC_ADD_PAGES by testing
> @source_vma->vm_flags & VM_MAYEXEC.
> >
> >>
> >>> * In hook security_file_free(), if @file is an enclave, free
> storage
> >>> allocated for WRITTEN flags.
^ permalink raw reply
* RE: [RFC PATCH 8/9] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX
From: Xing, Cedric @ 2019-06-04 21:43 UTC (permalink / raw)
To: Christopherson, Sean J, Andy Lutomirski
Cc: Jarkko Sakkinen, Stephen Smalley, James Morris, Serge E . Hallyn,
LSM List, Paul Moore, Eric Paris, selinux@vger.kernel.org,
Jethro Beekman, Hansen, Dave, Thomas Gleixner, Linus Torvalds,
LKML, X86 ML, linux-sgx@vger.kernel.org, Andrew Morton,
nhorman@redhat.com, npmccallum@redhat.com, Ayoun, Serge,
Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Svahn, Kai,
Borislav Petkov, Josh Triplett, Huang, Kai, David Rientjes,
Roberts, William C, Tricca, Philip B
In-Reply-To: <20190604203649.GC7775@linux.intel.com>
> From: Christopherson, Sean J
> Sent: Tuesday, June 04, 2019 1:37 PM
>
> On Tue, Jun 04, 2019 at 01:29:10PM -0700, Andy Lutomirski wrote:
> > On Fri, May 31, 2019 at 4:32 PM Sean Christopherson
> > <sean.j.christopherson@intel.com> wrote:
> > > static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long
> > > addr, diff --git a/include/linux/lsm_hooks.h
> > > b/include/linux/lsm_hooks.h index 47f58cfb6a19..0562775424a0 100644
> > > --- a/include/linux/lsm_hooks.h
> > > +++ b/include/linux/lsm_hooks.h
> > > @@ -1446,6 +1446,14 @@
> > > * @bpf_prog_free_security:
> > > * Clean up the security information stored inside bpf prog.
> > > *
> > > + * Security hooks for Intel SGX enclaves.
> > > + *
> > > + * @enclave_load:
> > > + * On success, returns 0 and optionally adjusts @allowed_prot
> > > + * @vma: the source memory region of the enclave page being
> loaded.
> > > + * @prot: the initial protection of the enclave page.
> >
> > What do you mean "initial"? The page is always mapped PROT_NONE when
> > this is called, right? I feel like I must be missing something here.
>
> Initial protection in the EPCM. Yet another reason to ignore SECINFO.
I know you guys are talking in the background that all pages are mmap()'ed PROT_NONE. But that's an unnecessary limitation. And @prot here should be @target_vma->vm_flags&(VM_READ|VM_WRITE|VM_EXEC).
^ permalink raw reply
* RE: [RFC PATCH 7/9] x86/sgx: Enforce noexec filesystem restriction for enclaves
From: Xing, Cedric @ 2019-06-04 21:54 UTC (permalink / raw)
To: Andy Lutomirski, Jarkko Sakkinen
Cc: Christopherson, Sean J, Stephen Smalley, James Morris,
Serge E . Hallyn, LSM List, Paul Moore, Eric Paris,
selinux@vger.kernel.org, Jethro Beekman, Hansen, Dave,
Thomas Gleixner, Linus Torvalds, LKML, X86 ML,
linux-sgx@vger.kernel.org, Andrew Morton, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, Andy Shevchenko, Svahn, Kai, Borislav Petkov,
Josh Triplett, Huang, Kai, David Rientjes, Roberts, William C,
Tricca, Philip B
In-Reply-To: <CALCETrUqcQNbRvBe2UqQih8RHnuwn3KaC=xJU1cRsaEVsCQUgw@mail.gmail.com>
> From: Andy Lutomirski [mailto:luto@kernel.org]
> Sent: Tuesday, June 04, 2019 1:25 PM
> To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
> On Tue, Jun 4, 2019 at 9:26 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Fri, May 31, 2019 at 04:31:57PM -0700, Sean Christopherson wrote:
> > > Do not allow an enclave page to be mapped with PROT_EXEC if the
> > > source page is backed by a file on a noexec file system.
> > >
> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> >
> > Why don't you just check in sgx_encl_add_page() that whether the path
> > comes from noexec and deny if SECINFO contains X?
> >
>
> SECINFO seems almost entirely useless for this kind of thing because of
> SGX2. I'm thinking that SECINFO should be completely ignored for
> anything other than its required architectural purpose.
For the purpose of allowing/denying EADD/EAUG, SECINFO is useless.
But SECINFO contains also the page type. What's coming as new feature of SGX2 is CONFIGID, which is a 512-bit value inside SECS, provided by untrusted code at ECREATE. Usually CONFIGID is a hash of something that would affect the behavior of the enclave. For example, the "main" enclave could be a JVM with the actual applet being loaded hashed into SECS.CONFIGID. In that case the enclave's measurements (MRENCLAVE) will stay the same for all applets yet individual applet will have distinct CONFIGID and receive distinct keys. When it comes to LSM, a policy may want to whitelist/blacklist applets for a JVM so a hook at ECREATE may be desirable. We could either define a new hook, or overload security_enclave_load() by providing SECINFO as one of its parameters.
^ permalink raw reply
* RE: [RFC PATCH 3/9] x86/sgx: Allow userspace to add multiple pages in single ioctl()
From: Xing, Cedric @ 2019-06-04 22:02 UTC (permalink / raw)
To: Andy Lutomirski, Christopherson, Sean J
Cc: Jarkko Sakkinen, Stephen Smalley, James Morris, Serge E . Hallyn,
LSM List, Paul Moore, Eric Paris, selinux@vger.kernel.org,
Jethro Beekman, Hansen, Dave, Thomas Gleixner, Linus Torvalds,
LKML, X86 ML, linux-sgx@vger.kernel.org, Andrew Morton,
nhorman@redhat.com, npmccallum@redhat.com, Ayoun, Serge,
Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Svahn, Kai,
Borislav Petkov, Josh Triplett, Huang, Kai, David Rientjes,
Roberts, William C, Tricca, Philip B
In-Reply-To: <CALCETrUb4X9_L9RXKhmyNpfSCsbNodP=BfbfO8Fz_efq24jp8w@mail.gmail.com>
> From: Andy Lutomirski [mailto:luto@kernel.org]
> Sent: Tuesday, June 04, 2019 1:18 PM
>
> On Mon, Jun 3, 2019 at 1:39 PM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > On Mon, Jun 03, 2019 at 01:08:04PM -0700, Sean Christopherson wrote:
> > > On Sun, Jun 02, 2019 at 11:26:09PM -0700, Xing, Cedric wrote:
> > > > > From: Christopherson, Sean J
> > > > > Sent: Friday, May 31, 2019 4:32 PM
> > > > >
> > > > > +/**
> > > > > + * sgx_ioc_enclave_add_pages - handler for
> > > > > +%SGX_IOC_ENCLAVE_ADD_PAGES
> > > > > + *
> > > > > + * @filep: open file to /dev/sgx
> > > > > + * @cmd: the command value
> > > > > + * @arg: pointer to an &sgx_enclave_add_page instance
> > > > > + *
> > > > > + * Add a range of pages to an uninitialized enclave (EADD), and
> > > > > +optionally
> > > > > + * extend the enclave's measurement with the contents of the
> page (EEXTEND).
> > > > > + * The range of pages must be virtually contiguous. The
> > > > > +SECINFO and
> > > > > + * measurement maskare applied to all pages, i.e. pages with
> > > > > +different
> > > > > + * properties must be added in separate calls.
> > > > > + *
> > > > > + * EADD and EEXTEND are done asynchronously via worker threads.
> > > > > +A successful
> > > > > + * sgx_ioc_enclave_add_page() only indicates the pages have
> > > > > +been added to the
> > > > > + * work queue, it does not guarantee adding the pages to the
> > > > > +enclave will
> > > > > + * succeed.
> > > > > + *
> > > > > + * Return:
> > > > > + * 0 on success,
> > > > > + * -errno otherwise
> > > > > + */
> > > > > +static long sgx_ioc_enclave_add_pages(struct file *filep,
> unsigned int cmd,
> > > > > + unsigned long arg) { struct
> > > > > +sgx_enclave_add_pages *addp = (void *)arg; struct sgx_encl
> > > > > +*encl = filep->private_data; struct sgx_secinfo secinfo;
> > > > > +unsigned int i; int ret;
> > > > > +
> > > > > + if (copy_from_user(&secinfo, (void __user *)addp->secinfo,
> > > > > + sizeof(secinfo)))
> > > > > + return -EFAULT;
> > > > > +
> > > > > + for (i = 0, ret = 0; i < addp->nr_pages && !ret; i++) {
> > > > > + if (signal_pending(current))
> > > > > + return -ERESTARTSYS;
> > > >
> > > > If interrupted, how would user mode code know how many pages have
> been EADD'ed?
> > >
> > > Hmm, updating nr_pages would be fairly simple and shouldn't confuse
> > > userspace, e.g. as opposed to overloading the return value.
> >
> > Or maybe update @addr and @src as well? That would allow userspace to
> > re-invoke the ioctl() without having to modify the struct.
>
> If you're going to use -ERESTARTSYS, that's the way to go. -EINTR would
> be an alternative. A benefit of -ERESTARTSYS is that, with -EINTR, it
> wouldn't be that surprising for user code to simply fail to handle it.
-EINTR means the call was interrupted before anything could be done. Am I correct?
But in this case some pages have been processed already so I guess we cannot return any error code. I think it more reasonable to return the number of pages (or bytes) processed.
^ permalink raw reply
* Re: [RFC][PATCH 0/8] Mount, FS, Block and Keyrings notifications [ver #2]
From: Casey Schaufler @ 2019-06-04 22:03 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David Howells, Al Viro, raven, Linux FS Devel, Linux API,
linux-block, keyrings, LSM List, LKML, casey
In-Reply-To: <CALCETrWFBA8H0RiZPikLtEi8xg-cqJLtQgnU2CGTuwByrHN7Dw@mail.gmail.com>
On 6/4/2019 2:05 PM, Andy Lutomirski wrote:
> On Tue, Jun 4, 2019 at 1:31 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> n 6/4/2019 10:43 AM, Andy Lutomirski wrote:
>>> On Tue, Jun 4, 2019 at 9:35 AM David Howells <dhowells@redhat.com> wrote:
>>>> Hi Al,
>>>>
>>>> Here's a set of patches to add a general variable-length notification queue
>>>> concept and to add sources of events for:
>>> I asked before and didn't see a response, so I'll ask again. Why are
>>> you paying any attention at all to the creds that generate an event?
>>> It seems like the resulting security model will be vary hard to
>>> understand and probably buggy. Can't you define a sensible model in
>>> which only the listener creds matter?
>> We've spent the last 18 months reeling from the implications
>> of what can happen when one process has the ability to snoop
>> on another. Introducing yet another mechanism that is trivial
>> to exploit is a very bad idea.
> If you're talking about Spectre, etc, this is IMO entirely irrelevant.
We're seeing significant interest in using obscure mechanisms
in system exploits. Mechanisms will be exploited.
> Among other things, setting these watches can and should require some
> degree of privilege.
Requiring privilege would address the concerns for most
situations, although I don't see that it would help for
SELinux. SELinux does not generally put much credence in
what others consider "privilege".
Extreme care would probably be required for namespaces, too.
>
>> I will try to explain the problem once again. If process A
>> sends a signal (writes information) to process B the kernel
>> checks that either process A has the same UID as process B
>> or that process A has privilege to override that policy.
>> Process B is passive in this access control decision, while
>> process A is active.
> Are you stating what you see to be a requirement?
Basic subject/object access control is the core of
the Linux security model. Yes, there are exceptions,
but mostly they're historical in origin.
>> Process A must have write access
>> (defined by some policy) to process B's event buffer.
> No, stop right here.
Listening ...
> Process B is monitoring some aspect of the
> system.
Process B is not "monitoring". At some point in the past it
has registered a request for information should an event occur.
It is currently passive.
> Process A is doing something.
Yes. It is active.'
> Process B should need
> permission to monitor whatever it's monitoring,
OK, I'm good with that. But the only time you
can tell that is when the event is registered,
and at that time you can't tell who might be causing
the event. (Or can you?)
> and process A should
> have permission to do whatever it's doing.
So there needs to be some connection between what B
can request events for and what events A can cause.
Then you can deny B's requests because of A.
> I don't think it makes
> sense to try to ascribe an identity to the actor doing some action to
> decide to omit it from the watch -- this has all kinds of correctness
> issues.
It works for signals and UDP, but in general I get the concern.
> If you're writing a policy and you don't like letting process B spy on
> processes doing various things, then disallow that type of spying.
That gets you into a situation where you can't do the legitimate
monitoring you want to do just because there's the off chance you
might see something you shouldn't. "I hate security! It's confusing,
and always gets in the way!"
>> To
>> implement such a policy requires A's credential,
> You may not design a new mechanism that looks at the credential in a
> context where looking at a credential is invalid unless you have some
> very strong justification for why all of the known reasons that it's a
> bad idea don't apply to what you're doing.
Point. But you also don't get to ignore basic security policy
just because someone's spiffy lazy memory free cache hashing
tree (or similar mechanism) throws away references to important
information while it's still needed.
> So, without a much stronger justification, NAK.
I try to be reasonable. Really. All I want is something
with a security model that can be explained coherently
within the context of the basic Linux security model.
There are enough variations as it is.
^ permalink raw reply
* RE: [RFC PATCH 2/9] x86/sgx: Do not naturally align MAP_FIXED address
From: Xing, Cedric @ 2019-06-04 22:10 UTC (permalink / raw)
To: Andy Lutomirski, Jarkko Sakkinen
Cc: Christopherson, Sean J, Stephen Smalley, James Morris,
Serge E . Hallyn, LSM List, Paul Moore, Eric Paris,
selinux@vger.kernel.org, Jethro Beekman, Hansen, Dave,
Thomas Gleixner, Linus Torvalds, LKML, X86 ML,
linux-sgx@vger.kernel.org, Andrew Morton, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, Andy Shevchenko, Svahn, Kai, Borislav Petkov,
Josh Triplett, Huang, Kai, David Rientjes, Roberts, William C,
Tricca, Philip B
In-Reply-To: <CALCETrVe0jhAWAFmx+NFEjJcijSJv2LDVC7cUXi0w99kNKjh_g@mail.gmail.com>
> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Andy Lutomirski
> Sent: Tuesday, June 04, 2019 1:16 PM
>
> On Tue, Jun 4, 2019 at 4:50 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Fri, May 31, 2019 at 04:31:52PM -0700, Sean Christopherson wrote:
> > > SGX enclaves have an associated Enclave Linear Range (ELRANGE) that
> > > is tracked and enforced by the CPU using a base+mask approach,
> > > similar to how hardware range registers such as the variable MTRRs.
> > > As a result, the ELRANGE must be naturally sized and aligned.
> > >
> > > To reduce boilerplate code that would be needed in every userspace
> > > enclave loader, the SGX driver naturally aligns the mmap() address
> > > and also requires the range to be naturally sized. Unfortunately,
> > > SGX fails to grant a waiver to the MAP_FIXED case, e.g. incorrectly
> > > rejects mmap() if userspace is attempting to map a small slice of an
> existing enclave.
> > >
> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> >
> > Why you want to allow mmap() to be called multiple times? mmap() could
> > be allowed only once with PROT_NONE and denied afterwards. Is this for
> > sending fd to another process that would map already existing enclave?
> >
> > I don't see any checks for whether the is enclave underneath. Also, I
> > think that in all cases mmap() callback should allow only PROT_NONE as
> > permissions for clarity even if it could called multiple times.
> >
>
> What's the advantage to only allowing PROT_NONE? The idea here is to
> allow a PROT_NONE map followed by some replacemets that overlay it for
> the individual segments. Admittedly, mprotect() can do the same thing,
> but disallowing mmap() seems at least a bit surprising.
Disallowing mmap() is not only surprising but also unnecessary.
A bit off topic here. This mmap()/mprotect() discussion reminds me a question (guess for Jarkko): Now that vma->vm_file->private_data keeps a pointer to the enclave, why do we store it again in vma->vm_private? It isn't a big deal but non-NULL vm_private does prevent mprotect() from merging adjacent VMAs.
^ 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