* [PATCH 01/11] keys: Invalidate used request_key authentication keys [ver #2]
From: David Howells @ 2019-04-30 15:06 UTC (permalink / raw)
To: ebiederm
Cc: keyrings, dhowells, linux-security-module, linux-fsdevel,
linux-kernel, dwalsh, vgoyal
In-Reply-To: <155663679069.31331.3777091898004242996.stgit@warthog.procyon.org.uk>
Invalidate used request_key authentication keys rather than revoking them
so that they get cleaned up immediately rather than potentially hanging
around. There doesn't seem any need to keep the revoked keys around.
Signed-off-by: David Howells <dhowells@redhat.com>
---
security/keys/key.c | 4 ++--
security/keys/request_key.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/keys/key.c b/security/keys/key.c
index 696f1c092c50..d705b950ce2a 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -459,7 +459,7 @@ static int __key_instantiate_and_link(struct key *key,
/* disable the authorisation key */
if (authkey)
- key_revoke(authkey);
+ key_invalidate(authkey);
if (prep->expiry != TIME64_MAX) {
key->expiry = prep->expiry;
@@ -607,7 +607,7 @@ int key_reject_and_link(struct key *key,
/* disable the authorisation key */
if (authkey)
- key_revoke(authkey);
+ key_invalidate(authkey);
}
mutex_unlock(&key_construction_mutex);
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 75d87f9e0f49..a7b698394257 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -222,7 +222,7 @@ static int construct_key(struct key *key, const void *callout_info,
/* check that the actor called complete_request_key() prior to
* returning an error */
WARN_ON(ret < 0 &&
- !test_bit(KEY_FLAG_REVOKED, &authkey->flags));
+ !test_bit(KEY_FLAG_INVALIDATED, &authkey->flags));
key_put(authkey);
kleave(" = %d", ret);
^ permalink raw reply related
* Re: [PATCH 1/2] apparmor: Use a memory pool instead per-CPU caches
From: Sebastian Andrzej Siewior @ 2019-04-30 14:47 UTC (permalink / raw)
To: John Johansen; +Cc: linux-security-module, James Morris, Serge E. Hallyn, tglx
In-Reply-To: <ae17e2a3-7d08-5863-4fba-66ddeac11541@canonical.com>
On 2019-04-28 16:56:59 [-0700], John Johansen wrote:
> So digging into why the history of the per cpu buffers in apparmor.
> We used to do buffer allocations via kmalloc and there were a few reasons
> for the switch
>
> * speed/lockless: speaks for it self, mediation is already slow enough
it is shared among all CPUs but it is a small/quick operation to
add/return a buffer.
> * some buffer allocations had to be done with GFP_ATOMIC, making them
> more likely to fail. Since we fail closed that means failure would
> block access. This actually became a serious problem in a couple
> places. Switching to per cpu buffers and blocking pre-empt was
> the solution.
GFP_KERNEL is allowed to use IO/SWAP and ATOMIC has emergency pools. The
new approach won't return a NULL pointer, simply spin to either allocate
new memory or get one which was just returned.
> * in heavy use cases we would see a lot of buffers being allocated
> and freed. Which resulted in locking slow downs and also buffer
> allocation failures. So having the buffers preallocated allowed us
> to bound this potential problem.
>
> This was all 6 years ago. Going to a mem pool certainly could help,
> reduce the memory foot print, and would definitely help with
> preempt/real time kernels.
>
> A big concern with this patchset is reverting back to GFP_KERNEL
> for everything. We definitely were getting failures due to allocations
> in atomic context. There have been lots of changes in the kernel over
> the last six years so it possible these cases don't exist anymore. I
> went through and built some kernels with this patchset and have run
> through some testing without tripping that problem but I don't think
> it has seen enough testing yet.
Do you want apply #1 now and #2 later? I audited the ATOMIC->KERNEL
changes manually and I didn't see any atomic context. It looked like the
only reason for ATOMIC was the preempt_disable() due to the memory pool.
Sebastian
^ permalink raw reply
* Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
From: Bartosz Szczepanek @ 2019-04-30 13:07 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-integrity, peterhuewe, jarkko.sakkinen, jgg, roberto.sassu,
linux-efi, linux-security-module, linux-kernel, tweek,
Matthew Garrett
In-Reply-To: <20190227202658.197113-3-matthewgarrett@google.com>
I may be a little late with this comment, but I've just tested these
patches on aarch64 platform (from the top of jjs/master) and got
kernel panic ("Unable to handle kernel read", full log at the end of
mail). I think there's problem with below call to
tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
passed as (void *) and never remapped:
> + tbl_size = tpm2_calc_event_log_size(final_tbl->events,
> + final_tbl->nr_events,
> + (void *)efi.tpm_log);
This is later used to get efispecid:
> efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
It seems event_header is not mapped during dereference. This is
somewhat expected, because it comes from different, already unmapped
memory region (region of initial TPM log) than "event" itself (which
comes from TPM final log).
Also, value passed as size_info shouldn't be pointing to
linux_efi_tpm_eventlog with its size and version fields, but to the
first event (header event) within. I tried with log_tbl->log and it
worked fine (I omitted unmapping part). On the other hand, with bare
log_tbl it still fails. Not sure how does it even work on other
platforms.
One more thing that's not clear for me – shouldn't the value returned
from early_memremap be used for further accesses? Throughout
__calc_tpm2_event_size() "mapping" is only checked for being zero.
When it is, you're still unmapping it – is it correct?
> + while (count > 0) {
> + header = data + size;
> + event_size = __calc_tpm2_event_size(header, size_info, true);
> + if (event_size == 0)
> + return -1;
> + size += event_size;
> + }
Loop condition here is always true, by the way.
One information about my setup – I'm working with below local diff to
enable operation on ARM:
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/arm-stub.c
> @@ -194,6 +194,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,
>
> /* Ask the firmware to clear memory on unclean shutdown */
> efi_enable_reset_attack_mitigation(sys_table);
> + efi_retrieve_tpm2_eventlog(sys_table);
Full log of kernel panic follows.
EFI stub: Booting Linux Kernel...
EFI stub: EFI_RNG_PROTOCOL unavailable, no randomness supplied
EFI stub: Using DTB from configuration table
EFI stub: Exiting boot services and installing virtual address map...
[ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x420f5162]
[ 0.000000] Linux version 5.1.0-rc2+ (root@localhost.localdomain)
(gcc version 7.3.1 20180712 (Red Hat 7.3.1-6) (GCC)) #69 SMP Fri Apr
26 03:20:57 EDT 2019
[ 0.000000] earlycon: pl11 at MMIO 0x0000000402020000 (options '115200n8')
[ 0.000000] printk: bootconsole [pl11] enabled
[ 0.000000] efi: Getting EFI parameters from FDT:
[ 0.000000] efi: EFI v2.60 by Cavium Inc.
TX2-FW-Release-7.2-build_08-0-g14f8c5bf8a Apr 15 2019 18:51:41
[ 0.000000] efi: TPMFinalLog=0xed5f0000 SMBIOS=0xfad90000 SMBIOS
3.0=0xed530000 ACPI 2.0=0xeda90000 ESRT=0xfafdb218
MEMATTR=0xf8489018 TPMEventLog=0xedaa9018 MEMRESERVE=0xedaa8018
[ 0.000000] Unable to handle kernel read from unreadable memory at
virtual address 00000000edaa9050
[ 0.000000] Mem abort info:
[ 0.000000] ESR = 0x96000004
[ 0.000000] Exception class = DABT (current EL), IL = 32 bits
[ 0.000000] SET = 0, FnV = 0
[ 0.000000] EA = 0, S1PTW = 0
[ 0.000000] Data abort info:
[ 0.000000] ISV = 0, ISS = 0x00000004
[ 0.000000] CM = 0, WnR = 0
[ 0.000000] [00000000edaa9050] user address but active_mm is swapper
[ 0.000000] Internal error: Oops: 96000004 [#1] SMP
[ 0.000000] Modules linked in:
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.1.0-rc2+ #69
[ 0.000000] pstate: 60400089 (nZCv daIf +PAN -UAO)
[ 0.000000] pc : efi_tpm_eventlog_init+0xfc/0x26c
[ 0.000000] lr : efi_tpm_eventlog_init+0xf4/0x26c
[ 0.000000] sp : ffff000011533ce0
[ 0.000000] x29: ffff000011533ce0 x28: 00000000edaa8018
[ 0.000000] x27: ffff7dfffe6fa010 x26: 0000000000000023
[ 0.000000] x25: ffff7dfffe6fa000 x24: 00000000edaa9038
[ 0.000000] x23: 0000000000000000 x22: ffff7dfffe6fa010
[ 0.000000] x21: ffff00001153d000 x20: ffff7dfffe6fa018
[ 0.000000] x19: ffff000011542500 x18: ffffffffffffffff
[ 0.000000] x17: 0000000000000435 x16: 0000000000000000
[ 0.000000] x15: ffff00001153d708 x14: 6576454d50542020
[ 0.000000] x13: 3831303938343866 x12: 78303d525454414d
[ 0.000000] x11: 454d202038313262 x10: 6466616678303d54
[ 0.000000] x9 : ffff00001153ef58 x8 : 0000020000000000
[ 0.000000] x7 : 0000000000000a30 x6 : ffff0000110d2a18
[ 0.000000] x5 : 000000000000013a x4 : 00000000000004c5
[ 0.000000] x3 : ffff000011714000 x2 : 0000000000000002
[ 0.000000] x1 : ffff7dfffe6fa000 x0 : ffff7dfffe73a010
[ 0.000000] Process swapper (pid: 0, stack limit = 0x(____ptrval____))
[ 0.000000] Call trace:
[ 0.000000] efi_tpm_eventlog_init+0xfc/0x26c
[ 0.000000] efi_config_parse_tables+0x180/0x29c
[ 0.000000] uefi_init+0x1d0/0x22c
[ 0.000000] efi_init+0x90/0x180
[ 0.000000] setup_arch+0x1f4/0x5fc
[ 0.000000] start_kernel+0x90/0x51c
[ 0.000000] Code: aa1603e0 97ff05c7 b4000860 b9400ac2 (b9401b01)
[ 0.000000] random: get_random_bytes called from
print_oops_end_marker+0x54/0x70 with crng_init=0
[ 0.000000] ---[ end trace 0000000000000000 ]---
[ 0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
[ 0.000000] ---[ end Kernel panic - not syncing: Attempted to kill
the idle task! ]---
^ permalink raw reply
* Re: [RFC PATCH 2/7] x86/sci: add core implementation for system call isolation
From: Ingo Molnar @ 2019-04-30 11:05 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Andy Lutomirski, Mike Rapoport, LKML, Alexandre Chartre,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar,
James Bottomley, Jonathan Adams, Kees Cook, Paul Turner,
Thomas Gleixner, Linux-MM, LSM List, X86 ML, Linus Torvalds,
Andrew Morton
In-Reply-To: <20190430093857.GO2623@hirez.programming.kicks-ass.net>
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Apr 30, 2019 at 07:03:37AM +0200, Ingo Molnar wrote:
> > So the question IMHO isn't whether it's "valid C", because we already
> > have the Linux kernel's own C syntax variant and are enforcing it with
> > varying degrees of success.
>
> I'm not getting into the whole 'safe' fight here; but you're under
> selling things. We don't have a C syntax, we have a full blown C
> lanugeage variant.
>
> The 'Kernel C' that we write is very much not 'ANSI/ISO C' anymore in a
> fair number of places. And if I can get my way, we'll only diverge
> further from the standard.
Yeah, but I think it would be fair to say that random style variations
aside, in the kernel we still allow about 95%+ of 'sensible C'.
> And this is quite separate from us using every GCC extention under the
> sun; which of course also doesn't help. It mostly has to do with us
> treating C as a portable assembler and the C people not wanting to
> commit to sensible things because they think C is a high-level
> language.
Indeed, and also because there's arguably somewhat of a "if the spec
allows it then performance first, common-sense semantics second" mindset.
Which is an understandable social dynamic, as compiler developers tend to
distinguish themselves via the optimizations they've authored.
Anyway, the main point I tried to make is that I think we'd still be able
to allow 95%+ of "sensible C" even if executed in a "safe runtime", and
we'd still be able to build and run without such strong runtime type
enforcement, i.e. get kernel code close to what we have today, minus a
handful of optimizations and data structures. (But the performance costs
even in that case are nonzero - I'm not sugarcoating it.)
( Plus even that isn't a fully secure solution with deterministic
outcomes, due to parallelism and data races. )
Thanks,
Ingo
^ permalink raw reply
* Re: [RFC PATCH 2/7] x86/sci: add core implementation for system call isolation
From: Peter Zijlstra @ 2019-04-30 9:38 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andy Lutomirski, Mike Rapoport, LKML, Alexandre Chartre,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar,
James Bottomley, Jonathan Adams, Kees Cook, Paul Turner,
Thomas Gleixner, Linux-MM, LSM List, X86 ML, Linus Torvalds,
Andrew Morton
In-Reply-To: <20190430050336.GA92357@gmail.com>
On Tue, Apr 30, 2019 at 07:03:37AM +0200, Ingo Molnar wrote:
> So the question IMHO isn't whether it's "valid C", because we already
> have the Linux kernel's own C syntax variant and are enforcing it with
> varying degrees of success.
I'm not getting into the whole 'safe' fight here; but you're under
selling things. We don't have a C syntax, we have a full blown C
lanugeage variant.
The 'Kernel C' that we write is very much not 'ANSI/ISO C' anymore in a
fair number of places. And if I can get my way, we'll only diverge
further from the standard.
And this is quite separate from us using every GCC extention under the
sun; which of course also doesn't help. It mostly has to do with us
treating C as a portable assembler and the C people not wanting to
commit to sensible things because they think C is a high-level language.
^ permalink raw reply
* Re: [PATCH V32 01/27] Add the ability to lock down access to the running kernel image
From: Andrew Donnellan @ 2019-04-30 5:15 UTC (permalink / raw)
To: Daniel Axtens, Matthew Garrett
Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells,
Linux API, Andy Lutomirski, linuxppc-dev, Michael Ellerman, cmr
In-Reply-To: <87tvehxvh0.fsf@dja-thinkpad.axtens.net>
On 29/4/19 2:54 pm, Daniel Axtens wrote:
> Hi,
>
>>>> I'm thinking about whether we should lock down the powerpc xmon debug
>>>> monitor - intuitively, I think the answer is yes if for no other reason
>>>> than Least Astonishment, when lockdown is enabled you probably don't
>>>> expect xmon to keep letting you access kernel memory.
>>>
>>> The original patchset contained a sysrq hotkey to allow physically
>>> present users to disable lockdown, so I'm not super concerned about
>>> this case - I could definitely be convinced otherwise, though.
>
> So Mimi contacted me offlist and very helpfully provided me with a much
> better and less confused justification for disabling xmon in lockdown:
>
> On x86, physical presence (== console access) is a trigger to
> disable/enable lockdown mode.
>
> In lockdown mode, you're not supposed to be able to modify memory. xmon
> allows you to modify memory, and therefore shouldn't be allowed in
> lockdown.
>
> So, if you can disable lockdown on the console that's probably OK, but
> it should be specifically disabling lockdown, not randomly editing
> memory with xmon.
That makes sense.
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [RFC PATCH 2/7] x86/sci: add core implementation for system call isolation
From: Ingo Molnar @ 2019-04-30 5:03 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Mike Rapoport, LKML, Alexandre Chartre, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Ingo Molnar, James Bottomley,
Jonathan Adams, Kees Cook, Paul Turner, Peter Zijlstra,
Thomas Gleixner, Linux-MM, LSM List, X86 ML, Linus Torvalds,
Peter Zijlstra, Andrew Morton
In-Reply-To: <CALCETrUn_86VAd8FGacJ169xcWE6XQngAMMhvgd1Aa6ZxhGhtA@mail.gmail.com>
* Andy Lutomirski <luto@kernel.org> wrote:
> On Sat, Apr 27, 2019 at 3:46 AM Ingo Molnar <mingo@kernel.org> wrote:
> > So I'm wondering whether there's a 4th choice as well, which avoids
> > control flow corruption *before* it happens:
> >
> > - A C language runtime that is a subset of current C syntax and
> > semantics used in the kernel, and which doesn't allow access outside
> > of existing objects and thus creates a strictly enforced separation
> > between memory used for data, and memory used for code and control
> > flow.
> >
> > - This would involve, at minimum:
> >
> > - tracking every type and object and its inherent length and valid
> > access patterns, and never losing track of its type.
> >
> > - being a lot more organized about initialization, i.e. no
> > uninitialized variables/fields.
> >
> > - being a lot more strict about type conversions and pointers in
> > general.
>
> You're not the only one to suggest this. There are at least a few
> things that make this extremely difficult if not impossible. For
> example, consider this code:
>
> void maybe_buggy(void)
> {
> int a, b;
> int *p = &a;
> int *q = (int *)some_function((unsigned long)p);
> *q = 1;
> }
>
> If some_function(&a) returns &a, then all is well. But if
> some_function(&a) returns &b or even a valid address of some unrelated
> kernel object, then the code might be entirely valid and correct C,
> but I don't see how the runtime checks are supposed to tell whether
> the resulting address is valid or is a bug. This type of code is, I
> think, quite common in the kernel -- it happens in every data
> structure where we have unions of pointers and integers or where we
> steal some known-zero bits of a pointer to store something else.
So the thing is, for the infinitely large state space of "valid C code"
we already disallow an infinitely many versions in the Linux kernel.
We have complicated rules that disallow certain C syntactical and
semantical constructs, both on the tooling (build failure/warning) and on
the review (style/taste) level.
So the question IMHO isn't whether it's "valid C", because we already
have the Linux kernel's own C syntax variant and are enforcing it with
varying degrees of success.
The question is whether the example you gave can be written in a strongly
typed fashion, whether it makes sense to do so, and what the costs are.
I think it's evident that it can be written with strongly typed
constructs, by separating pointers from embedded error codes - with
negative side effects to code generation: for example it increases
structure sizes and error return paths.
I think there's four main costs of converting such a pattern to strongly
typed constructs:
- memory/cache footprint: there's a nonzero cost there.
- performance: this will hurt too.
- code readability: this will probably improve.
- code robustness: this will improve too.
So I think the proper question to ask is not whether there's common C
syntax within the kernel that would have to be rewritten, but whether the
total sum of memory and runtime overhead of strongly typed C programming
(if it's possible/desirable) is larger than the total sum of a typical
Linux distro enabling the various current and proposed kernel hardening
features that have a runtime overhead:
- the SMAP/SMEP overhead of STAC/CLAC for every single user copy
- other usercopy hardening features
- stackprotector
- KASLR
- compiler plugins against information leaks
- proposed KASLR extension to implement module randomization and -PIE overhead
- proposed function call integrity checks
- proposed per system call kernel stack offset randomization
- ( and I'm sure I forgot about a few more, and it's all still only
reactive security, not proactive security. )
That's death by a thousand cuts and CR3 switching during system calls is
also throwing a hand grenade into the fight ;-)
So if people are also proposing to do CR3 switches in every system call,
I'm pretty sure the answer is "yes, even a managed C runtime is probably
faster than *THAT* sum of a performanc mess" - at least with the current
CR3 switching x86-uarch cost structure...
Thanks,
Ingo
^ permalink raw reply
* [PULL] Smack: one more change for 5.2
From: Casey Schaufler @ 2019-04-30 0:38 UTC (permalink / raw)
To: James Morris, Linux Security Module list, Tetsuo Handa; +Cc: casey
James, would you pull this fix from Tetsuo? Thank you.
The following changes since commit f7450bc6e76860564f3842a41892f9b74313cc23:
Smack: Fix IPv6 handling of 0 secmark (2019-04-03 14:28:38 -0700)
are available in the Git repository at:
https://github.com/cschaufler/next-smack.git smack-for-5.2-b
for you to fetch changes up to b9ef5513c99bf9c8bfd9c9e8051b67f52b2dee1e:
smack: Check address length before reading address family (2019-04-29 17:32:27 -0700)
----------------------------------------------------------------
Tetsuo Handa (1):
smack: Check address length before reading address family
security/smack/smack_lsm.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
^ permalink raw reply
* Re: [PATCH V32 01/27] Add the ability to lock down access to the running kernel image
From: Matthew Garrett @ 2019-04-29 22:56 UTC (permalink / raw)
To: James Morris
Cc: LSM List, Linux Kernel Mailing List, David Howells, Linux API,
Andy Lutomirski
In-Reply-To: <20190404003249.14356-2-matthewgarrett@google.com>
Hi James,
What's the best way forward with this? I'm still not entirely clear on
how it can be implemented purely as an LSM, but if you have ideas on
what sort of implementation you'd prefer I'm happy to work on that.
^ permalink raw reply
* [GIT PULL] SELinux fixes for v5.1 (#3)
From: Paul Moore @ 2019-04-29 22:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: selinux, linux-security-module, linux-kernel
Hi Linus,
One small patch for the stable folks to fix a problem when building
against the latest glibc. I'll be honest and say that I'm not really
thrilled with the idea of sending this up right now, but Greg is a
little annoyed so here I figured I would at least send the PR.
-Paul
--
The following changes since commit 6a1afffb08ce5f9fb9ccc20f7ab24846c0142984:
selinux: fix NULL dereference in policydb_destroy()
(2019-03-18 12:19:48 -0400)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
tags/selinux-pr-20190429
for you to fetch changes up to dfbd199a7cfe3e3cd8531e1353cdbd7175bfbc5e:
selinux: use kernel linux/socket.h for genheaders and mdp
(2019-04-29 11:34:58 -0400)
----------------------------------------------------------------
selinux/stable-5.1 PR 20190429
----------------------------------------------------------------
Paulo Alcantara (1):
selinux: use kernel linux/socket.h for genheaders and mdp
scripts/selinux/genheaders/genheaders.c | 1 -
scripts/selinux/mdp/mdp.c | 1 -
security/selinux/include/classmap.h | 1 +
3 files changed, 1 insertion(+), 2 deletions(-)
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH 2/3] smack: Check address length before reading address family
From: Casey Schaufler @ 2019-04-29 20:21 UTC (permalink / raw)
To: James Morris; +Cc: Tetsuo Handa, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1904300558310.20645@namei.org>
On 4/29/2019 12:58 PM, James Morris wrote:
> On Fri, 12 Apr 2019, Casey Schaufler wrote:
>
>> On 4/12/2019 3:59 AM, Tetsuo Handa wrote:
>>> KMSAN will complain if valid address length passed to bind()/connect()/
>>> sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes.
>>>
>>> Also, since smk_ipv6_port_label()/smack_netlabel_send()/
>>> smack_ipv6host_label()/smk_ipv6_check()/smk_ipv6_port_check() are not
>>> checking valid address length and/or address family, make sure we check
>>> both. The minimal valid length in smack_socket_connect() is changed from
>>> sizeof(struct sockaddr_in6) bytes to SIN6_LEN_RFC2133 bytes, for it seems
>>> that Smack is not using "struct sockaddr_in6"->sin6_scope_id field.
>>>
>>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> Casey: will you be taking this via your tree?
Sure. I will add it today.
^ permalink raw reply
* Re: [PATCH] tomoyo: Change pathname calculation for read-only filesystems.
From: James Morris @ 2019-04-29 20:07 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: linux-security-module
In-Reply-To: <52027db5-a4e3-fc5e-4149-529d95d251b2@i-love.sakura.ne.jp>
On Fri, 12 Apr 2019, Tetsuo Handa wrote:
> >> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> >> ---
> >> security/tomoyo/realpath.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c
> >> index 85e6e31..e7832448 100644
> >> --- a/security/tomoyo/realpath.c
> >> +++ b/security/tomoyo/realpath.c
> >> @@ -295,7 +295,8 @@ char *tomoyo_realpath_from_path(const struct path *path)
> >> * or dentry without vfsmount.
> >> */
> >> if (!path->mnt ||
> >> - (!inode->i_op->rename))
> >> + (!inode->i_op->rename &&
> >> + !(sb->s_type->fs_flags & FS_REQUIRES_DEV)))
> >> pos = tomoyo_get_local_path(path->dentry, buf,
> >> buf_len - 1);
> >> /* Get absolute name for the rest. */
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-tomoyo
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH (resend)] tomoyo: Add a kernel config option for fuzzing testing.
From: James Morris @ 2019-04-29 20:07 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: linux-security-module, syzbot, syzbot, syzbot
In-Reply-To: <1555067094-9861-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>
On Fri, 12 Apr 2019, Tetsuo Handa wrote:
> syzbot is reporting kernel panic triggered by memory allocation fault
> injection before loading TOMOYO's policy [1]. To make the fuzzing tests
> useful, we need to assign a profile other than "disabled" (no-op) mode.
> Therefore, let's allow syzbot to load TOMOYO's built-in policy for
> "learning" mode using a kernel config option. This option must not be
> enabled for kernels built for production system, for this option also
> disables domain/program checks when modifying policy configuration via
> /sys/kernel/security/tomoyo/ interface.
>
> [1] https://syzkaller.appspot.com/bug?extid=29569ed06425fcf67a95
>
> Reported-by: syzbot <syzbot+e1b8084e532b6ee7afab@syzkaller.appspotmail.com>
> Reported-by: syzbot <syzbot+29569ed06425fcf67a95@syzkaller.appspotmail.com>
> Reported-by: syzbot <syzbot+2ee3f8974c2e7dc69feb@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> security/tomoyo/Kconfig | 10 ++++++++++
> security/tomoyo/common.c | 13 ++++++++++++-
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-tomoyo
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH 3/3] tomoyo: Check address length before reading address family
From: James Morris @ 2019-04-29 20:06 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: linux-security-module
In-Reply-To: <1555066776-9758-3-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>
On Fri, 12 Apr 2019, Tetsuo Handa wrote:
> KMSAN will complain if valid address length passed to bind()/connect()/
> sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-tomoyo
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH 2/3] smack: Check address length before reading address family
From: James Morris @ 2019-04-29 19:58 UTC (permalink / raw)
To: Casey Schaufler; +Cc: Tetsuo Handa, linux-security-module
In-Reply-To: <738815cc-ac59-63c9-1768-26df95689d7a@schaufler-ca.com>
On Fri, 12 Apr 2019, Casey Schaufler wrote:
> On 4/12/2019 3:59 AM, Tetsuo Handa wrote:
> > KMSAN will complain if valid address length passed to bind()/connect()/
> > sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes.
> >
> > Also, since smk_ipv6_port_label()/smack_netlabel_send()/
> > smack_ipv6host_label()/smk_ipv6_check()/smk_ipv6_port_check() are not
> > checking valid address length and/or address family, make sure we check
> > both. The minimal valid length in smack_socket_connect() is changed from
> > sizeof(struct sockaddr_in6) bytes to SIN6_LEN_RFC2133 bytes, for it seems
> > that Smack is not using "struct sockaddr_in6"->sin6_scope_id field.
> >
> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>
> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Casey: will you be taking this via your tree?
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [RFC PATCH 2/7] x86/sci: add core implementation for system call isolation
From: Andy Lutomirski @ 2019-04-29 18:46 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andy Lutomirski, Mike Rapoport, LKML, Alexandre Chartre,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar,
James Bottomley, Jonathan Adams, Kees Cook, Paul Turner,
Peter Zijlstra, Thomas Gleixner, Linux-MM, LSM List, X86 ML,
Linus Torvalds, Peter Zijlstra, Andrew Morton
In-Reply-To: <20190427104615.GA55518@gmail.com>
On Sat, Apr 27, 2019 at 3:46 AM Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Ingo Molnar <mingo@kernel.org> wrote:
>
> > * Andy Lutomirski <luto@kernel.org> wrote:
> >
> > > > And no, I'm not arguing for Java or C#, but I am arguing for a saner
> > > > version of C.
> > >
> > > IMO three are three credible choices:
> > >
> > > 1. C with fairly strong CFI protection. Grsecurity has this (supposedly
> > > — there’s a distinct lack of source code available), and clang is
> > > gradually working on it.
> > >
> > > 2. A safe language for parts of the kernel, e.g. drivers and maybe
> > > eventually filesystems. Rust is probably the only credible candidate.
> > > Actually creating a decent Rust wrapper around the core kernel
> > > facilities would be quite a bit of work. Things like sysfs would be
> > > interesting in Rust, since AFAIK few or even no drivers actually get
> > > the locking fully correct. This means that naive users of the API
> > > cannot port directly to safe Rust, because all the races won't compile
> > > :)
> > >
> > > 3. A sandbox for parts of the kernel, e.g. drivers. The obvious
> > > candidates are eBPF and WASM.
> > >
> > > #2 will give very good performance. #3 gives potentially stronger
> > > protection against a sandboxed component corrupting the kernel overall,
> > > but it gives much weaker protection against a sandboxed component
> > > corrupting itself.
> > >
> > > In an ideal world, we could do #2 *and* #3. Drivers could, for
> > > example, be written in a language like Rust, compiled to WASM, and run
> > > in the kernel.
> >
> > So why not go for #1, which would still outperform #2/#3, right? Do we
> > know what it would take, roughly, and how the runtime overhead looks
> > like?
>
> BTW., CFI protection is in essence a compiler (or hardware) technique to
> detect stack frame or function pointer corruption after the fact.
>
> So I'm wondering whether there's a 4th choice as well, which avoids
> control flow corruption *before* it happens:
>
> - A C language runtime that is a subset of current C syntax and
> semantics used in the kernel, and which doesn't allow access outside
> of existing objects and thus creates a strictly enforced separation
> between memory used for data, and memory used for code and control
> flow.
>
> - This would involve, at minimum:
>
> - tracking every type and object and its inherent length and valid
> access patterns, and never losing track of its type.
>
> - being a lot more organized about initialization, i.e. no
> uninitialized variables/fields.
>
> - being a lot more strict about type conversions and pointers in
> general.
You're not the only one to suggest this. There are at least a few
things that make this extremely difficult if not impossible. For
example, consider this code:
void maybe_buggy(void)
{
int a, b;
int *p = &a;
int *q = (int *)some_function((unsigned long)p);
*q = 1;
}
If some_function(&a) returns &a, then all is well. But if
some_function(&a) returns &b or even a valid address of some unrelated
kernel object, then the code might be entirely valid and correct C,
but I don't see how the runtime checks are supposed to tell whether
the resulting address is valid or is a bug. This type of code is, I
think, quite common in the kernel -- it happens in every data
structure where we have unions of pointers and integers or where we
steal some known-zero bits of a pointer to store something else.
--Andy
^ permalink raw reply
* Re: [RFC PATCH 2/7] x86/sci: add core implementation for system call isolation
From: Andy Lutomirski @ 2019-04-29 18:43 UTC (permalink / raw)
To: James Morris
Cc: Ingo Molnar, Andy Lutomirski, Mike Rapoport, LKML,
Alexandre Chartre, Borislav Petkov, Dave Hansen, H. Peter Anvin,
Ingo Molnar, James Bottomley, Jonathan Adams, Kees Cook,
Paul Turner, Peter Zijlstra, Thomas Gleixner, Linux-MM, LSM List,
X86 ML, Linus Torvalds, Peter Zijlstra, Andrew Morton
In-Reply-To: <alpine.LRH.2.21.1904300425200.20645@namei.org>
On Mon, Apr 29, 2019 at 11:27 AM James Morris <jmorris@namei.org> wrote:
>
> On Sat, 27 Apr 2019, Ingo Molnar wrote:
>
> > - A C language runtime that is a subset of current C syntax and
> > semantics used in the kernel, and which doesn't allow access outside
> > of existing objects and thus creates a strictly enforced separation
> > between memory used for data, and memory used for code and control
> > flow.
>
> Might be better to start with Rust.
>
I think that Rust would be the clear winner as measured by how fun it sounds :)
^ permalink raw reply
* Re: [RFC PATCH 2/7] x86/sci: add core implementation for system call isolation
From: James Morris @ 2019-04-29 18:26 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andy Lutomirski, Mike Rapoport, LKML, Alexandre Chartre,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar,
James Bottomley, Jonathan Adams, Kees Cook, Paul Turner,
Peter Zijlstra, Thomas Gleixner, Linux-MM, LSM List, X86 ML,
Linus Torvalds, Peter Zijlstra, Andrew Morton
In-Reply-To: <20190427104615.GA55518@gmail.com>
On Sat, 27 Apr 2019, Ingo Molnar wrote:
> - A C language runtime that is a subset of current C syntax and
> semantics used in the kernel, and which doesn't allow access outside
> of existing objects and thus creates a strictly enforced separation
> between memory used for data, and memory used for code and control
> flow.
Might be better to start with Rust.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH] proc: prevent changes to overridden credentials
From: Paul Moore @ 2019-04-29 13:57 UTC (permalink / raw)
To: James Morris
Cc: linux-security-module, selinux, cj.chengjian, john.johansen,
casey
In-Reply-To: <alpine.LRH.2.21.1904200626400.21756@namei.org>
On Fri, Apr 19, 2019 at 4:27 PM James Morris <jmorris@namei.org> wrote:
> On Fri, 19 Apr 2019, Paul Moore wrote:
> > On Fri, Apr 19, 2019 at 2:55 PM Paul Moore <paul@paul-moore.com> wrote:
> > > Prevent userspace from changing the the /proc/PID/attr values if the
> > > task's credentials are currently overriden. This not only makes sense
> > > conceptually, it also prevents some really bizarre error cases caused
> > > when trying to commit credentials to a task with overridden
> > > credentials.
> > >
> > > Cc: <stable@vger.kernel.org>
> > > Reported-by: "chengjian (D)" <cj.chengjian@huawei.com>
> > > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > > ---
> > > fs/proc/base.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> >
> > I sent this to the LSM list as I figure it should probably go via
> > James' linux-security tree since it is cross-LSM and doesn't really
> > contain any LSM specific code. That said, if you don't want this
> > James let me know and I'll send it via the SELinux tree assuming I can
> > get ACKs from John and Casey (this should only affect SELinux,
> > AppArmor, and Smack).
>
> This is fine to go via your tree.
Okay. I just merged this into selinux/next. I was sitting on this
patch to see how the other thread developed, but that doesn't really
seem to be reaching any conclusion and I really want this to get at
least one week in -next before the merge window opens.
Thanks everyone.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH V32 01/27] Add the ability to lock down access to the running kernel image
From: Daniel Axtens @ 2019-04-29 4:54 UTC (permalink / raw)
To: Matthew Garrett, Andrew Donnellan
Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells,
Linux API, Andy Lutomirski, linuxppc-dev, Michael Ellerman, cmr
In-Reply-To: <87wojdy8ro.fsf@dja-thinkpad.axtens.net>
Hi,
>>> I'm thinking about whether we should lock down the powerpc xmon debug
>>> monitor - intuitively, I think the answer is yes if for no other reason
>>> than Least Astonishment, when lockdown is enabled you probably don't
>>> expect xmon to keep letting you access kernel memory.
>>
>> The original patchset contained a sysrq hotkey to allow physically
>> present users to disable lockdown, so I'm not super concerned about
>> this case - I could definitely be convinced otherwise, though.
So Mimi contacted me offlist and very helpfully provided me with a much
better and less confused justification for disabling xmon in lockdown:
On x86, physical presence (== console access) is a trigger to
disable/enable lockdown mode.
In lockdown mode, you're not supposed to be able to modify memory. xmon
allows you to modify memory, and therefore shouldn't be allowed in
lockdown.
So, if you can disable lockdown on the console that's probably OK, but
it should be specifically disabling lockdown, not randomly editing
memory with xmon.
Regards,
Daniel
^ permalink raw reply
* Re: [PATCH V32 01/27] Add the ability to lock down access to the running kernel image
From: Daniel Axtens @ 2019-04-29 0:06 UTC (permalink / raw)
To: Matthew Garrett, Andrew Donnellan
Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells,
Linux API, Andy Lutomirski, linuxppc-dev, Michael Ellerman, cmr
In-Reply-To: <CACdnJus9AhAAYs-R94BH7HDuuQfXjgdhdqUR6Pvk9mxbuPx1=Q@mail.gmail.com>
Matthew Garrett <mjg59@google.com> writes:
> On Tue, Apr 16, 2019 at 1:40 AM Andrew Donnellan
> <andrew.donnellan@au1.ibm.com> wrote:
>> I'm thinking about whether we should lock down the powerpc xmon debug
>> monitor - intuitively, I think the answer is yes if for no other reason
>> than Least Astonishment, when lockdown is enabled you probably don't
>> expect xmon to keep letting you access kernel memory.
>
> The original patchset contained a sysrq hotkey to allow physically
> present users to disable lockdown, so I'm not super concerned about
> this case - I could definitely be convinced otherwise, though.
So currently (and I'm pretty new to this as I've only recently rejoined
IBM) we aren't considering access to the console to be sufficient to
assert physical presence on bare-metal server-class Power machines. The
short argument for this is that with IPMI and BMCs, a server's console
isn't what it used to be. Our console is also a bit different to x86:
we don't generally have bios configuration screens on the console.
In your example, a sysrq key would allow you to disable lockdown after
the system has booted. On Power though, we use Linux as a bootloader
(Petitboot: https://github.com/open-power/petitboot) so being able to
disable lockdown there allows an IPMI-connected user to prevent a signed
kernel being loaded in the first place. I don't know if this is
_actually_ worse, but it certainly feels worse.
There are of course some arguments against our approach. I'm aware of
some of them. I'm also very open to being told that not equating console
access with physical access is fundamentally silly or broken and that we
should rethink things.
Regards,
Daniel
^ permalink raw reply
* Re: [PATCH 1/2] apparmor: Use a memory pool instead per-CPU caches
From: John Johansen @ 2019-04-28 23:56 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, linux-security-module
Cc: James Morris, Serge E. Hallyn, tglx
In-Reply-To: <20190405133458.4809-1-bigeasy@linutronix.de>
On 4/5/19 6:34 AM, Sebastian Andrzej Siewior wrote:
> The get_buffers() macro may provide one or two buffers to the caller.
> Those buffers are preallocated on init for each CPU. By default it
> allocates
> 2* 2 * MAX_PATH * POSSIBLE_CPU
>
> which equals 64KiB on a system with 4 CPUs or 1MiB with 64 CPUs and so
> on.
>
> Replace the per-CPU buffers with a common memory pool which is shared
> across all CPUs. The pool grows on demand and never shrinks.
> By using this pool it is possible to request a buffer and keeping
> preemption enabled which avoids the hack in profile_transition().
>
> During light testing I didn't get more than two buffers in total with
> this patch. So it seems to make sense to allocate the buffers on demand
> and keep them for further use for a quick access.
>
So digging into why the history of the per cpu buffers in apparmor.
We used to do buffer allocations via kmalloc and there were a few reasons
for the switch
* speed/lockless: speaks for it self, mediation is already slow enough
* some buffer allocations had to be done with GFP_ATOMIC, making them
more likely to fail. Since we fail closed that means failure would
block access. This actually became a serious problem in a couple
places. Switching to per cpu buffers and blocking pre-empt was
the solution.
* in heavy use cases we would see a lot of buffers being allocated
and freed. Which resulted in locking slow downs and also buffer
allocation failures. So having the buffers preallocated allowed us
to bound this potential problem.
This was all 6 years ago. Going to a mem pool certainly could help,
reduce the memory foot print, and would definitely help with
preempt/real time kernels.
A big concern with this patchset is reverting back to GFP_KERNEL
for everything. We definitely were getting failures due to allocations
in atomic context. There have been lots of changes in the kernel over
the last six years so it possible these cases don't exist anymore. I
went through and built some kernels with this patchset and have run
through some testing without tripping that problem but I don't think
it has seen enough testing yet.
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> security/apparmor/domain.c | 19 ++-----
> security/apparmor/file.c | 15 +++---
> security/apparmor/include/path.h | 49 +----------------
> security/apparmor/lsm.c | 90 +++++++++++++++-----------------
> security/apparmor/mount.c | 36 ++++++++-----
> 5 files changed, 77 insertions(+), 132 deletions(-)
>
> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> index ca2dccf5b445e..1f4a6e420b6d3 100644
> --- a/security/apparmor/domain.c
> +++ b/security/apparmor/domain.c
> @@ -689,20 +689,9 @@ static struct aa_label *profile_transition(struct aa_profile *profile,
> } else if (COMPLAIN_MODE(profile)) {
> /* no exec permission - learning mode */
> struct aa_profile *new_profile = NULL;
> - char *n = kstrdup(name, GFP_ATOMIC);
>
> - if (n) {
> - /* name is ptr into buffer */
> - long pos = name - buffer;
> - /* break per cpu buffer hold */
> - put_buffers(buffer);
> - new_profile = aa_new_null_profile(profile, false, n,
> - GFP_KERNEL);
> - get_buffers(buffer);
> - name = buffer + pos;
> - strcpy((char *)name, n);
> - kfree(n);
> - }
> + new_profile = aa_new_null_profile(profile, false, name,
> + GFP_KERNEL);
> if (!new_profile) {
> error = -ENOMEM;
> info = "could not create null profile";
> @@ -907,7 +896,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
> ctx->nnp = aa_get_label(label);
>
> /* buffer freed below, name is pointer into buffer */
> - get_buffers(buffer);
> + buffer = aa_get_buffer();
> /* Test for onexec first as onexec override other x transitions. */
> if (ctx->onexec)
> new = handle_onexec(label, ctx->onexec, ctx->token,
> @@ -979,7 +968,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>
> done:
> aa_put_label(label);
> - put_buffers(buffer);
> + aa_put_buffer(buffer);
>
> return error;
>
> diff --git a/security/apparmor/file.c b/security/apparmor/file.c
> index d0afed9ebd0ed..e422a3f59e80c 100644
> --- a/security/apparmor/file.c
> +++ b/security/apparmor/file.c
> @@ -336,12 +336,12 @@ int aa_path_perm(const char *op, struct aa_label *label,
>
> flags |= PATH_DELEGATE_DELETED | (S_ISDIR(cond->mode) ? PATH_IS_DIR :
> 0);
> - get_buffers(buffer);
> + buffer = aa_get_buffer();
> error = fn_for_each_confined(label, profile,
> profile_path_perm(op, profile, path, buffer, request,
> cond, flags, &perms));
>
> - put_buffers(buffer);
> + aa_put_buffer(buffer);
>
> return error;
> }
> @@ -479,12 +479,13 @@ int aa_path_link(struct aa_label *label, struct dentry *old_dentry,
> int error;
>
> /* buffer freed below, lname is pointer in buffer */
> - get_buffers(buffer, buffer2);
> + buffer = aa_get_buffer();
> + buffer2 = aa_get_buffer();
> error = fn_for_each_confined(label, profile,
> profile_path_link(profile, &link, buffer, &target,
> buffer2, &cond));
> - put_buffers(buffer, buffer2);
> -
> + aa_put_buffer(buffer);
> + aa_put_buffer(buffer2);
> return error;
> }
>
> @@ -528,7 +529,7 @@ static int __file_path_perm(const char *op, struct aa_label *label,
> return 0;
>
> flags = PATH_DELEGATE_DELETED | (S_ISDIR(cond.mode) ? PATH_IS_DIR : 0);
> - get_buffers(buffer);
> + buffer = aa_get_buffer();
>
> /* check every profile in task label not in current cache */
> error = fn_for_each_not_in_set(flabel, label, profile,
> @@ -557,7 +558,7 @@ static int __file_path_perm(const char *op, struct aa_label *label,
> if (!error)
> update_file_ctx(file_ctx(file), label, request);
>
> - put_buffers(buffer);
> + aa_put_buffer(buffer);
>
> return error;
> }
> diff --git a/security/apparmor/include/path.h b/security/apparmor/include/path.h
> index b6380c5f00972..b0b2ab85e42d8 100644
> --- a/security/apparmor/include/path.h
> +++ b/security/apparmor/include/path.h
> @@ -15,7 +15,6 @@
> #ifndef __AA_PATH_H
> #define __AA_PATH_H
>
> -
> enum path_flags {
> PATH_IS_DIR = 0x1, /* path is a directory */
> PATH_CONNECT_PATH = 0x4, /* connect disconnected paths to / */
> @@ -30,51 +29,7 @@ int aa_path_name(const struct path *path, int flags, char *buffer,
> const char **name, const char **info,
> const char *disconnected);
>
> -#define MAX_PATH_BUFFERS 2
> -
> -/* Per cpu buffers used during mediation */
> -/* preallocated buffers to use during path lookups */
> -struct aa_buffers {
> - char *buf[MAX_PATH_BUFFERS];
> -};
> -
> -#include <linux/percpu.h>
> -#include <linux/preempt.h>
> -
> -DECLARE_PER_CPU(struct aa_buffers, aa_buffers);
> -
> -#define ASSIGN(FN, A, X, N) ((X) = FN(A, N))
> -#define EVAL1(FN, A, X) ASSIGN(FN, A, X, 0) /*X = FN(0)*/
> -#define EVAL2(FN, A, X, Y...) \
> - do { ASSIGN(FN, A, X, 1); EVAL1(FN, A, Y); } while (0)
> -#define EVAL(FN, A, X...) CONCATENATE(EVAL, COUNT_ARGS(X))(FN, A, X)
> -
> -#define for_each_cpu_buffer(I) for ((I) = 0; (I) < MAX_PATH_BUFFERS; (I)++)
> -
> -#ifdef CONFIG_DEBUG_PREEMPT
> -#define AA_BUG_PREEMPT_ENABLED(X) AA_BUG(preempt_count() <= 0, X)
> -#else
> -#define AA_BUG_PREEMPT_ENABLED(X) /* nop */
> -#endif
> -
> -#define __get_buffer(C, N) ({ \
> - AA_BUG_PREEMPT_ENABLED("__get_buffer without preempt disabled"); \
> - (C)->buf[(N)]; })
> -
> -#define __get_buffers(C, X...) EVAL(__get_buffer, C, X)
> -
> -#define __put_buffers(X, Y...) ((void)&(X))
> -
> -#define get_buffers(X...) \
> -do { \
> - struct aa_buffers *__cpu_var = get_cpu_ptr(&aa_buffers); \
> - __get_buffers(__cpu_var, X); \
> -} while (0)
> -
> -#define put_buffers(X, Y...) \
> -do { \
> - __put_buffers(X, Y); \
> - put_cpu_ptr(&aa_buffers); \
> -} while (0)
> +char *aa_get_buffer(void);
> +void aa_put_buffer(char *buf);
>
> #endif /* __AA_PATH_H */
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 49d664ddff444..224a99b12bc54 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -47,8 +47,13 @@
> /* Flag indicating whether initialization completed */
> int apparmor_initialized;
>
> -DEFINE_PER_CPU(struct aa_buffers, aa_buffers);
> -
> +/* aa_g_path_max */
> +union aa_buffer {
> + struct list_head list;
> + char buffer[1];
> +};
> +static LIST_HEAD(aa_global_buffers);
> +static DEFINE_SPINLOCK(aa_buffers_lock);
>
> /*
> * LSM hook functions
> @@ -1399,6 +1404,7 @@ static int param_set_aauint(const char *val, const struct kernel_param *kp)
> return -EPERM;
>
> error = param_set_uint(val, kp);
> + aa_g_path_max = min_t(uint32_t, aa_g_path_max, sizeof(union aa_buffer));
> pr_info("AppArmor: buffer size set to %d bytes\n", aa_g_path_max);
>
> return error;
> @@ -1471,6 +1477,38 @@ static int param_set_mode(const char *val, const struct kernel_param *kp)
> return 0;
> }
>
> +char *aa_get_buffer(void)
> +{
> + union aa_buffer *aa_buf;
> +
> +try_again:
> + spin_lock(&aa_buffers_lock);
> + if (!list_empty(&aa_global_buffers)) {
> + aa_buf = list_first_entry(&aa_global_buffers, union aa_buffer,
> + list);
> + list_del(&aa_buf->list);
> + spin_unlock(&aa_buffers_lock);
> + return &aa_buf->buffer[0];
> + }
> + spin_unlock(&aa_buffers_lock);
> +
> + aa_buf = kmalloc(aa_g_path_max, GFP_KERNEL);
> + if (WARN_ON_ONCE(!aa_buf))
> + goto try_again;
> + return &aa_buf->buffer[0];
> +}
> +
> +void aa_put_buffer(char *buf)
> +{
> + union aa_buffer *aa_buf;
> +
> + aa_buf = container_of(buf, union aa_buffer, buffer[0]);
> +
> + spin_lock(&aa_buffers_lock);
> + list_add(&aa_buf->list, &aa_global_buffers);
> + spin_unlock(&aa_buffers_lock);
> +}
> +
> /*
> * AppArmor init functions
> */
> @@ -1489,43 +1527,6 @@ static int __init set_init_ctx(void)
> return 0;
> }
>
> -static void destroy_buffers(void)
> -{
> - u32 i, j;
> -
> - for_each_possible_cpu(i) {
> - for_each_cpu_buffer(j) {
> - kfree(per_cpu(aa_buffers, i).buf[j]);
> - per_cpu(aa_buffers, i).buf[j] = NULL;
> - }
> - }
> -}
> -
> -static int __init alloc_buffers(void)
> -{
> - u32 i, j;
> -
> - for_each_possible_cpu(i) {
> - for_each_cpu_buffer(j) {
> - char *buffer;
> -
> - if (cpu_to_node(i) > num_online_nodes())
> - /* fallback to kmalloc for offline nodes */
> - buffer = kmalloc(aa_g_path_max, GFP_KERNEL);
> - else
> - buffer = kmalloc_node(aa_g_path_max, GFP_KERNEL,
> - cpu_to_node(i));
> - if (!buffer) {
> - destroy_buffers();
> - return -ENOMEM;
> - }
> - per_cpu(aa_buffers, i).buf[j] = buffer;
> - }
> - }
> -
> - return 0;
> -}
> -
> #ifdef CONFIG_SYSCTL
> static int apparmor_dointvec(struct ctl_table *table, int write,
> void __user *buffer, size_t *lenp, loff_t *ppos)
> @@ -1684,17 +1685,11 @@ static int __init apparmor_init(void)
>
> }
>
> - error = alloc_buffers();
> - if (error) {
> - AA_ERROR("Unable to allocate work buffers\n");
> - goto buffers_out;
> - }
> -
> error = set_init_ctx();
> if (error) {
> AA_ERROR("Failed to set context on init task\n");
> aa_free_root_ns();
> - goto buffers_out;
> + goto alloc_out;
> }
> security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
> "apparmor");
> @@ -1710,9 +1705,6 @@ static int __init apparmor_init(void)
>
> return error;
>
> -buffers_out:
> - destroy_buffers();
> -
> alloc_out:
> aa_destroy_aafs();
> aa_teardown_dfa_engine();
> diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
> index 8c3787399356b..8a6cf1c14e82a 100644
> --- a/security/apparmor/mount.c
> +++ b/security/apparmor/mount.c
> @@ -412,11 +412,11 @@ int aa_remount(struct aa_label *label, const struct path *path,
>
> binary = path->dentry->d_sb->s_type->fs_flags & FS_BINARY_MOUNTDATA;
>
> - get_buffers(buffer);
> + buffer = aa_get_buffer();
> error = fn_for_each_confined(label, profile,
> match_mnt(profile, path, buffer, NULL, NULL, NULL,
> flags, data, binary));
> - put_buffers(buffer);
> + aa_put_buffer(buffer);
>
> return error;
> }
> @@ -441,11 +441,13 @@ int aa_bind_mount(struct aa_label *label, const struct path *path,
> if (error)
> return error;
>
> - get_buffers(buffer, old_buffer);
> + buffer = aa_get_buffer();
> + old_buffer = aa_get_buffer();
> error = fn_for_each_confined(label, profile,
> match_mnt(profile, path, buffer, &old_path, old_buffer,
> NULL, flags, NULL, false));
> - put_buffers(buffer, old_buffer);
> + aa_put_buffer(buffer);
> + aa_put_buffer(old_buffer);
> path_put(&old_path);
>
> return error;
> @@ -465,11 +467,11 @@ int aa_mount_change_type(struct aa_label *label, const struct path *path,
> flags &= (MS_REC | MS_SILENT | MS_SHARED | MS_PRIVATE | MS_SLAVE |
> MS_UNBINDABLE);
>
> - get_buffers(buffer);
> + buffer = aa_get_buffer();
> error = fn_for_each_confined(label, profile,
> match_mnt(profile, path, buffer, NULL, NULL, NULL,
> flags, NULL, false));
> - put_buffers(buffer);
> + aa_put_buffer(buffer);
>
> return error;
> }
> @@ -492,11 +494,13 @@ int aa_move_mount(struct aa_label *label, const struct path *path,
> if (error)
> return error;
>
> - get_buffers(buffer, old_buffer);
> + buffer = aa_get_buffer();
> + old_buffer = aa_get_buffer();
> error = fn_for_each_confined(label, profile,
> match_mnt(profile, path, buffer, &old_path, old_buffer,
> NULL, MS_MOVE, NULL, false));
> - put_buffers(buffer, old_buffer);
> + aa_put_buffer(buffer);
> + aa_put_buffer(old_buffer);
> path_put(&old_path);
>
> return error;
> @@ -537,17 +541,19 @@ int aa_new_mount(struct aa_label *label, const char *dev_name,
> }
> }
>
> - get_buffers(buffer, dev_buffer);
> + buffer = aa_get_buffer();
> if (dev_path) {
> error = fn_for_each_confined(label, profile,
> match_mnt(profile, path, buffer, dev_path, dev_buffer,
> type, flags, data, binary));
> } else {
> + dev_buffer = aa_get_buffer();
> error = fn_for_each_confined(label, profile,
> match_mnt_path_str(profile, path, buffer, dev_name,
> type, flags, data, binary, NULL));
> + aa_put_buffer(dev_buffer);
> }
> - put_buffers(buffer, dev_buffer);
> + aa_put_buffer(buffer);
> if (dev_path)
> path_put(dev_path);
>
> @@ -595,10 +601,10 @@ int aa_umount(struct aa_label *label, struct vfsmount *mnt, int flags)
> AA_BUG(!label);
> AA_BUG(!mnt);
>
> - get_buffers(buffer);
> + buffer = aa_get_buffer();
> error = fn_for_each_confined(label, profile,
> profile_umount(profile, &path, buffer));
> - put_buffers(buffer);
> + aa_put_buffer(buffer);
>
> return error;
> }
> @@ -671,7 +677,8 @@ int aa_pivotroot(struct aa_label *label, const struct path *old_path,
> AA_BUG(!old_path);
> AA_BUG(!new_path);
>
> - get_buffers(old_buffer, new_buffer);
> + old_buffer = aa_get_buffer();
> + new_buffer = aa_get_buffer();
> target = fn_label_build(label, profile, GFP_ATOMIC,
> build_pivotroot(profile, new_path, new_buffer,
> old_path, old_buffer));
> @@ -690,7 +697,8 @@ int aa_pivotroot(struct aa_label *label, const struct path *old_path,
> /* already audited error */
> error = PTR_ERR(target);
> out:
> - put_buffers(old_buffer, new_buffer);
> + aa_put_buffer(old_buffer);
> + aa_put_buffer(new_buffer);
>
> return error;
>
>
^ permalink raw reply
* Re: [RFC PATCH 0/7] x86: introduce system calls addess space isolation
From: Mike Rapoport @ 2019-04-28 6:08 UTC (permalink / raw)
To: Dave Hansen
Cc: linux-kernel, Alexandre Chartre, Andy Lutomirski, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Ingo Molnar, James Bottomley,
Jonathan Adams, Kees Cook, Paul Turner, Peter Zijlstra,
Thomas Gleixner, linux-mm, linux-security-module, x86
In-Reply-To: <1c12e195-1286-0136-eae5-4b392d9fe4c0@intel.com>
On Fri, Apr 26, 2019 at 07:41:09AM -0700, Dave Hansen wrote:
> On 4/25/19 2:45 PM, Mike Rapoport wrote:
> > The idea behind the prevention is that if we fault in pages in the
> > execution path, we can compare target address against the kernel symbol
> > table. So if we're in a function, we allow local jumps (and simply falling
> > of the end of a page) but if we're jumping to a new function it must be to
> > an external label in the symbol table. Since ROP attacks are all about
> > jumping to gadget code which is effectively in the middle of real
> > functions, the jumps they induce are to code that doesn't have an external
> > symbol, so it should mostly detect when they happen.
>
> This turns the problem from: "attackers can leverage any data/code that
> the kernel has mapped (anything)" to "attackers can leverage any
> code/data that the current syscall has faulted in".
>
> That seems like a pretty restrictive change.
>
> > At this time we are not suggesting any API that will enable the system
> > calls isolation. Because of the overhead required for this, it should only
> > be activated for processes or containers we know should be untrusted. We
> > still have no actual numbers, but surely forcing page faults during system
> > call execution will not come for free.
>
> What's the minimum number of faults that have to occur to handle the
> simplest dummy fault?
For the current implementation it's 3.
Here is the example trace of #PF's produced by a dummy get_answer
system call from patch 7:
[ 12.012906] #PF: DATA: do_syscall_64+0x26b/0x4c0 fault at 0xffffffff82000bb8
[ 12.012918] #PF: INSN: __x86_indirect_thunk_rax+0x0/0x20 fault at __x86_indirect_thunk_rax+0x0/0x20
[ 12.012929] #PF: INSN: __x64_sys_get_answer+0x0/0x10 fault at__x64_sys_get_answer+0x0/0x10
For the sci_write_dmesg syscall that does copy_from_user() and printk() its
between 35 and 60 depending on console and /proc/sys/kernel/printk values.
This includes both code and data accesses. The data page faults can be
avoided if we pre-populate SCI page tables with data.
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC PATCH 0/7] x86: introduce system calls addess space isolation
From: Mike Rapoport @ 2019-04-28 6:01 UTC (permalink / raw)
To: Andy Lutomirski
Cc: LKML, Alexandre Chartre, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Ingo Molnar, James Bottomley, Jonathan Adams,
Kees Cook, Paul Turner, Peter Zijlstra, Thomas Gleixner, Linux-MM,
LSM List, X86 ML
In-Reply-To: <CALCETrWkYYh=L1nSO7GYt0FvMhjCcEaQiM2JEi3FfkJbYJFh2g@mail.gmail.com>
On Thu, Apr 25, 2019 at 05:30:13PM -0700, Andy Lutomirski wrote:
> On Thu, Apr 25, 2019 at 2:46 PM Mike Rapoport <rppt@linux.ibm.com> wrote:
> >
> > Hi,
> >
> > Address space isolation has been used to protect the kernel from the
> > userspace and userspace programs from each other since the invention of the
> > virtual memory.
> >
> > Assuming that kernel bugs and therefore vulnerabilities are inevitable it
> > might be worth isolating parts of the kernel to minimize damage that these
> > vulnerabilities can cause.
> >
> > The idea here is to allow an untrusted user access to a potentially
> > vulnerable kernel in such a way that any kernel vulnerability they find to
> > exploit is either prevented or the consequences confined to their isolated
> > address space such that the compromise attempt has minimal impact on other
> > tenants or the protected structures of the monolithic kernel. Although we
> > hope to prevent many classes of attack, the first target we're looking at
> > is ROP gadget protection.
> >
> > These patches implement a "system call isolation (SCI)" mechanism that
> > allows running system calls in an isolated address space with reduced page
> > tables to prevent ROP attacks.
> >
> > ROP attacks involve corrupting the stack return address to repoint it to a
> > segment of code you know exists in the kernel that can be used to perform
> > the action you need to exploit the system.
> >
> > The idea behind the prevention is that if we fault in pages in the
> > execution path, we can compare target address against the kernel symbol
> > table. So if we're in a function, we allow local jumps (and simply falling
> > of the end of a page) but if we're jumping to a new function it must be to
> > an external label in the symbol table.
>
> That's quite an assumption. The entry code at least uses .L labels.
> Do you get that right?
>
> As far as I can see, most of what's going on here has very little to
> do with jumps and calls. The benefit seems to come from making sure
> that the RET instruction actually goes somewhere that's already been
> faulted in. Am I understanding right?
Well, RET indeed will go somewhere that's already been faulted in. But
before that, the first CALL to not-yet-mapped code will fault and bring in
the page containing the CALL target.
If the CALL is made into a middle of a function, SCI will refuse to
continue the syscall execution.
As for the local jumps, as long as they are inside a page that was already
mapped or the next page, they are allowed.
This does not take care (yet) of larger functions where local jumps are
further then PAGE_SIZE.
Here's an example trace of #PF's produced by a dummy get_answer system call
from patch 7:
[ 12.012906] #PF: DATA: do_syscall_64+0x26b/0x4c0 fault at 0xffffffff82000bb8
[ 12.012918] #PF: INSN: __x86_indirect_thunk_rax+0x0/0x20 fault at __x86_indirect_thunk_rax+0x0/0x20
[ 12.012929] #PF: INSN: __x64_sys_get_answer+0x0/0x10 fault at __x64_sys_get_answer+0x0/0x10
> --Andy
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC PATCH 5/7] x86/mm/fault: hook up SCI verification
From: Mike Rapoport @ 2019-04-28 5:47 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Alexandre Chartre, Andy Lutomirski, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Ingo Molnar, James Bottomley,
Jonathan Adams, Kees Cook, Paul Turner, Thomas Gleixner, linux-mm,
linux-security-module, x86
In-Reply-To: <20190426074223.GY4038@hirez.programming.kicks-ass.net>
On Fri, Apr 26, 2019 at 09:42:23AM +0200, Peter Zijlstra wrote:
> On Fri, Apr 26, 2019 at 12:45:52AM +0300, Mike Rapoport wrote:
> > If a system call runs in isolated context, it's accesses to kernel code and
> > data will be verified by SCI susbsytem.
> >
> > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > ---
> > arch/x86/mm/fault.c | 28 ++++++++++++++++++++++++++++
> > 1 file changed, 28 insertions(+)
>
> There's a distinct lack of touching do_double_fault(). It appears to me
> that you'll instantly trigger #DF when you #PF, because the #PF handler
> itself will not be able to run.
The #PF handler is able to run. On interrupt/error entry the cr3 is
switched to the full kernel page tables, pretty much like PTI does for
user <-> kernel transitions. It's in the patch 3.
> And then obviously you have to be very careful to make sure #DF can,
> _at_all_times_ run, otherwise you'll tripple-fault and we all know what
> that does.
>
--
Sincerely yours,
Mike.
^ 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