* [PATCH v4 17/23] modules: Use vmalloc special flag
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Rick Edgecombe, Jessica Yu,
Steven Rostedt
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
Use new flag for handling freeing of special permissioned memory in vmalloc
and remove places where memory was set RW before freeing which is no longer
needed.
Since freeing of VM_FLUSH_RESET_PERMS memory is not supported in an
interrupt by vmalloc, the freeing of init sections is moved to a work
queue. Instead of call_rcu it now uses synchronize_rcu() in the work
queue.
Lastly, there is now a WARN_ON in module_memfree since it should not be
called in an interrupt with special memory as is required for
VM_FLUSH_RESET_PERMS.
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
kernel/module.c | 77 +++++++++++++++++++++++++------------------------
1 file changed, 39 insertions(+), 38 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 2b2845ae983e..a9020bdd4cf6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -98,6 +98,10 @@ DEFINE_MUTEX(module_mutex);
EXPORT_SYMBOL_GPL(module_mutex);
static LIST_HEAD(modules);
+/* Work queue for freeing init sections in success case */
+static struct work_struct init_free_wq;
+static struct llist_head init_free_list;
+
#ifdef CONFIG_MODULES_TREE_LOOKUP
/*
@@ -1949,6 +1953,8 @@ void module_enable_ro(const struct module *mod, bool after_init)
if (!rodata_enabled)
return;
+ set_vm_flush_reset_perms(mod->core_layout.base);
+ set_vm_flush_reset_perms(mod->init_layout.base);
frob_text(&mod->core_layout, set_memory_ro);
frob_text(&mod->core_layout, set_memory_x);
@@ -1972,15 +1978,6 @@ static void module_enable_nx(const struct module *mod)
frob_writable_data(&mod->init_layout, set_memory_nx);
}
-static void module_disable_nx(const struct module *mod)
-{
- frob_rodata(&mod->core_layout, set_memory_x);
- frob_ro_after_init(&mod->core_layout, set_memory_x);
- frob_writable_data(&mod->core_layout, set_memory_x);
- frob_rodata(&mod->init_layout, set_memory_x);
- frob_writable_data(&mod->init_layout, set_memory_x);
-}
-
/* Iterate through all modules and set each module's text as RW */
void set_all_modules_text_rw(void)
{
@@ -2024,23 +2021,8 @@ void set_all_modules_text_ro(void)
}
mutex_unlock(&module_mutex);
}
-
-static void disable_ro_nx(const struct module_layout *layout)
-{
- if (rodata_enabled) {
- frob_text(layout, set_memory_rw);
- frob_rodata(layout, set_memory_rw);
- frob_ro_after_init(layout, set_memory_rw);
- }
- frob_rodata(layout, set_memory_x);
- frob_ro_after_init(layout, set_memory_x);
- frob_writable_data(layout, set_memory_x);
-}
-
#else
-static void disable_ro_nx(const struct module_layout *layout) { }
static void module_enable_nx(const struct module *mod) { }
-static void module_disable_nx(const struct module *mod) { }
#endif
#ifdef CONFIG_LIVEPATCH
@@ -2120,6 +2102,11 @@ static void free_module_elf(struct module *mod)
void __weak module_memfree(void *module_region)
{
+ /*
+ * This memory may be RO, and freeing RO memory in an interrupt is not
+ * supported by vmalloc.
+ */
+ WARN_ON(in_interrupt());
vfree(module_region);
}
@@ -2171,7 +2158,6 @@ static void free_module(struct module *mod)
mutex_unlock(&module_mutex);
/* This may be empty, but that's OK */
- disable_ro_nx(&mod->init_layout);
module_arch_freeing_init(mod);
module_memfree(mod->init_layout.base);
kfree(mod->args);
@@ -2181,7 +2167,6 @@ static void free_module(struct module *mod)
lockdep_free_key_range(mod->core_layout.base, mod->core_layout.size);
/* Finally, free the core (containing the module structure) */
- disable_ro_nx(&mod->core_layout);
module_memfree(mod->core_layout.base);
}
@@ -3420,17 +3405,34 @@ static void do_mod_ctors(struct module *mod)
/* For freeing module_init on success, in case kallsyms traversing */
struct mod_initfree {
- struct rcu_head rcu;
+ struct llist_node node;
void *module_init;
};
-static void do_free_init(struct rcu_head *head)
+static void do_free_init(struct work_struct *w)
{
- struct mod_initfree *m = container_of(head, struct mod_initfree, rcu);
- module_memfree(m->module_init);
- kfree(m);
+ struct llist_node *pos, *n, *list;
+ struct mod_initfree *initfree;
+
+ list = llist_del_all(&init_free_list);
+
+ synchronize_rcu();
+
+ llist_for_each_safe(pos, n, list) {
+ initfree = container_of(pos, struct mod_initfree, node);
+ module_memfree(initfree->module_init);
+ kfree(initfree);
+ }
}
+static int __init modules_wq_init(void)
+{
+ INIT_WORK(&init_free_wq, do_free_init);
+ init_llist_head(&init_free_list);
+ return 0;
+}
+module_init(modules_wq_init);
+
/*
* This is where the real work happens.
*
@@ -3507,7 +3509,6 @@ static noinline int do_init_module(struct module *mod)
#endif
module_enable_ro(mod, true);
mod_tree_remove_init(mod);
- disable_ro_nx(&mod->init_layout);
module_arch_freeing_init(mod);
mod->init_layout.base = NULL;
mod->init_layout.size = 0;
@@ -3518,14 +3519,18 @@ static noinline int do_init_module(struct module *mod)
* We want to free module_init, but be aware that kallsyms may be
* walking this with preempt disabled. In all the failure paths, we
* call synchronize_rcu(), but we don't want to slow down the success
- * path, so use actual RCU here.
+ * path. module_memfree() cannot be called in an interrupt, so do the
+ * work and call synchronize_rcu() in a work queue.
+ *
* Note that module_alloc() on most architectures creates W+X page
* mappings which won't be cleaned up until do_free_init() runs. Any
* code such as mark_rodata_ro() which depends on those mappings to
* be cleaned up needs to sync with the queued work - ie
* rcu_barrier()
*/
- call_rcu(&freeinit->rcu, do_free_init);
+ if (llist_add(&freeinit->node, &init_free_list))
+ schedule_work(&init_free_wq);
+
mutex_unlock(&module_mutex);
wake_up_all(&module_wq);
@@ -3822,10 +3827,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
module_bug_cleanup(mod);
mutex_unlock(&module_mutex);
- /* we can't deallocate the module until we clear memory protection */
- module_disable_ro(mod);
- module_disable_nx(mod);
-
ddebug_cleanup:
ftrace_release_mod(mod);
dynamic_debug_remove(mod, info->debug);
--
2.17.1
^ permalink raw reply related
* [PATCH v4 19/23] x86/ftrace: Use vmalloc special flag
From: Rick Edgecombe @ 2019-04-22 18:58 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Rick Edgecombe, Steven Rostedt
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
Use new flag VM_FLUSH_RESET_PERMS for handling freeing of special
permissioned memory in vmalloc and remove places where memory was set NX
and RW before freeing which is no longer needed.
Cc: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/ftrace.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 53ba1aa3a01f..0caf8122d680 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -678,12 +678,8 @@ static inline void *alloc_tramp(unsigned long size)
{
return module_alloc(size);
}
-static inline void tramp_free(void *tramp, int size)
+static inline void tramp_free(void *tramp)
{
- int npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
-
- set_memory_nx((unsigned long)tramp, npages);
- set_memory_rw((unsigned long)tramp, npages);
module_memfree(tramp);
}
#else
@@ -692,7 +688,7 @@ static inline void *alloc_tramp(unsigned long size)
{
return NULL;
}
-static inline void tramp_free(void *tramp, int size) { }
+static inline void tramp_free(void *tramp) { }
#endif
/* Defined as markers to the end of the ftrace default trampolines */
@@ -808,6 +804,8 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
/* ALLOC_TRAMP flags lets us know we created it */
ops->flags |= FTRACE_OPS_FL_ALLOC_TRAMP;
+ set_vm_flush_reset_perms(trampoline);
+
/*
* Module allocation needs to be completed by making the page
* executable. The page is still writable, which is a security hazard,
@@ -816,7 +814,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
set_memory_x((unsigned long)trampoline, npages);
return (unsigned long)trampoline;
fail:
- tramp_free(trampoline, *tramp_size);
+ tramp_free(trampoline);
return 0;
}
@@ -947,7 +945,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops)
if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
return;
- tramp_free((void *)ops->trampoline, ops->trampoline_size);
+ tramp_free((void *)ops->trampoline);
ops->trampoline = 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 18/23] bpf: Use vmalloc special flag
From: Rick Edgecombe @ 2019-04-22 18:58 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Rick Edgecombe, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
Use new flag VM_FLUSH_RESET_PERMS for handling freeing of special
permissioned memory in vmalloc and remove places where memory was set RW
before freeing which is no longer needed. Don't track if the memory is RO
anymore because it is now tracked in vmalloc.
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
include/linux/filter.h | 17 +++--------------
kernel/bpf/core.c | 1 -
2 files changed, 3 insertions(+), 15 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 14ec3bdad9a9..7d3abde3f183 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -20,6 +20,7 @@
#include <linux/set_memory.h>
#include <linux/kallsyms.h>
#include <linux/if_vlan.h>
+#include <linux/vmalloc.h>
#include <net/sch_generic.h>
@@ -503,7 +504,6 @@ struct bpf_prog {
u16 pages; /* Number of allocated pages */
u16 jited:1, /* Is our filter JIT'ed? */
jit_requested:1,/* archs need to JIT the prog */
- undo_set_mem:1, /* Passed set_memory_ro() checkpoint */
gpl_compatible:1, /* Is filter GPL compatible? */
cb_access:1, /* Is control block accessed? */
dst_needed:1, /* Do we need dst entry? */
@@ -733,27 +733,17 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
{
- fp->undo_set_mem = 1;
+ set_vm_flush_reset_perms(fp);
set_memory_ro((unsigned long)fp, fp->pages);
}
-static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
-{
- if (fp->undo_set_mem)
- set_memory_rw((unsigned long)fp, fp->pages);
-}
-
static inline void bpf_jit_binary_lock_ro(struct bpf_binary_header *hdr)
{
+ set_vm_flush_reset_perms(hdr);
set_memory_ro((unsigned long)hdr, hdr->pages);
set_memory_x((unsigned long)hdr, hdr->pages);
}
-static inline void bpf_jit_binary_unlock_ro(struct bpf_binary_header *hdr)
-{
- set_memory_rw((unsigned long)hdr, hdr->pages);
-}
-
static inline struct bpf_binary_header *
bpf_jit_binary_hdr(const struct bpf_prog *fp)
{
@@ -789,7 +779,6 @@ void __bpf_prog_free(struct bpf_prog *fp);
static inline void bpf_prog_unlock_free(struct bpf_prog *fp)
{
- bpf_prog_unlock_ro(fp);
__bpf_prog_free(fp);
}
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ff09d32a8a1b..c605397c79f0 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -848,7 +848,6 @@ void __weak bpf_jit_free(struct bpf_prog *fp)
if (fp->jited) {
struct bpf_binary_header *hdr = bpf_jit_binary_hdr(fp);
- bpf_jit_binary_unlock_ro(hdr);
bpf_jit_binary_free(hdr);
WARN_ON_ONCE(!bpf_prog_kallsyms_verify_off(fp));
--
2.17.1
^ permalink raw reply related
* [PATCH v4 16/23] vmalloc: Add flag for free of special permsissions
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
Add a new flag VM_FLUSH_RESET_PERMS, for enabling vfree operations to
immediately clear executable TLB entries before freeing pages, and handle
resetting permissions on the directmap. This flag is useful for any kind
of memory with elevated permissions, or where there can be related
permissions changes on the directmap. Today this is RO+X and RO memory.
Although this enables directly vfreeing non-writeable memory now,
non-writable memory cannot be freed in an interrupt because the allocation
itself is used as a node on deferred free list. So when RO memory needs to
be freed in an interrupt the code doing the vfree needs to have its own
work queue, as was the case before the deferred vfree list was added to
vmalloc.
For architectures with set_direct_map_ implementations this whole operation
can be done with one TLB flush when centralized like this. For others with
directmap permissions, currently only arm64, a backup method using
set_memory functions is used to reset the directmap. When arm64 adds
set_direct_map_ functions, this backup can be removed.
When the TLB is flushed to both remove TLB entries for the vmalloc range
mapping and the direct map permissions, the lazy purge operation could be
done to try to save a TLB flush later. However today vm_unmap_aliases
could flush a TLB range that does not include the directmap. So a helper
is added with extra parameters that can allow both the vmalloc address and
the direct mapping to be flushed during this operation. The behavior of the
normal vm_unmap_aliases function is unchanged.
Cc: Borislav Petkov <bp@alien8.de>
Suggested-by: Dave Hansen <dave.hansen@intel.com>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Suggested-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
include/linux/vmalloc.h | 15 ++++++
mm/vmalloc.c | 113 +++++++++++++++++++++++++++++++++-------
2 files changed, 109 insertions(+), 19 deletions(-)
diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 398e9c95cd61..c6eebb839552 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -21,6 +21,11 @@ struct notifier_block; /* in notifier.h */
#define VM_UNINITIALIZED 0x00000020 /* vm_struct is not fully initialized */
#define VM_NO_GUARD 0x00000040 /* don't add guard page */
#define VM_KASAN 0x00000080 /* has allocated kasan shadow memory */
+/*
+ * Memory with VM_FLUSH_RESET_PERMS cannot be freed in an interrupt or with
+ * vfree_atomic().
+ */
+#define VM_FLUSH_RESET_PERMS 0x00000100 /* Reset direct map and flush TLB on unmap */
/* bits [20..32] reserved for arch specific ioremap internals */
/*
@@ -142,6 +147,13 @@ extern int map_kernel_range_noflush(unsigned long start, unsigned long size,
pgprot_t prot, struct page **pages);
extern void unmap_kernel_range_noflush(unsigned long addr, unsigned long size);
extern void unmap_kernel_range(unsigned long addr, unsigned long size);
+static inline void set_vm_flush_reset_perms(void *addr)
+{
+ struct vm_struct *vm = find_vm_area(addr);
+
+ if (vm)
+ vm->flags |= VM_FLUSH_RESET_PERMS;
+}
#else
static inline int
map_kernel_range_noflush(unsigned long start, unsigned long size,
@@ -157,6 +169,9 @@ static inline void
unmap_kernel_range(unsigned long addr, unsigned long size)
{
}
+static inline void set_vm_flush_reset_perms(void *addr)
+{
+}
#endif
/* Allocate/destroy a 'vmalloc' VM area. */
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index e86ba6e74b50..e5e9e1fcac01 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -18,6 +18,7 @@
#include <linux/interrupt.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <linux/set_memory.h>
#include <linux/debugobjects.h>
#include <linux/kallsyms.h>
#include <linux/list.h>
@@ -1059,24 +1060,9 @@ static void vb_free(const void *addr, unsigned long size)
spin_unlock(&vb->lock);
}
-/**
- * vm_unmap_aliases - unmap outstanding lazy aliases in the vmap layer
- *
- * The vmap/vmalloc layer lazily flushes kernel virtual mappings primarily
- * to amortize TLB flushing overheads. What this means is that any page you
- * have now, may, in a former life, have been mapped into kernel virtual
- * address by the vmap layer and so there might be some CPUs with TLB entries
- * still referencing that page (additional to the regular 1:1 kernel mapping).
- *
- * vm_unmap_aliases flushes all such lazy mappings. After it returns, we can
- * be sure that none of the pages we have control over will have any aliases
- * from the vmap layer.
- */
-void vm_unmap_aliases(void)
+static void _vm_unmap_aliases(unsigned long start, unsigned long end, int flush)
{
- unsigned long start = ULONG_MAX, end = 0;
int cpu;
- int flush = 0;
if (unlikely(!vmap_initialized))
return;
@@ -1113,6 +1099,27 @@ void vm_unmap_aliases(void)
flush_tlb_kernel_range(start, end);
mutex_unlock(&vmap_purge_lock);
}
+
+/**
+ * vm_unmap_aliases - unmap outstanding lazy aliases in the vmap layer
+ *
+ * The vmap/vmalloc layer lazily flushes kernel virtual mappings primarily
+ * to amortize TLB flushing overheads. What this means is that any page you
+ * have now, may, in a former life, have been mapped into kernel virtual
+ * address by the vmap layer and so there might be some CPUs with TLB entries
+ * still referencing that page (additional to the regular 1:1 kernel mapping).
+ *
+ * vm_unmap_aliases flushes all such lazy mappings. After it returns, we can
+ * be sure that none of the pages we have control over will have any aliases
+ * from the vmap layer.
+ */
+void vm_unmap_aliases(void)
+{
+ unsigned long start = ULONG_MAX, end = 0;
+ int flush = 0;
+
+ _vm_unmap_aliases(start, end, flush);
+}
EXPORT_SYMBOL_GPL(vm_unmap_aliases);
/**
@@ -1505,6 +1512,72 @@ struct vm_struct *remove_vm_area(const void *addr)
return NULL;
}
+static inline void set_area_direct_map(const struct vm_struct *area,
+ int (*set_direct_map)(struct page *page))
+{
+ int i;
+
+ for (i = 0; i < area->nr_pages; i++)
+ if (page_address(area->pages[i]))
+ set_direct_map(area->pages[i]);
+}
+
+/* Handle removing and resetting vm mappings related to the vm_struct. */
+static void vm_remove_mappings(struct vm_struct *area, int deallocate_pages)
+{
+ unsigned long addr = (unsigned long)area->addr;
+ unsigned long start = ULONG_MAX, end = 0;
+ int flush_reset = area->flags & VM_FLUSH_RESET_PERMS;
+ int i;
+
+ /*
+ * The below block can be removed when all architectures that have
+ * direct map permissions also have set_direct_map_() implementations.
+ * This is concerned with resetting the direct map any an vm alias with
+ * execute permissions, without leaving a RW+X window.
+ */
+ if (flush_reset && !IS_ENABLED(CONFIG_ARCH_HAS_SET_DIRECT_MAP)) {
+ set_memory_nx(addr, area->nr_pages);
+ set_memory_rw(addr, area->nr_pages);
+ }
+
+ remove_vm_area(area->addr);
+
+ /* If this is not VM_FLUSH_RESET_PERMS memory, no need for the below. */
+ if (!flush_reset)
+ return;
+
+ /*
+ * If not deallocating pages, just do the flush of the VM area and
+ * return.
+ */
+ if (!deallocate_pages) {
+ vm_unmap_aliases();
+ return;
+ }
+
+ /*
+ * If execution gets here, flush the vm mapping and reset the direct
+ * map. Find the start and end range of the direct mappings to make sure
+ * the vm_unmap_aliases() flush includes the direct map.
+ */
+ for (i = 0; i < area->nr_pages; i++) {
+ if (page_address(area->pages[i])) {
+ start = min(addr, start);
+ end = max(addr, end);
+ }
+ }
+
+ /*
+ * Set direct map to something invalid so that it won't be cached if
+ * there are any accesses after the TLB flush, then flush the TLB and
+ * reset the direct map permissions to the default.
+ */
+ set_area_direct_map(area, set_direct_map_invalid_noflush);
+ _vm_unmap_aliases(start, end, 1);
+ set_area_direct_map(area, set_direct_map_default_noflush);
+}
+
static void __vunmap(const void *addr, int deallocate_pages)
{
struct vm_struct *area;
@@ -1526,7 +1599,8 @@ static void __vunmap(const void *addr, int deallocate_pages)
debug_check_no_locks_freed(area->addr, get_vm_area_size(area));
debug_check_no_obj_freed(area->addr, get_vm_area_size(area));
- remove_vm_area(addr);
+ vm_remove_mappings(area, deallocate_pages);
+
if (deallocate_pages) {
int i;
@@ -1961,8 +2035,9 @@ EXPORT_SYMBOL(vzalloc_node);
*/
void *vmalloc_exec(unsigned long size)
{
- return __vmalloc_node(size, 1, GFP_KERNEL, PAGE_KERNEL_EXEC,
- NUMA_NO_NODE, __builtin_return_address(0));
+ return __vmalloc_node_range(size, 1, VMALLOC_START, VMALLOC_END,
+ GFP_KERNEL, PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS,
+ NUMA_NO_NODE, __builtin_return_address(0));
}
#if defined(CONFIG_64BIT) && defined(CONFIG_ZONE_DMA32)
--
2.17.1
^ permalink raw reply related
* [PATCH v4 12/23] x86/jump-label: Remove support for custom poker
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Kees Cook, Dave Hansen,
Masami Hiramatsu, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
There are only two types of poking: early and breakpoint based. The use
of a function pointer to perform poking complicates the code and is
probably inefficient due to the use of indirect branches.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/jump_label.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index e7d8c636b228..e631c358f7f4 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -37,7 +37,6 @@ static void bug_at(unsigned char *ip, int line)
static void __ref __jump_label_transform(struct jump_entry *entry,
enum jump_label_type type,
- void *(*poker)(void *, const void *, size_t),
int init)
{
union jump_code_union jmp;
@@ -50,14 +49,6 @@ static void __ref __jump_label_transform(struct jump_entry *entry,
jmp.offset = jump_entry_target(entry) -
(jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
- /*
- * As long as only a single processor is running and the code is still
- * not marked as RO, text_poke_early() can be used; Checking that
- * system_state is SYSTEM_BOOTING guarantees it.
- */
- if (system_state == SYSTEM_BOOTING)
- poker = text_poke_early;
-
if (type == JUMP_LABEL_JMP) {
if (init) {
expect = default_nop; line = __LINE__;
@@ -80,16 +71,19 @@ static void __ref __jump_label_transform(struct jump_entry *entry,
bug_at((void *)jump_entry_code(entry), line);
/*
- * Make text_poke_bp() a default fallback poker.
+ * As long as only a single processor is running and the code is still
+ * not marked as RO, text_poke_early() can be used; Checking that
+ * system_state is SYSTEM_BOOTING guarantees it. It will be set to
+ * SYSTEM_SCHEDULING before other cores are awaken and before the
+ * code is write-protected.
*
* At the time the change is being done, just ignore whether we
* are doing nop -> jump or jump -> nop transition, and assume
* always nop being the 'currently valid' instruction
- *
*/
- if (poker) {
- (*poker)((void *)jump_entry_code(entry), code,
- JUMP_LABEL_NOP_SIZE);
+ if (init || system_state == SYSTEM_BOOTING) {
+ text_poke_early((void *)jump_entry_code(entry), code,
+ JUMP_LABEL_NOP_SIZE);
return;
}
@@ -101,7 +95,7 @@ void arch_jump_label_transform(struct jump_entry *entry,
enum jump_label_type type)
{
mutex_lock(&text_mutex);
- __jump_label_transform(entry, type, NULL, 0);
+ __jump_label_transform(entry, type, 0);
mutex_unlock(&text_mutex);
}
@@ -131,5 +125,5 @@ __init_or_module void arch_jump_label_transform_static(struct jump_entry *entry,
jlstate = JL_STATE_NO_UPDATE;
}
if (jlstate == JL_STATE_UPDATE)
- __jump_label_transform(entry, type, text_poke_early, 1);
+ __jump_label_transform(entry, type, 1);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 11/23] x86/module: Avoid breaking W^X while loading modules
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Kees Cook, Dave Hansen,
Masami Hiramatsu, Jessica Yu, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
When modules and BPF filters are loaded, there is a time window in
which some memory is both writable and executable. An attacker that has
already found another vulnerability (e.g., a dangling pointer) might be
able to exploit this behavior to overwrite kernel code. Prevent having
writable executable PTEs in this stage.
In addition, avoiding having W+X mappings can also slightly simplify the
patching of modules code on initialization (e.g., by alternatives and
static-key), as would be done in the next patch. This was actually the
main motivation for this patch.
To avoid having W+X mappings, set them initially as RW (NX) and after
they are set as RO set them as X as well. Setting them as executable is
done as a separate step to avoid one core in which the old PTE is cached
(hence writable), and another which sees the updated PTE (executable),
which would break the W^X protection.
Cc: Kees Cook <keescook@chromium.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/alternative.c | 28 +++++++++++++++++++++-------
arch/x86/kernel/module.c | 2 +-
include/linux/filter.h | 1 +
kernel/module.c | 5 +++++
4 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 599203876c32..3d2b6b6fb20c 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -668,15 +668,29 @@ void __init alternative_instructions(void)
* handlers seeing an inconsistent instruction while you patch.
*/
void *__init_or_module text_poke_early(void *addr, const void *opcode,
- size_t len)
+ size_t len)
{
unsigned long flags;
- local_irq_save(flags);
- memcpy(addr, opcode, len);
- local_irq_restore(flags);
- sync_core();
- /* Could also do a CLFLUSH here to speed up CPU recovery; but
- that causes hangs on some VIA CPUs. */
+
+ if (boot_cpu_has(X86_FEATURE_NX) &&
+ is_module_text_address((unsigned long)addr)) {
+ /*
+ * Modules text is marked initially as non-executable, so the
+ * code cannot be running and speculative code-fetches are
+ * prevented. Just change the code.
+ */
+ memcpy(addr, opcode, len);
+ } else {
+ local_irq_save(flags);
+ memcpy(addr, opcode, len);
+ local_irq_restore(flags);
+ sync_core();
+
+ /*
+ * Could also do a CLFLUSH here to speed up CPU recovery; but
+ * that causes hangs on some VIA CPUs.
+ */
+ }
return addr;
}
diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index b052e883dd8c..cfa3106faee4 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -87,7 +87,7 @@ void *module_alloc(unsigned long size)
p = __vmalloc_node_range(size, MODULE_ALIGN,
MODULES_VADDR + get_module_load_offset(),
MODULES_END, GFP_KERNEL,
- PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
+ PAGE_KERNEL, 0, NUMA_NO_NODE,
__builtin_return_address(0));
if (p && (kasan_module_alloc(p, size) < 0)) {
vfree(p);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 6074aa064b54..14ec3bdad9a9 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -746,6 +746,7 @@ static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
static inline void bpf_jit_binary_lock_ro(struct bpf_binary_header *hdr)
{
set_memory_ro((unsigned long)hdr, hdr->pages);
+ set_memory_x((unsigned long)hdr, hdr->pages);
}
static inline void bpf_jit_binary_unlock_ro(struct bpf_binary_header *hdr)
diff --git a/kernel/module.c b/kernel/module.c
index 0b9aa8ab89f0..2b2845ae983e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1950,8 +1950,13 @@ void module_enable_ro(const struct module *mod, bool after_init)
return;
frob_text(&mod->core_layout, set_memory_ro);
+ frob_text(&mod->core_layout, set_memory_x);
+
frob_rodata(&mod->core_layout, set_memory_ro);
+
frob_text(&mod->init_layout, set_memory_ro);
+ frob_text(&mod->init_layout, set_memory_x);
+
frob_rodata(&mod->init_layout, set_memory_ro);
if (after_init)
--
2.17.1
^ permalink raw reply related
* [PATCH v4 10/23] x86/kprobes: Set instruction page as executable
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
Set the page as executable after allocation. This patch is a
preparatory patch for a following patch that makes module allocated
pages non-executable.
While at it, do some small cleanup of what appears to be unnecessary
masking.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/kprobes/core.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index a034cb808e7e..1591852d3ac4 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -431,8 +431,20 @@ void *alloc_insn_page(void)
void *page;
page = module_alloc(PAGE_SIZE);
- if (page)
- set_memory_ro((unsigned long)page & PAGE_MASK, 1);
+ if (!page)
+ return NULL;
+
+ /*
+ * First make the page read-only, and only then make it executable to
+ * prevent it from being W+X in between.
+ */
+ set_memory_ro((unsigned long)page, 1);
+
+ /*
+ * TODO: Once additional kernel code protection mechanisms are set, ensure
+ * that the page was not maliciously altered and it is still zeroed.
+ */
+ set_memory_x((unsigned long)page, 1);
return page;
}
@@ -440,8 +452,12 @@ void *alloc_insn_page(void)
/* Recover page to RW mode before releasing it */
void free_insn_page(void *page)
{
- set_memory_nx((unsigned long)page & PAGE_MASK, 1);
- set_memory_rw((unsigned long)page & PAGE_MASK, 1);
+ /*
+ * First make the page non-executable, and only then make it writable to
+ * prevent it from being W+X in between.
+ */
+ set_memory_nx((unsigned long)page, 1);
+ set_memory_rw((unsigned long)page, 1);
module_memfree(page);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 13/23] x86/alternative: Remove the return value of text_poke_*()
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Kees Cook, Dave Hansen,
Masami Hiramatsu, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
The return value of text_poke_early() and text_poke_bp() is useless.
Remove it.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/text-patching.h | 4 ++--
arch/x86/kernel/alternative.c | 11 ++++-------
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index a75eed841eed..c90678fd391a 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -18,7 +18,7 @@ static inline void apply_paravirt(struct paravirt_patch_site *start,
#define __parainstructions_end NULL
#endif
-extern void *text_poke_early(void *addr, const void *opcode, size_t len);
+extern void text_poke_early(void *addr, const void *opcode, size_t len);
/*
* Clear and restore the kernel write-protection flag on the local CPU.
@@ -37,7 +37,7 @@ extern void *text_poke_early(void *addr, const void *opcode, size_t len);
extern void *text_poke(void *addr, const void *opcode, size_t len);
extern void *text_poke_kgdb(void *addr, const void *opcode, size_t len);
extern int poke_int3_handler(struct pt_regs *regs);
-extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
+extern void text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
extern int after_bootmem;
extern __ro_after_init struct mm_struct *poking_mm;
extern __ro_after_init unsigned long poking_addr;
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 3d2b6b6fb20c..18f959975ea0 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -265,7 +265,7 @@ static void __init_or_module add_nops(void *insns, unsigned int len)
extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
extern s32 __smp_locks[], __smp_locks_end[];
-void *text_poke_early(void *addr, const void *opcode, size_t len);
+void text_poke_early(void *addr, const void *opcode, size_t len);
/*
* Are we looking at a near JMP with a 1 or 4-byte displacement.
@@ -667,8 +667,8 @@ void __init alternative_instructions(void)
* instructions. And on the local CPU you need to be protected again NMI or MCE
* handlers seeing an inconsistent instruction while you patch.
*/
-void *__init_or_module text_poke_early(void *addr, const void *opcode,
- size_t len)
+void __init_or_module text_poke_early(void *addr, const void *opcode,
+ size_t len)
{
unsigned long flags;
@@ -691,7 +691,6 @@ void *__init_or_module text_poke_early(void *addr, const void *opcode,
* that causes hangs on some VIA CPUs.
*/
}
- return addr;
}
__ro_after_init struct mm_struct *poking_mm;
@@ -893,7 +892,7 @@ NOKPROBE_SYMBOL(poke_int3_handler);
* replacing opcode
* - sync cores
*/
-void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler)
+void text_poke_bp(void *addr, const void *opcode, size_t len, void *handler)
{
unsigned char int3 = 0xcc;
@@ -935,7 +934,5 @@ void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler)
* the writing of the new instruction.
*/
bp_patching_in_progress = false;
-
- return addr;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 09/23] x86/ftrace: Set trampoline pages as executable
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
Since alloc_module() will not set the pages as executable soon, set
ftrace trampoline pages as executable after they are allocated.
For the time being, do not change ftrace to use the text_poke()
interface. As a result, ftrace still breaks W^X.
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/ftrace.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index ef49517f6bb2..53ba1aa3a01f 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -730,6 +730,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
unsigned long end_offset;
unsigned long op_offset;
unsigned long offset;
+ unsigned long npages;
unsigned long size;
unsigned long retq;
unsigned long *ptr;
@@ -762,6 +763,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
return 0;
*tramp_size = size + RET_SIZE + sizeof(void *);
+ npages = DIV_ROUND_UP(*tramp_size, PAGE_SIZE);
/* Copy ftrace_caller onto the trampoline memory */
ret = probe_kernel_read(trampoline, (void *)start_offset, size);
@@ -806,6 +808,12 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
/* ALLOC_TRAMP flags lets us know we created it */
ops->flags |= FTRACE_OPS_FL_ALLOC_TRAMP;
+ /*
+ * Module allocation needs to be completed by making the page
+ * executable. The page is still writable, which is a security hazard,
+ * but anyhow ftrace breaks W^X completely.
+ */
+ set_memory_x((unsigned long)trampoline, npages);
return (unsigned long)trampoline;
fail:
tramp_free(trampoline, *tramp_size);
--
2.17.1
^ permalink raw reply related
* [PATCH v4 08/23] x86/kgdb: Avoid redundant comparison of patched code
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
text_poke() already ensures that the written value is the correct one
and fails if that is not the case. There is no need for an additional
comparison. Remove it.
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/kgdb.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index 2b203ee5b879..13b13311b792 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -747,7 +747,6 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long ip)
int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
{
int err;
- char opc[BREAK_INSTR_SIZE];
bpt->type = BP_BREAKPOINT;
err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr,
@@ -766,11 +765,6 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
return -EBUSY;
text_poke_kgdb((void *)bpt->bpt_addr, arch_kgdb_ops.gdb_bpt_instr,
BREAK_INSTR_SIZE);
- err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
- if (err)
- return err;
- if (memcmp(opc, arch_kgdb_ops.gdb_bpt_instr, BREAK_INSTR_SIZE))
- return -EINVAL;
bpt->type = BP_POKE_BREAKPOINT;
return err;
@@ -778,9 +772,6 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
{
- int err;
- char opc[BREAK_INSTR_SIZE];
-
if (bpt->type != BP_POKE_BREAKPOINT)
goto knl_write;
/*
@@ -791,10 +782,7 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
goto knl_write;
text_poke_kgdb((void *)bpt->bpt_addr, bpt->saved_instr,
BREAK_INSTR_SIZE);
- err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
- if (err || memcmp(opc, bpt->saved_instr, BREAK_INSTR_SIZE))
- goto knl_write;
- return err;
+ return 0;
knl_write:
return probe_kernel_write((char *)bpt->bpt_addr,
--
2.17.1
^ permalink raw reply related
* [PATCH v4 06/23] x86/alternative: Initialize temporary mm for patching
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Kees Cook, Dave Hansen,
Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
To prevent improper use of the PTEs that are used for text patching, the
next patches will use a temporary mm struct. Initailize it by copying
the init mm.
The address that will be used for patching is taken from the lower area
that is usually used for the task memory. Doing so prevents the need to
frequently synchronize the temporary-mm (e.g., when BPF programs are
installed), since different PGDs are used for the task memory.
Finally, randomize the address of the PTEs to harden against exploits
that use these PTEs.
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Masami Hiramatsu <mhiramat@kernel.org>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/pgtable.h | 3 +++
arch/x86/include/asm/text-patching.h | 2 ++
arch/x86/kernel/alternative.c | 3 +++
arch/x86/mm/init_64.c | 36 ++++++++++++++++++++++++++++
init/main.c | 3 +++
5 files changed, 47 insertions(+)
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 5cfbbb6d458d..6b6bfdfe83aa 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -1038,6 +1038,9 @@ static inline void __meminit init_trampoline_default(void)
/* Default trampoline pgd value */
trampoline_pgd_entry = init_top_pgt[pgd_index(__PAGE_OFFSET)];
}
+
+void __init poking_init(void);
+
# ifdef CONFIG_RANDOMIZE_MEMORY
void __meminit init_trampoline(void);
# else
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index f8fc8e86cf01..a75eed841eed 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -39,5 +39,7 @@ extern void *text_poke_kgdb(void *addr, const void *opcode, size_t len);
extern int poke_int3_handler(struct pt_regs *regs);
extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
extern int after_bootmem;
+extern __ro_after_init struct mm_struct *poking_mm;
+extern __ro_after_init unsigned long poking_addr;
#endif /* _ASM_X86_TEXT_PATCHING_H */
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 0a814d73547a..11d5c710a94f 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -679,6 +679,9 @@ void *__init_or_module text_poke_early(void *addr, const void *opcode,
return addr;
}
+__ro_after_init struct mm_struct *poking_mm;
+__ro_after_init unsigned long poking_addr;
+
static void *__text_poke(void *addr, const void *opcode, size_t len)
{
unsigned long flags;
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index bccff68e3267..125c8c48aa24 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -53,6 +53,7 @@
#include <asm/init.h>
#include <asm/uv/uv.h>
#include <asm/setup.h>
+#include <asm/text-patching.h>
#include "mm_internal.h"
@@ -1383,6 +1384,41 @@ unsigned long memory_block_size_bytes(void)
return memory_block_size_probed;
}
+/*
+ * Initialize an mm_struct to be used during poking and a pointer to be used
+ * during patching.
+ */
+void __init poking_init(void)
+{
+ spinlock_t *ptl;
+ pte_t *ptep;
+
+ poking_mm = copy_init_mm();
+ BUG_ON(!poking_mm);
+
+ /*
+ * Randomize the poking address, but make sure that the following page
+ * will be mapped at the same PMD. We need 2 pages, so find space for 3,
+ * and adjust the address if the PMD ends after the first one.
+ */
+ poking_addr = TASK_UNMAPPED_BASE;
+ if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
+ poking_addr += (kaslr_get_random_long("Poking") & PAGE_MASK) %
+ (TASK_SIZE - TASK_UNMAPPED_BASE - 3 * PAGE_SIZE);
+
+ if (((poking_addr + PAGE_SIZE) & ~PMD_MASK) == 0)
+ poking_addr += PAGE_SIZE;
+
+ /*
+ * We need to trigger the allocation of the page-tables that will be
+ * needed for poking now. Later, poking may be performed in an atomic
+ * section, which might cause allocation to fail.
+ */
+ ptep = get_locked_pte(poking_mm, poking_addr, &ptl);
+ BUG_ON(!ptep);
+ pte_unmap_unlock(ptep, ptl);
+}
+
#ifdef CONFIG_SPARSEMEM_VMEMMAP
/*
* Initialise the sparsemem vmemmap using huge-pages at the PMD level.
diff --git a/init/main.c b/init/main.c
index 598e278b46f7..949eed8015ec 100644
--- a/init/main.c
+++ b/init/main.c
@@ -504,6 +504,8 @@ void __init __weak thread_stack_cache_init(void)
void __init __weak mem_encrypt_init(void) { }
+void __init __weak poking_init(void) { }
+
bool initcall_debug;
core_param(initcall_debug, initcall_debug, bool, 0644);
@@ -737,6 +739,7 @@ asmlinkage __visible void __init start_kernel(void)
taskstats_init_early();
delayacct_init();
+ poking_init();
check_bugs();
acpi_subsystem_init();
--
2.17.1
^ permalink raw reply related
* [PATCH v4 07/23] x86/alternative: Use temporary mm for text poking
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Kees Cook, Dave Hansen,
Masami Hiramatsu, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
text_poke() can potentially compromise security as it sets temporary
PTEs in the fixmap. These PTEs might be used to rewrite the kernel code
from other cores accidentally or maliciously, if an attacker gains the
ability to write onto kernel memory.
Moreover, since remote TLBs are not flushed after the temporary PTEs are
removed, the time-window in which the code is writable is not limited if
the fixmap PTEs - maliciously or accidentally - are cached in the TLB.
To address these potential security hazards, use a temporary mm for
patching the code.
Finally, text_poke() is also not conservative enough when mapping pages,
as it always tries to map 2 pages, even when a single one is sufficient.
So try to be more conservative, and do not map more than needed.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/fixmap.h | 2 -
arch/x86/kernel/alternative.c | 108 +++++++++++++++++++++++++++-------
arch/x86/xen/mmu_pv.c | 2 -
3 files changed, 86 insertions(+), 26 deletions(-)
diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
index 50ba74a34a37..9da8cccdf3fb 100644
--- a/arch/x86/include/asm/fixmap.h
+++ b/arch/x86/include/asm/fixmap.h
@@ -103,8 +103,6 @@ enum fixed_addresses {
#ifdef CONFIG_PARAVIRT
FIX_PARAVIRT_BOOTMAP,
#endif
- FIX_TEXT_POKE1, /* reserve 2 pages for text_poke() */
- FIX_TEXT_POKE0, /* first page is last, because allocation is backward */
#ifdef CONFIG_X86_INTEL_MID
FIX_LNW_VRTC,
#endif
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 11d5c710a94f..599203876c32 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -12,6 +12,7 @@
#include <linux/slab.h>
#include <linux/kdebug.h>
#include <linux/kprobes.h>
+#include <linux/mmu_context.h>
#include <asm/text-patching.h>
#include <asm/alternative.h>
#include <asm/sections.h>
@@ -684,41 +685,104 @@ __ro_after_init unsigned long poking_addr;
static void *__text_poke(void *addr, const void *opcode, size_t len)
{
+ bool cross_page_boundary = offset_in_page(addr) + len > PAGE_SIZE;
+ struct page *pages[2] = {NULL};
+ temp_mm_state_t prev;
unsigned long flags;
- char *vaddr;
- struct page *pages[2];
- int i;
+ pte_t pte, *ptep;
+ spinlock_t *ptl;
+ pgprot_t pgprot;
/*
- * While boot memory allocator is runnig we cannot use struct
- * pages as they are not yet initialized.
+ * While boot memory allocator is running we cannot use struct pages as
+ * they are not yet initialized. There is no way to recover.
*/
BUG_ON(!after_bootmem);
if (!core_kernel_text((unsigned long)addr)) {
pages[0] = vmalloc_to_page(addr);
- pages[1] = vmalloc_to_page(addr + PAGE_SIZE);
+ if (cross_page_boundary)
+ pages[1] = vmalloc_to_page(addr + PAGE_SIZE);
} else {
pages[0] = virt_to_page(addr);
WARN_ON(!PageReserved(pages[0]));
- pages[1] = virt_to_page(addr + PAGE_SIZE);
+ if (cross_page_boundary)
+ pages[1] = virt_to_page(addr + PAGE_SIZE);
}
- BUG_ON(!pages[0]);
+ /*
+ * If something went wrong, crash and burn since recovery paths are not
+ * implemented.
+ */
+ BUG_ON(!pages[0] || (cross_page_boundary && !pages[1]));
+
local_irq_save(flags);
- set_fixmap(FIX_TEXT_POKE0, page_to_phys(pages[0]));
- if (pages[1])
- set_fixmap(FIX_TEXT_POKE1, page_to_phys(pages[1]));
- vaddr = (char *)fix_to_virt(FIX_TEXT_POKE0);
- memcpy(&vaddr[(unsigned long)addr & ~PAGE_MASK], opcode, len);
- clear_fixmap(FIX_TEXT_POKE0);
- if (pages[1])
- clear_fixmap(FIX_TEXT_POKE1);
- local_flush_tlb();
- sync_core();
- /* Could also do a CLFLUSH here to speed up CPU recovery; but
- that causes hangs on some VIA CPUs. */
- for (i = 0; i < len; i++)
- BUG_ON(((char *)addr)[i] != ((char *)opcode)[i]);
+
+ /*
+ * Map the page without the global bit, as TLB flushing is done with
+ * flush_tlb_mm_range(), which is intended for non-global PTEs.
+ */
+ pgprot = __pgprot(pgprot_val(PAGE_KERNEL) & ~_PAGE_GLOBAL);
+
+ /*
+ * The lock is not really needed, but this allows to avoid open-coding.
+ */
+ ptep = get_locked_pte(poking_mm, poking_addr, &ptl);
+
+ /*
+ * This must not fail; preallocated in poking_init().
+ */
+ VM_BUG_ON(!ptep);
+
+ pte = mk_pte(pages[0], pgprot);
+ set_pte_at(poking_mm, poking_addr, ptep, pte);
+
+ if (cross_page_boundary) {
+ pte = mk_pte(pages[1], pgprot);
+ set_pte_at(poking_mm, poking_addr + PAGE_SIZE, ptep + 1, pte);
+ }
+
+ /*
+ * Loading the temporary mm behaves as a compiler barrier, which
+ * guarantees that the PTE will be set at the time memcpy() is done.
+ */
+ prev = use_temporary_mm(poking_mm);
+
+ kasan_disable_current();
+ memcpy((u8 *)poking_addr + offset_in_page(addr), opcode, len);
+ kasan_enable_current();
+
+ /*
+ * Ensure that the PTE is only cleared after the instructions of memcpy
+ * were issued by using a compiler barrier.
+ */
+ barrier();
+
+ pte_clear(poking_mm, poking_addr, ptep);
+ if (cross_page_boundary)
+ pte_clear(poking_mm, poking_addr + PAGE_SIZE, ptep + 1);
+
+ /*
+ * Loading the previous page-table hierarchy requires a serializing
+ * instruction that already allows the core to see the updated version.
+ * Xen-PV is assumed to serialize execution in a similar manner.
+ */
+ unuse_temporary_mm(prev);
+
+ /*
+ * Flushing the TLB might involve IPIs, which would require enabled
+ * IRQs, but not if the mm is not used, as it is in this point.
+ */
+ flush_tlb_mm_range(poking_mm, poking_addr, poking_addr +
+ (cross_page_boundary ? 2 : 1) * PAGE_SIZE,
+ PAGE_SHIFT, false);
+
+ /*
+ * If the text does not match what we just wrote then something is
+ * fundamentally screwy; there's nothing we can really do about that.
+ */
+ BUG_ON(memcmp(addr, opcode, len));
+
+ pte_unmap_unlock(ptep, ptl);
local_irq_restore(flags);
return addr;
}
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index a21e1734fc1f..beb44e22afdf 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2318,8 +2318,6 @@ static void xen_set_fixmap(unsigned idx, phys_addr_t phys, pgprot_t prot)
#elif defined(CONFIG_X86_VSYSCALL_EMULATION)
case VSYSCALL_PAGE:
#endif
- case FIX_TEXT_POKE0:
- case FIX_TEXT_POKE1:
/* All local page mappings */
pte = pfn_pte(phys, prot);
break;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 05/23] fork: Provide a function for copying init_mm
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Kees Cook, Dave Hansen,
Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
Provide a function for copying init_mm. This function will be later used
for setting a temporary mm.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
include/linux/sched/task.h | 1 +
kernel/fork.c | 24 ++++++++++++++++++------
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 2e97a2227045..f1227f2c38a4 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -76,6 +76,7 @@ extern void exit_itimers(struct signal_struct *);
extern long _do_fork(unsigned long, unsigned long, unsigned long, int __user *, int __user *, unsigned long);
extern long do_fork(unsigned long, unsigned long, unsigned long, int __user *, int __user *);
struct task_struct *fork_idle(int);
+struct mm_struct *copy_init_mm(void);
extern pid_t kernel_thread(int (*fn)(void *), void *arg, unsigned long flags);
extern long kernel_wait4(pid_t, int __user *, int, struct rusage *);
diff --git a/kernel/fork.c b/kernel/fork.c
index 9dcd18aa210b..099cca8f701c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1298,13 +1298,20 @@ void mm_release(struct task_struct *tsk, struct mm_struct *mm)
complete_vfork_done(tsk);
}
-/*
- * Allocate a new mm structure and copy contents from the
- * mm structure of the passed in task structure.
+/**
+ * dup_mm() - duplicates an existing mm structure
+ * @tsk: the task_struct with which the new mm will be associated.
+ * @oldmm: the mm to duplicate.
+ *
+ * Allocates a new mm structure and duplicates the provided @oldmm structure
+ * content into it.
+ *
+ * Return: the duplicated mm or NULL on failure.
*/
-static struct mm_struct *dup_mm(struct task_struct *tsk)
+static struct mm_struct *dup_mm(struct task_struct *tsk,
+ struct mm_struct *oldmm)
{
- struct mm_struct *mm, *oldmm = current->mm;
+ struct mm_struct *mm;
int err;
mm = allocate_mm();
@@ -1371,7 +1378,7 @@ static int copy_mm(unsigned long clone_flags, struct task_struct *tsk)
}
retval = -ENOMEM;
- mm = dup_mm(tsk);
+ mm = dup_mm(tsk, current->mm);
if (!mm)
goto fail_nomem;
@@ -2186,6 +2193,11 @@ struct task_struct *fork_idle(int cpu)
return task;
}
+struct mm_struct *copy_init_mm(void)
+{
+ return dup_mm(NULL, &init_mm);
+}
+
/*
* Ok, this is the main fork-routine.
*
--
2.17.1
^ permalink raw reply related
* [PATCH v4 04/23] x86/mm: Save DRs when loading a temporary mm
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
Prevent user watchpoints from mistakenly firing while the temporary mm
is being used. As the addresses that of the temporary mm might overlap
those of the user-process, this is necessary to prevent wrong signals
or worse things from happening.
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/mmu_context.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index d684b954f3c0..81861862038a 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -13,6 +13,7 @@
#include <asm/tlbflush.h>
#include <asm/paravirt.h>
#include <asm/mpx.h>
+#include <asm/debugreg.h>
extern atomic64_t last_mm_ctx_id;
@@ -380,6 +381,21 @@ static inline temp_mm_state_t use_temporary_mm(struct mm_struct *mm)
lockdep_assert_irqs_disabled();
state.prev = this_cpu_read(cpu_tlbstate.loaded_mm);
switch_mm_irqs_off(NULL, mm, current);
+
+ /*
+ * If breakpoints are enabled, disable them while the temporary mm is
+ * used. Userspace might set up watchpoints on addresses that are used
+ * in the temporary mm, which would lead to wrong signals being sent or
+ * crashes.
+ *
+ * Note that breakpoints are not disabled selectively, which also causes
+ * kernel breakpoints (e.g., perf's) to be disabled. This might be
+ * undesirable, but still seems reasonable as the code that runs in the
+ * temporary mm should be short.
+ */
+ if (hw_breakpoint_active())
+ hw_breakpoint_disable();
+
return state;
}
@@ -387,6 +403,13 @@ static inline void unuse_temporary_mm(temp_mm_state_t prev)
{
lockdep_assert_irqs_disabled();
switch_mm_irqs_off(NULL, prev.prev, current);
+
+ /*
+ * Restore the breakpoints if they were disabled before the temporary mm
+ * was loaded.
+ */
+ if (hw_breakpoint_active())
+ hw_breakpoint_restore();
}
#endif /* _ASM_X86_MMU_CONTEXT_H */
--
2.17.1
^ permalink raw reply related
* [PATCH v4 01/23] Fix "x86/alternatives: Lockdep-enforce text_mutex in text_poke*()"
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Kees Cook, Dave Hansen,
Masami Hiramatsu, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
text_mutex is currently expected to be held before text_poke() is
called, but kgdb does not take the mutex, and instead *supposedly*
ensures the lock is not taken and will not be acquired by any other core
while text_poke() is running.
The reason for the "supposedly" comment is that it is not entirely clear
that this would be the case if gdb_do_roundup is zero.
Create two wrapper functions, text_poke() and text_poke_kgdb(), which do
or do not run the lockdep assertion respectively.
While we are at it, change the return code of text_poke() to something
meaningful. One day, callers might actually respect it and the existing
BUG_ON() when patching fails could be removed. For kgdb, the return
value can actually be used.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Fixes: 9222f606506c ("x86/alternatives: Lockdep-enforce text_mutex in text_poke*()")
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Acked-by: Jiri Kosina <jkosina@suse.cz>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/text-patching.h | 1 +
arch/x86/kernel/alternative.c | 52 ++++++++++++++++++++--------
arch/x86/kernel/kgdb.c | 11 +++---
3 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index e85ff65c43c3..f8fc8e86cf01 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -35,6 +35,7 @@ extern void *text_poke_early(void *addr, const void *opcode, size_t len);
* inconsistent instruction while you patch.
*/
extern void *text_poke(void *addr, const void *opcode, size_t len);
+extern void *text_poke_kgdb(void *addr, const void *opcode, size_t len);
extern int poke_int3_handler(struct pt_regs *regs);
extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
extern int after_bootmem;
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 9a79c7808f9c..0a814d73547a 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -679,18 +679,7 @@ void *__init_or_module text_poke_early(void *addr, const void *opcode,
return addr;
}
-/**
- * text_poke - Update instructions on a live kernel
- * @addr: address to modify
- * @opcode: source of the copy
- * @len: length to copy
- *
- * Only atomic text poke/set should be allowed when not doing early patching.
- * It means the size must be writable atomically and the address must be aligned
- * in a way that permits an atomic write. It also makes sure we fit on a single
- * page.
- */
-void *text_poke(void *addr, const void *opcode, size_t len)
+static void *__text_poke(void *addr, const void *opcode, size_t len)
{
unsigned long flags;
char *vaddr;
@@ -703,8 +692,6 @@ void *text_poke(void *addr, const void *opcode, size_t len)
*/
BUG_ON(!after_bootmem);
- lockdep_assert_held(&text_mutex);
-
if (!core_kernel_text((unsigned long)addr)) {
pages[0] = vmalloc_to_page(addr);
pages[1] = vmalloc_to_page(addr + PAGE_SIZE);
@@ -733,6 +720,43 @@ void *text_poke(void *addr, const void *opcode, size_t len)
return addr;
}
+/**
+ * text_poke - Update instructions on a live kernel
+ * @addr: address to modify
+ * @opcode: source of the copy
+ * @len: length to copy
+ *
+ * Only atomic text poke/set should be allowed when not doing early patching.
+ * It means the size must be writable atomically and the address must be aligned
+ * in a way that permits an atomic write. It also makes sure we fit on a single
+ * page.
+ */
+void *text_poke(void *addr, const void *opcode, size_t len)
+{
+ lockdep_assert_held(&text_mutex);
+
+ return __text_poke(addr, opcode, len);
+}
+
+/**
+ * text_poke_kgdb - Update instructions on a live kernel by kgdb
+ * @addr: address to modify
+ * @opcode: source of the copy
+ * @len: length to copy
+ *
+ * Only atomic text poke/set should be allowed when not doing early patching.
+ * It means the size must be writable atomically and the address must be aligned
+ * in a way that permits an atomic write. It also makes sure we fit on a single
+ * page.
+ *
+ * Context: should only be used by kgdb, which ensures no other core is running,
+ * despite the fact it does not hold the text_mutex.
+ */
+void *text_poke_kgdb(void *addr, const void *opcode, size_t len)
+{
+ return __text_poke(addr, opcode, len);
+}
+
static void do_sync_core(void *info)
{
sync_core();
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index 4ff6b4cdb941..2b203ee5b879 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -759,13 +759,13 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
if (!err)
return err;
/*
- * It is safe to call text_poke() because normal kernel execution
+ * It is safe to call text_poke_kgdb() because normal kernel execution
* is stopped on all cores, so long as the text_mutex is not locked.
*/
if (mutex_is_locked(&text_mutex))
return -EBUSY;
- text_poke((void *)bpt->bpt_addr, arch_kgdb_ops.gdb_bpt_instr,
- BREAK_INSTR_SIZE);
+ text_poke_kgdb((void *)bpt->bpt_addr, arch_kgdb_ops.gdb_bpt_instr,
+ BREAK_INSTR_SIZE);
err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
if (err)
return err;
@@ -784,12 +784,13 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
if (bpt->type != BP_POKE_BREAKPOINT)
goto knl_write;
/*
- * It is safe to call text_poke() because normal kernel execution
+ * It is safe to call text_poke_kgdb() because normal kernel execution
* is stopped on all cores, so long as the text_mutex is not locked.
*/
if (mutex_is_locked(&text_mutex))
goto knl_write;
- text_poke((void *)bpt->bpt_addr, bpt->saved_instr, BREAK_INSTR_SIZE);
+ text_poke_kgdb((void *)bpt->bpt_addr, bpt->saved_instr,
+ BREAK_INSTR_SIZE);
err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
if (err || memcmp(opc, bpt->saved_instr, BREAK_INSTR_SIZE))
goto knl_write;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 02/23] x86/jump_label: Use text_poke_early() during early init
From: Rick Edgecombe @ 2019-04-22 18:57 UTC (permalink / raw)
To: Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
Peter Zijlstra, linux_dti, linux-integrity, linux-security-module,
akpm, kernel-hardening, linux-mm, will.deacon, ard.biesheuvel,
kristen, deneen.t.dock, Nadav Amit, Kees Cook, Dave Hansen,
Masami Hiramatsu, Rick Edgecombe
In-Reply-To: <20190422185805.1169-1-rick.p.edgecombe@intel.com>
From: Nadav Amit <namit@vmware.com>
There is no apparent reason not to use text_poke_early() during
early-init, since no patching of code that might be on the stack is done
and only a single core is running.
This is required for the next patches that would set a temporary mm for
text poking, and this mm is only initialized after some static-keys are
enabled/disabled.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/jump_label.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index f99bd26bd3f1..e7d8c636b228 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -50,7 +50,12 @@ static void __ref __jump_label_transform(struct jump_entry *entry,
jmp.offset = jump_entry_target(entry) -
(jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
- if (early_boot_irqs_disabled)
+ /*
+ * As long as only a single processor is running and the code is still
+ * not marked as RO, text_poke_early() can be used; Checking that
+ * system_state is SYSTEM_BOOTING guarantees it.
+ */
+ if (system_state == SYSTEM_BOOTING)
poker = text_poke_early;
if (type == JUMP_LABEL_JMP) {
--
2.17.1
^ permalink raw reply related
* Re: [RFC PATCH v9 03/13] mm: Add support for eXclusive Page Frame Ownership (XPFO)
From: Khalid Aziz @ 2019-04-22 19:30 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski, Linus Torvalds
Cc: Thomas Gleixner, Nadav Amit, Ingo Molnar, Juerg Haefliger,
Tycho Andersen, Julian Stecklina, Konrad Rzeszutek Wilk,
Juerg Haefliger, deepa.srinivasan, chris hyser, Tyler Hicks,
David Woodhouse, Andrew Cooper, Jon Masters, Boris Ostrovsky,
iommu, X86 ML, linux-alpha@vger.kernel.org,
open list:DOCUMENTATION, Linux List Kernel Mailing, Linux-MM,
LSM List, Khalid Aziz, Andrew Morton, Peter Zijlstra, Dave Hansen,
Borislav Petkov, H. Peter Anvin, Arjan van de Ven,
Greg Kroah-Hartman
In-Reply-To: <8f9d059d-e720-cd24-faa6-45493fc012e0@oracle.com>
On 4/18/19 8:34 AM, Khalid Aziz wrote:
> On 4/17/19 11:41 PM, Kees Cook wrote:
>> On Wed, Apr 17, 2019 at 11:41 PM Andy Lutomirski <luto@kernel.org> wrote:
>>> I don't think this type of NX goof was ever the argument for XPFO.
>>> The main argument I've heard is that a malicious user program writes a
>>> ROP payload into user memory (regular anonymous user memory) and then
>>> gets the kernel to erroneously set RSP (*not* RIP) to point there.
>>
>> Well, more than just ROP. Any of the various attack primitives. The NX
>> stuff is about moving RIP: SMEP-bypassing. But there is still basic
>> SMAP-bypassing for putting a malicious structure in userspace and
>> having the kernel access it via the linear mapping, etc.
>>
>>> I find this argument fairly weak for a couple reasons. First, if
>>> we're worried about this, let's do in-kernel CFI, not XPFO, to
>>
>> CFI is getting much closer. Getting the kernel happy under Clang, LTO,
>> and CFI is under active development. (It's functional for arm64
>> already, and pieces have been getting upstreamed.)
>>
>
> CFI theoretically offers protection with fairly low overhead. I have not
> played much with CFI in clang. I agree with Linus that probability of
> bugs in XPFO implementation itself is a cause of concern. If CFI in
> Clang can provide us the same level of protection as XPFO does, I
> wouldn't want to push for an expensive change like XPFO.
>
> If Clang/CFI can't get us there for extended period of time, does it
> make sense to continue to poke at XPFO?
Any feedback on continued effort on XPFO? If it makes sense to have XPFO
available as a solution for ret2dir issue in case Clang/CFI does not
work out, I will continue to refine it.
--
Khalid
^ permalink raw reply
* Re: kernel BUG at kernel/cred.c:434!
From: Paul Moore @ 2019-04-22 19:48 UTC (permalink / raw)
To: Yang Yingliang
Cc: Casey Schaufler, Oleg Nesterov, john.johansen, chengjian (D),
Kees Cook, NeilBrown, Anna Schumaker,
linux-kernel@vger.kernel.org, Al Viro, Xiexiuqi (Xie XiuQi),
Li Bin, Jason Yan, Peter Zijlstra, Ingo Molnar,
Linux Security Module list, SELinux
In-Reply-To: <5CBACC8F.8010409@huawei.com>
On Sat, Apr 20, 2019 at 3:39 AM Yang Yingliang <yangyingliang@huawei.com> wrote:
> I'm not sure you got my point.
I went back and looked at your previous emails again to try and
understand what you are talking about, and I'm a little confused by
some of the output ...
> --- a/kernel/acct.c
> +++ b/kernel/acct.c
> @@ -481,6 +481,7 @@ static void do_acct_process(struct bsd_acct_struct
> *acct)
> flim = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
> current->signal->rlim[RLIMIT_FSIZE].rlim_cur = RLIM_INFINITY;
> /* Perform file operations on behalf of whoever enabled
> accounting */
> + pr_info("task:%px new cred:%px real cred:%px cred:%px\n",
> current, file->f_cred, current->real_cred, current->cred);
> orig_cred = override_creds(file->f_cred);
Okay, with this patch applied we should the task/cred info when
do_acct_process is called. Got it.
> Messages:
> [ 56.643298] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real
> cred:ffff88841ae450c0 cred:ffff88841ae450c0 //They are same.
Okay, it looks like do_acct_process() was called and f_cred,
real_cred, and cred are all the same.
> [ 56.646609] Process accounting resumed
It looks like do_acct_process() has called check_free_space() now. So
far so good.
> [ 56.649943] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real
> cred:ffff88841c96c300 cred:ffff88841ae450c0
Wait a minute ... why are we seeing this again? Looking at the task
pointer and the timestamp, this is the same task exiting and trying to
write to the accounting file, yes? This output is particularly
curious since it appears that real_cred has changed; where is this
happening?
> [ 56.653565] ------------[ cut here ]------------
> [ 56.655119] kernel BUG at kernel/cred.c:434!
> [ 56.656590] invalid opcode: 0000 [#1] SMP PTI
> [ 56.658033] CPU: 2 PID: 4169 Comm: syz-executor.15 Not tainted
> 5.1.0-rc4-00034-g869e3305f23d-dirty #143
> [ 56.661077] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> [ 56.664895] RIP: 0010:commit_creds+0x1eb/0x230
> [ 56.666344] Code: 43 1c 0f 85 08 ff ff ff e9 10 ff ff ff 8b 45 10 39
> 43 10 0f 85 18 ff ff ff 8b 43 20 39 45 20 0f 85 0c ff ff ff e9 14 ff ff
> ff <0f> 0b 48 c7 c7 d0 d2 49 82 e8 17 3b 3e 00 0f 0b 48 c7 c7 c0 d2 49
> [ 56.672410] RSP: 0018:ffffc90003a17b20 EFLAGS: 00010287
> [ 56.674098] RAX: ffff88841a9595c0 RBX: ffff88841ae450c0 RCX:
> 0000000000000000
> [ 56.676410] RDX: 0000000000000001 RSI: 0000000000000020 RDI:
> ffff88841c96ce40
> [ 56.678691] RBP: 0000000000000001 R08: 0000000000800000 R09:
> 0000000000000000
> [ 56.680997] R10: ffff88841c9265a0 R11: ffffffff810d6940 R12:
> ffff88841a9595c0
> [ 56.681198] task:ffff88841a9195c0 new cred:ffff88841aeaa0c0 real
> cred:ffff88841aeaa0c0 cred:ffff88841aeaa0c0
> [ 56.683293] R13: 0000000000000040 R14: ffff88841c96ce40 R15:
> 0000000000000040
> [ 56.683296] FS: 00007f5969a5c700(0000) GS:ffff88842fa80000(0000)
> knlGS:0000000000000000
> [ 56.683297] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 56.683299] CR2: 00007f82742214f0 CR3: 000000041cbc0005 CR4:
> 00000000000206e0
> [ 56.683305] Call Trace:
> [ 56.683340] selinux_setprocattr+0x17b/0x480
> [ 56.686513] Process accounting resumed
> [ 56.688849] proc_pid_attr_write+0xc0/0xf0
> [ 56.688857] __kernel_write+0x4f/0xf0
> [ 56.688866] do_acct_process+0x538/0x750
> [ 56.703090] ? __schedule+0x290/0x960
> [ 56.704311] ? __queue_work+0x160/0x5c0
> [ 56.705571] acct_pin_kill+0x1e/0x70
> [ 56.706743] pin_kill+0x81/0x150
> [ 56.707813] ? finish_wait+0x80/0x80
> [ 56.708985] mnt_pin_kill+0x1e/0x30
> [ 56.710127] cleanup_mnt+0x6e/0x70
> [ 56.711247] task_work_run+0x8a/0xb0
> [ 56.712453] do_exit+0x2e0/0xc80
> [ 56.713525] do_group_exit+0x33/0xb0
> [ 56.714701] get_signal+0x143/0x810
> [ 56.715865] do_signal+0x36/0x610
> [ 56.716962] ? __x64_sys_futex+0x134/0x180
> [ 56.718307] ? _copy_to_user+0x22/0x30
> [ 56.719606] exit_to_usermode_loop+0x80/0xe0
> [ 56.721003] do_syscall_64+0x16c/0x180
> [ 56.722242] entry_SYSCALL_64_after_hwframe+0x44/0xa9
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH 69/90] LSM: Use full security context in security_inode_setsecctx
From: Casey Schaufler @ 2019-04-22 20:45 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: casey, jmorris, linux-security-module, selinux
In-Reply-To: <04b749f6-097e-4faf-dc87-7892db728e3b@I-love.SAKURA.ne.jp>
On 4/22/2019 6:13 AM, Tetsuo Handa wrote:
> On 2019/04/19 9:45, Casey Schaufler wrote:
>> + hlist_for_each_entry(hp, &security_hook_heads.inode_setsecctx, list) {
>> + if (strncmp(ctx, hp->lsm, strlen(hp->lsm))) {
>> + WARN_ONCE(1, "security_inode_setsecctx form1 error\n");
>> + rc = -EINVAL;
>> + break;
>> + }
> Will you avoid using WARN*() ?
> Since syzbot tests using panic_on_warn == 1, this WARN_ONCE() will act as panic().
If syzbot hits any of the WARN_ONCE()s in security_inode_setsecctx()
I want it to panic and generate a report. A badly formatted inode secctx
would indicate that kernfs isn't getting the string from
security_inode_getsecctx() or that it is getting corrupted somehow. In
either case, it would be a bug that needs fixing. I used WARN instead of
BUG for the kernfs people, who might break something by accident.
If there's a strong objection to WARN_ONCE() in general, I can pull it.
^ permalink raw reply
* Re: [PATCH 69/90] LSM: Use full security context in security_inode_setsecctx
From: Tetsuo Handa @ 2019-04-22 21:01 UTC (permalink / raw)
To: Casey Schaufler; +Cc: jmorris, linux-security-module, selinux
In-Reply-To: <57c13f30-2f24-cdc7-52a0-27d791ebe315@schaufler-ca.com>
On 2019/04/23 5:45, Casey Schaufler wrote:
> On 4/22/2019 6:13 AM, Tetsuo Handa wrote:
>> On 2019/04/19 9:45, Casey Schaufler wrote:
>>> + hlist_for_each_entry(hp, &security_hook_heads.inode_setsecctx, list) {
>>> + if (strncmp(ctx, hp->lsm, strlen(hp->lsm))) {
>>> + WARN_ONCE(1, "security_inode_setsecctx form1 error\n");
>>> + rc = -EINVAL;
>>> + break;
>>> + }
>> Will you avoid using WARN*() ?
>> Since syzbot tests using panic_on_warn == 1, this WARN_ONCE() will act as panic().
>
> If syzbot hits any of the WARN_ONCE()s in security_inode_setsecctx()
> I want it to panic and generate a report. A badly formatted inode secctx
> would indicate that kernfs isn't getting the string from
> security_inode_getsecctx() or that it is getting corrupted somehow. In
> either case, it would be a bug that needs fixing. I used WARN instead of
> BUG for the kernfs people, who might break something by accident.
Since the code continues with -EINVAL error, I assumed that this is not
a bad situation. But if this can't be triggered by invalid input from
userspace, BUG() is better.
>
> If there's a strong objection to WARN_ONCE() in general, I can pull it.
>
^ permalink raw reply
* Re: [RFC PATCH v9 03/13] mm: Add support for eXclusive Page Frame Ownership (XPFO)
From: Kees Cook @ 2019-04-22 22:23 UTC (permalink / raw)
To: Khalid Aziz
Cc: Andy Lutomirski, Linus Torvalds, Thomas Gleixner, Nadav Amit,
Ingo Molnar, Juerg Haefliger, Tycho Andersen, Julian Stecklina,
Konrad Rzeszutek Wilk, Juerg Haefliger, deepa.srinivasan,
chris hyser, Tyler Hicks, David Woodhouse, Andrew Cooper,
Jon Masters, Boris Ostrovsky, iommu, X86 ML,
linux-alpha@vger.kernel.org, open list:DOCUMENTATION,
Linux List Kernel Mailing, Linux-MM, LSM List, Khalid Aziz,
Andrew Morton, Peter Zijlstra, Dave Hansen, Borislav Petkov,
H. Peter Anvin, Arjan van de Ven, Greg Kroah-Hartman
In-Reply-To: <8f9d059d-e720-cd24-faa6-45493fc012e0@oracle.com>
On Thu, Apr 18, 2019 at 7:35 AM Khalid Aziz <khalid.aziz@oracle.com> wrote:
>
> On 4/17/19 11:41 PM, Kees Cook wrote:
> > On Wed, Apr 17, 2019 at 11:41 PM Andy Lutomirski <luto@kernel.org> wrote:
> >> I don't think this type of NX goof was ever the argument for XPFO.
> >> The main argument I've heard is that a malicious user program writes a
> >> ROP payload into user memory (regular anonymous user memory) and then
> >> gets the kernel to erroneously set RSP (*not* RIP) to point there.
> >
> > Well, more than just ROP. Any of the various attack primitives. The NX
> > stuff is about moving RIP: SMEP-bypassing. But there is still basic
> > SMAP-bypassing for putting a malicious structure in userspace and
> > having the kernel access it via the linear mapping, etc.
> >
> >> I find this argument fairly weak for a couple reasons. First, if
> >> we're worried about this, let's do in-kernel CFI, not XPFO, to
> >
> > CFI is getting much closer. Getting the kernel happy under Clang, LTO,
> > and CFI is under active development. (It's functional for arm64
> > already, and pieces have been getting upstreamed.)
> >
>
> CFI theoretically offers protection with fairly low overhead. I have not
> played much with CFI in clang. I agree with Linus that probability of
> bugs in XPFO implementation itself is a cause of concern. If CFI in
> Clang can provide us the same level of protection as XPFO does, I
> wouldn't want to push for an expensive change like XPFO.
>
> If Clang/CFI can't get us there for extended period of time, does it
> make sense to continue to poke at XPFO?
Well, I think CFI will certainly vastly narrow the execution paths
available to an attacker, but what I continue to see XPFO useful for
is stopping attacks that need to locate something in memory. (i.e. not
ret2dir but, like, read2dir.) It's arguable that such attacks would
just use heap, stack, etc to hold such things, but the linear map
remains relatively easy to find/target. But I agree: the protection is
getting more and more narrow (especially with CFI coming down the
pipe), and if it's still a 28% hit, that's not going to be tenable for
anyone but the truly paranoid. :)
All that said, there isn't a very good backward-edge CFI protection
(i.e. ROP defense) on x86 in Clang. The forward-edge looks decent, but
requires LTO, etc. My point is there is still a long path to gaining
CFI in upstream.
--
Kees Cook
^ permalink raw reply
* Re: [PATCH (resend)] tomoyo: Add a kernel config option for fuzzing testing.
From: James Morris @ 2019-04-22 22:52 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: linux-security-module
In-Reply-To: <49c5ae17-86fb-e4d3-0e06-c205cf324c1d@I-love.SAKURA.ne.jp>
On Mon, 22 Apr 2019, Tetsuo Handa wrote:
> James, will you apply this patch and
> "[PATCH 3/3] tomoyo: Check address length before reading address family" and
> "[PATCH] tomoyo: Change pathname calculation for read-only filesystems." ?
On the 2nd one, did we see any feedback from Al?
>
> On 2019/04/12 20:04, 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(-)
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH] kexec_buffer measure
From: Mimi Zohar @ 2019-04-23 0:18 UTC (permalink / raw)
To: prakhar srivastava; +Cc: linux-integrity, linux-kernel, linux-security-module
In-Reply-To: <CAEFn8qKkXgxUKtribbtFwvG9NykGQo10jQ5Du_i9wJz-wKreOA@mail.gmail.com>
[Cc'ing LSM mailing list]
On Fri, 2019-04-19 at 17:30 -0700, prakhar srivastava wrote:
> 2) Adding a LSM hook
> We are doing both the command line and kernel version measurement in IMA.
> Can you please elaborate on how this can be used outside of the scenario?
> That will help me come back with a better design and code. I am
> neutral about this.
As I said previously, initially you might want to only measure the
kexec boot command line, but will you ever want to verify or audit log
the boot command line hash? Perhaps LSMs would be interested in the
boot command line. Should this be an LSM hook?
Mimi
^ permalink raw reply
* Re: kernel BUG at kernel/cred.c:434!
From: Yang Yingliang @ 2019-04-23 4:08 UTC (permalink / raw)
To: Paul Moore
Cc: Casey Schaufler, Oleg Nesterov, john.johansen, chengjian (D),
Kees Cook, NeilBrown, Anna Schumaker,
linux-kernel@vger.kernel.org, Al Viro, Xiexiuqi (Xie XiuQi),
Li Bin, Jason Yan, Peter Zijlstra, Ingo Molnar,
Linux Security Module list, SELinux, yangyingliang
In-Reply-To: <CAHC9VhSZBh8B+1CPM=PdLdbSFq1ba1ffuOJTgnzE5oBLXUEDxQ@mail.gmail.com>
On 2019/4/23 3:48, Paul Moore wrote:
> On Sat, Apr 20, 2019 at 3:39 AM Yang Yingliang <yangyingliang@huawei.com> wrote:
>> I'm not sure you got my point.
> I went back and looked at your previous emails again to try and
> understand what you are talking about, and I'm a little confused by
> some of the output ...
>
>> --- a/kernel/acct.c
>> +++ b/kernel/acct.c
>> @@ -481,6 +481,7 @@ static void do_acct_process(struct bsd_acct_struct
>> *acct)
>> flim = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
>> current->signal->rlim[RLIMIT_FSIZE].rlim_cur = RLIM_INFINITY;
>> /* Perform file operations on behalf of whoever enabled
>> accounting */
>> + pr_info("task:%px new cred:%px real cred:%px cred:%px\n",
>> current, file->f_cred, current->real_cred, current->cred);
>> orig_cred = override_creds(file->f_cred);
> Okay, with this patch applied we should the task/cred info when
> do_acct_process is called. Got it.
>
>> Messages:
>> [ 56.643298] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real
>> cred:ffff88841ae450c0 cred:ffff88841ae450c0 //They are same.
> Okay, it looks like do_acct_process() was called and f_cred,
> real_cred, and cred are all the same.
This is a original message, without patch applied.
>
>> [ 56.646609] Process accounting resumed
> It looks like do_acct_process() has called check_free_space() now. So
> far so good.
>
>> [ 56.649943] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real
>> cred:ffff88841c96c300 cred:ffff88841ae450c0
> Wait a minute ... why are we seeing this again? Looking at the task
> pointer and the timestamp, this is the same task exiting and trying to
> write to the accounting file, yes? This output is particularly
> curious since it appears that real_cred has changed; where is this
> happening?
This is the message when the BUG_ON was triggered without applying any
fix patch.
If we apply this patch "proc: prevent changes to overridden
credentials", program
runs like this:
1. As print message shows, before overriden, the pointer has the
following value:
real_cread=cred=0xffff88841ae450c0, f_cred=0xffff88841ae450c0
override_creds() is called in do_acct_process():
...
/* Perform file operations on behalf of whoever enabled accounting */
orig_cred = override_creds(file->f_cred);
...
2. After override_creds(), if (current_cred() != current_real_cred()) is
not work here,
we will call commit_creds() in security_setprocattr().
...
/* Prevent changes to overridden credentials. */
if (current_cred() != current_real_cred()) {
rcu_read_unlock();
return -EBUSY;
}
...
3. After commit_creds(), we have new cred and real_cred.
security_setprocattr() //commit_creds is called here
4. revert_creds() is called in in do_acct_process(), the cred
is reverted to the old value(0xffff88841ae450c0)
...
current->signal->rlim[RLIMIT_FSIZE].rlim_cur = flim;
revert_creds(orig_cred);
5. After reverting, cred and real_cred are not equal.
If it has a risk to trigger the BUG_ON, when doing another
commit_creds() ?
^ permalink raw reply
* Re: [PATCH 1/3] mm: security: introduce the init_allocations=1 boot option
From: Michal Hocko @ 2019-04-23 8:31 UTC (permalink / raw)
To: Dave Hansen
Cc: Alexander Potapenko, akpm, cl, dvyukov, keescook, labbott,
linux-mm, linux-security-module, kernel-hardening
In-Reply-To: <981d439a-1107-2730-f27e-17635ee4a125@intel.com>
On Thu 18-04-19 09:35:32, Dave Hansen wrote:
> On 4/18/19 8:42 AM, Alexander Potapenko wrote:
> > This option adds the possibility to initialize newly allocated pages and
> > heap objects with zeroes. This is needed to prevent possible information
> > leaks and make the control-flow bugs that depend on uninitialized values
> > more deterministic.
>
> Isn't it better to do this at free time rather than allocation time? If
> doing it at free, you can't even have information leaks for pages that
> are in the allocator.
I would tend to agree here. Free path is usually less performance sensitive
than the allocation. Those really hot paths tend to defer the work.
I am also worried that an opt-out gfp flag would tend to be used
incorrectly as the history has shown for others - e.g. __GFP_TEMPORARY.
So I would rather see this robust without a fine tuning unless there is
real use case that would suffer from this and we can think of a
background scrubbing or something similar.
--
Michal Hocko
SUSE Labs
^ 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