x86: text_poke() fixes and executable lockdowns
Subsystems (22)
v5 cover x86: text_poke() fixes and executable lockdowns
Yet another version, per PeterZ request, addressing the latest feedback.
This patchset improves several overlapping issues around stale TLB entries and W^X violations. It is combined from "x86/alternative:text_poke() enhancements v7" [1] and "Don't leave executable TLB entries to freed pages v2" [2] patchsets that were conflicting.
The related issues that this fixes:
1. Fixmap PTEs that are used for patching are available for access from other cores and might be exploited. They are not even flushed from the TLB in remote cores, so the risk is even higher. Address this issue by introducing a temporary mm that is only used during patching. Unfortunately, due to init ordering, fixmap is still used during boot-time patching. Future patches can eliminate the need for it.
2. Missing lockdep assertion to ensure text_mutex is taken. It is actually not always taken, so fix the instances that were found not to take the lock (although they should be safe even without taking the lock).
3. Module_alloc returning memory that is RWX until a module is finished loading.
4. Sometimes when memory is freed via the module subsystem, an executable permissioned TLB entry can remain to a freed page. If the page is re-used to back an address that will receive data from userspace, it can result in user data being mapped as executable in the kernel. The root of this behavior is vfree lazily flushing the TLB, but not lazily freeing the underlying pages.
Changes v4 to v5:
- Change temporary state variable name [Borislav]
- Commit log and comment fixes [Borislav]
Changes v3 to v4:
- Remove the size parameter from tramp_free() [Steven]
- Remove caching of hw_breakpoint_active() [Sean]
- Prevent the use of bpf_probe_write_user() while using temporary mm [Jann]
- Fix build issues on other archs
Changes v2 to v3:
- Fix commit messages and comments [Boris]
- Rename VM_HAS_SPECIAL_PERMS [Boris]
- Remove unnecessary local variables [Boris]
- Rename set_alias_*() functions [Boris, Andy]
- Save/restore DR registers when using temporary mm
- Move line deletion from patch 10 to patch 17
Changes v1 to v2:
- Adding "Reviewed-by tag" [Masami]
- Comment instead of code to warn against module removal while patching [Masami]
- Avoiding open-coded TLB flush [Andy]
- Remove "This patch" [Borislav Petkov]
- Not set global bit during text poking [Andy, hpa]
- Add Ack from [Pavel Machek]
- Split patch 16 "Plug in new special vfree flag" into 4 patches (16-19) to make it easier to review. There were no code changes.
The changes from "Don't leave executable TLB entries to freed pages v2" to v1:
- Add support for case of hibernate trying to save an unmapped page on the directmap. (Ard Biesheuvel)
- No week arch breakout for vfree-ing special memory (Andy Lutomirski)
- Avoid changing deferred free code by moving modules init free to work queue (Andy Lutomirski)
- Plug in new flag for kprobes and ftrace
- More arch generic names for set_pages functions (Ard Biesheuvel)
- Fix for TLB not always flushing the directmap (Nadav Amit)
Changes from "x86/alternative: text_poke() enhancements v7" to v1
- Fix build failure on CONFIG_RANDOMIZE_BASE=n (Rick)
- Remove text_poke usage from ftrace (Nadav)
[1] https://lkml.org/lkml/2018/12/5/200
[2] https://lkml.org/lkml/2018/12/11/1571
Andy Lutomirski (1):
x86/mm: Introduce temporary mm structs
Nadav Amit (15):
Fix "x86/alternatives: Lockdep-enforce text_mutex in text_poke*()" x86/jump_label: Use text_poke_early() during early init x86/mm: Save debug registers when loading a temporary mm
fork: Provide a function for copying init_mm x86/alternative: Initialize temporary mm for patching x86/alternative: Use temporary mm for text poking x86/kgdb: Avoid redundant comparison of patched code x86/ftrace: Set trampoline pages as executable x86/kprobes: Set instruction page as executable x86/module: Avoid breaking W^X while loading modules x86/jump-label: Remove support for custom poker x86/alternative: Remove the return value of text_poke_*() x86/alternative: Comment about module removal races mm/tlb: Provide default nmi_uaccess_okay()
bpf: Fail bpf_probe_write_user() while mm is switched
Rick Edgecombe (7):
x86/mm/cpa: Add set_direct_map_ functions
mm: Make hibernate handle unmapped pages
vmalloc: Add flag for free of special permsissions
modules: Use vmalloc special flag
bpf: Use vmalloc special flag
x86/ftrace: Use vmalloc special flag x86/kprobes: Use vmalloc special flag
arch/Kconfig | 4 + arch/x86/Kconfig | 1 + arch/x86/include/asm/fixmap.h | 2 - arch/x86/include/asm/mmu_context.h | 56 ++++++++ arch/x86/include/asm/pgtable.h | 3 + arch/x86/include/asm/set_memory.h | 3 + arch/x86/include/asm/text-patching.h | 7 +- arch/x86/include/asm/tlbflush.h | 2 + arch/x86/kernel/alternative.c | 201 ++++++++++++++++++++------- arch/x86/kernel/ftrace.c | 22 +-- arch/x86/kernel/jump_label.c | 21 ++- arch/x86/kernel/kgdb.c | 25 +--- arch/x86/kernel/kprobes/core.c | 19 ++- arch/x86/kernel/module.c | 2 +- arch/x86/mm/init_64.c | 36 +++++ arch/x86/mm/pageattr.c | 16 ++- arch/x86/xen/mmu_pv.c | 2 - include/asm-generic/tlb.h | 9 ++ include/linux/filter.h | 18 +-- include/linux/mm.h | 18 +-- include/linux/sched/task.h | 1 + include/linux/set_memory.h | 11 ++ include/linux/vmalloc.h | 15 ++ init/main.c | 3 + kernel/bpf/core.c | 1 - kernel/fork.c | 24 +++- kernel/module.c | 82 ++++++----- kernel/power/snapshot.c | 5 +- kernel/trace/bpf_trace.c | 8 ++ mm/page_alloc.c | 7 +- mm/vmalloc.c | 113 ++++++++++++--- 31 files changed, 542 insertions(+), 195 deletions(-)
Signature · 1 line
2.17.1
PATCH v5 1/23 Fix "x86/alternatives: Lockdep-enforce text_mutex in text_poke*()"
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.
--- 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(-)
arch/x86/include/asm/text-patching.h +1 -0
@@ -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;
arch/x86/kernel/alternative.c +38 -14
70 lines not on this page · raw message
arch/x86/kernel/kgdb.c +6 -5
@@ -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;
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/alternatives: Add text_poke_kgdb() to not assert the lock when debugging
x86/alternatives: Add text_poke_kgdb() to not assert the lock when debugging
Commit-ID: e836673c9b4966bc78e38aeda25f7022c57f0e90 Gitweb: https://git.kernel.org/tip/e836673c9b4966bc78e38aeda25f7022c57f0e90 Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:21 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:47 +0200
x86/alternatives: Add text_poke_kgdb() to not assert the lock when debugging
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.
--- 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(-)
arch/x86/include/asm/text-patching.h +1 -0
@@ -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;
arch/x86/kernel/alternative.c +38 -14
70 lines not on this page · raw message
arch/x86/kernel/kgdb.c +6 -5
@@ -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;
PATCH v5 2/23 x86/jump_label: Use text_poke_early() during early init
In reply to Nadav AmitThere 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.
--- arch/x86/kernel/jump_label.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
arch/x86/kernel/jump_label.c +6 -1
@@ -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 ) {
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/jump_label: Use text_poke_early() during early init
x86/jump_label: Use text_poke_early() during early init
Commit-ID: 1fd8de46d01d95f875c12684a6a03559831e8b4c Gitweb: https://git.kernel.org/tip/1fd8de46d01d95f875c12684a6a03559831e8b4c Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:22 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:49 +0200
x86/jump_label: Use text_poke_early() during early init
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.
--- arch/x86/kernel/jump_label.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
arch/x86/kernel/jump_label.c +6 -1
@@ -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 ) {
PATCH v5 3/23 x86/mm: Introduce temporary mm structs
In reply to Nadav AmitFrom: Andy Lutomirski <[email protected]>
Using a dedicated page-table for temporary PTEs prevents other cores from using - even speculatively - these PTEs, thereby providing two benefits:
(1) Security hardening: an attacker that gains kernel memory writing abilities cannot easily overwrite sensitive data.
(2) Avoiding TLB shootdowns: the PTEs do not need to be flushed in remote page-tables.
To do so a temporary mm_struct can be used. Mappings which are private for this mm can be set in the userspace part of the address-space. During the whole time in which the temporary mm is loaded, interrupts must be disabled.
The first use-case for temporary mm struct, which will follow, is for poking the kernel text.
[ Commit message was written by Nadav Amit ]
--- arch/x86/include/asm/mmu_context.h | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
arch/x86/include/asm/mmu_context.h +33 -0
@@ -356,4 +356,37 @@ static inline unsigned long __get_current_cr3_fast(void) return cr3; } +typedef struct { +struct mm_struct *mm; +}temp_mm_state_t ; + +/* +* Using a temporary mm allows to set temporary mappings that are not accessible +* by other CPUs. Such mappings are needed to perform sensitive memory writes +* that override the kernel memory protections (e.g., W^X), without exposing the +* temporary page-table mappings that are required for these write operations to +* other CPUs. Using a temporary mm also allows to avoid TLB shootdowns when the +* mapping is torn down. +* +* Context: The temporary mm needs to be used exclusively by a single core. To +* harden security IRQs must be disabled while the temporary mm is +* loaded, thereby preventing interrupt handler bugs from overriding +* the kernel memory protection. +*/ +static inline temp_mm_state_t use_temporary_mm (struct mm_struct *mm) +{ +temp_mm_state_t temp_state; + +lockdep_assert_irqs_disabled (); + temp_state.mm =this_cpu_read (cpu_tlbstate.loaded_mm); +switch_mm_irqs_off (NULL , mm, current); +return temp_state; +} + +static inline void unuse_temporary_mm (temp_mm_state_t prev_state) +{ +lockdep_assert_irqs_disabled (); +switch_mm_irqs_off (NULL , prev_state.mm, current); +} +#endif /* _ASM_X86_MMU_CONTEXT_H */
Signature · 1 line
2.17.1
tip-bot for Andy Lutomirski
merge bot[tip:x86/mm] x86/mm: Introduce temporary mm structs
x86/mm: Introduce temporary mm structs
Commit-ID: cefa929c034eb5d9c15c50088235a0093a219687 Gitweb: https://git.kernel.org/tip/cefa929c034eb5d9c15c50088235a0093a219687 Author: Andy Lutomirski <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:23 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:50 +0200
x86/mm: Introduce temporary mm structs
Using a dedicated page-table for temporary PTEs prevents other cores from using - even speculatively - these PTEs, thereby providing two benefits:
(1) Security hardening: an attacker that gains kernel memory writing abilities cannot easily overwrite sensitive data.
(2) Avoiding TLB shootdowns: the PTEs do not need to be flushed in remote page-tables.
To do so a temporary mm_struct can be used. Mappings which are private for this mm can be set in the userspace part of the address-space. During the whole time in which the temporary mm is loaded, interrupts must be disabled.
The first use-case for temporary mm struct, which will follow, is for poking the kernel text.
[ Commit message was written by Nadav Amit ]
--- arch/x86/include/asm/mmu_context.h | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
arch/x86/include/asm/mmu_context.h +33 -0
@@ -356,4 +356,37 @@ static inline unsigned long __get_current_cr3_fast(void) return cr3; } +typedef struct { +struct mm_struct *mm; +}temp_mm_state_t ; + +/* +* Using a temporary mm allows to set temporary mappings that are not accessible +* by other CPUs. Such mappings are needed to perform sensitive memory writes +* that override the kernel memory protections (e.g., W^X), without exposing the +* temporary page-table mappings that are required for these write operations to +* other CPUs. Using a temporary mm also allows to avoid TLB shootdowns when the +* mapping is torn down. +* +* Context: The temporary mm needs to be used exclusively by a single core. To +* harden security IRQs must be disabled while the temporary mm is +* loaded, thereby preventing interrupt handler bugs from overriding +* the kernel memory protection. +*/ +static inline temp_mm_state_t use_temporary_mm (struct mm_struct *mm) +{ +temp_mm_state_t temp_state; + +lockdep_assert_irqs_disabled (); + temp_state.mm =this_cpu_read (cpu_tlbstate.loaded_mm); +switch_mm_irqs_off (NULL , mm, current); +return temp_state; +} + +static inline void unuse_temporary_mm (temp_mm_state_t prev_state) +{ +lockdep_assert_irqs_disabled (); +switch_mm_irqs_off (NULL , prev_state.mm, current); +} +#endif /* _ASM_X86_MMU_CONTEXT_H */
PATCH v5 4/23 x86/mm: Save debug registers when loading a temporary mm
In reply to Nadav AmitPrevent user watchpoints from mistakenly firing while the temporary mm is being used. As the addresses of the temporary mm might overlap those of the user-process, this is necessary to prevent wrong signals or worse things from happening.
--- arch/x86/include/asm/mmu_context.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
arch/x86/include/asm/mmu_context.h +23 -0
@@ -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 (); temp_state.mm =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 temp_state; }
@@ -387,6 +403,13 @@ static inline void unuse_temporary_mm(temp_mm_state_t prev_state) {lockdep_assert_irqs_disabled ();switch_mm_irqs_off (NULL , prev_state.mm, 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 */
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/mm: Save debug registers when loading a temporary mm
x86/mm: Save debug registers when loading a temporary mm
Commit-ID: d97080ebed7811a53c931032a284166ee46b9565 Gitweb: https://git.kernel.org/tip/d97080ebed7811a53c931032a284166ee46b9565 Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:24 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:50 +0200
x86/mm: Save debug registers when loading a temporary mm
Prevent user watchpoints from mistakenly firing while the temporary mm is being used. As the addresses of the temporary mm might overlap those of the user-process, this is necessary to prevent wrong signals or worse things from happening.
--- arch/x86/include/asm/mmu_context.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
arch/x86/include/asm/mmu_context.h +23 -0
@@ -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 (); temp_state.mm =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 temp_state; }
@@ -387,6 +403,13 @@ static inline void unuse_temporary_mm(temp_mm_state_t prev_state) {lockdep_assert_irqs_disabled ();switch_mm_irqs_off (NULL , prev_state.mm, 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 */
PATCH v5 5/23 fork: Provide a function for copying init_mm
In reply to Nadav AmitProvide a function for copying init_mm. This function will be later used for setting a temporary mm.
--- include/linux/sched/task.h | 1 + kernel/fork.c | 24 ++++++++++++++++++------ 2 files changed, 19 insertions(+), 6 deletions(-)
include/linux/sched/task.h +1 -0
@@ -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 *);
kernel/fork.c +18 -6
@@ -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. *
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] fork: Provide a function for copying init_mm
fork: Provide a function for copying init_mm
Commit-ID: 13585fa0668c724efab9635aaeef6ec390217415 Gitweb: https://git.kernel.org/tip/13585fa0668c724efab9635aaeef6ec390217415 Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:25 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:51 +0200
fork: Provide a function for copying init_mm
Provide a function for copying init_mm. This function will be later used for setting a temporary mm.
--- include/linux/sched/task.h | 1 + kernel/fork.c | 24 ++++++++++++++++++------ 2 files changed, 19 insertions(+), 6 deletions(-)
include/linux/sched/task.h +1 -0
@@ -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 *);
kernel/fork.c +18 -6
@@ -1299,13 +1299,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 ();
@@ -1372,7 +1379,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;
@@ -2187,6 +2194,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. *
PATCH v5 6/23 x86/alternative: Initialize temporary mm for patching
In reply to Nadav AmitTo 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.
--- 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(+)
arch/x86/include/asm/pgtable.h +3 -0
@@ -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
arch/x86/include/asm/text-patching.h +2 -0
@@ -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 */
arch/x86/kernel/alternative.c +3 -0
@@ -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;
arch/x86/mm/init_64.c +36 -0
@@ -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.
init/main.c +3 -0
@@ -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 ();
Signature · 1 line
2.17.1
PATCH v5 7/23 x86/alternative: Use temporary mm for text poking
In reply to Nadav Amittext_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.
--- 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(-)
arch/x86/include/asm/fixmap.h +0 -2
@@ -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
arch/x86/kernel/alternative.c +86 -22
133 lines not on this page · raw message
arch/x86/xen/mmu_pv.c +0 -2
@@ -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 ;
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/alternatives: Use temporary mm for text poking
x86/alternatives: Use temporary mm for text poking
Commit-ID: b3fd8e83ada0d51b71a84297480187e2d40e5ded Gitweb: https://git.kernel.org/tip/b3fd8e83ada0d51b71a84297480187e2d40e5ded Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:27 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:52 +0200
x86/alternatives: Use temporary mm for text poking
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.
--- 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(-)
arch/x86/include/asm/fixmap.h +0 -2
@@ -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
arch/x86/kernel/alternative.c +86 -22
133 lines not on this page · raw message
arch/x86/xen/mmu_pv.c +0 -2
@@ -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 ;
PATCH v5 8/23 x86/kgdb: Avoid redundant comparison of patched code
In reply to Nadav Amittext_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.
--- arch/x86/kernel/kgdb.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
arch/x86/kernel/kgdb.c +1 -13
@@ -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,
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/kgdb: Avoid redundant comparison of patched code
x86/kgdb: Avoid redundant comparison of patched code
Commit-ID: 86a22057127d1c0462a18901421bf1ff89491392 Gitweb: https://git.kernel.org/tip/86a22057127d1c0462a18901421bf1ff89491392 Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:28 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:53 +0200
x86/kgdb: Avoid redundant comparison of patched code
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.
--- arch/x86/kernel/kgdb.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
arch/x86/kernel/kgdb.c +1 -13
@@ -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,
PATCH v5 9/23 x86/ftrace: Set trampoline pages as executable
In reply to Nadav AmitSince 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.
--- arch/x86/kernel/ftrace.c | 8 ++++++++ 1 file changed, 8 insertions(+)
arch/x86/kernel/ftrace.c +8 -0
@@ -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);
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/ftrace: Set trampoline pages as executable
x86/ftrace: Set trampoline pages as executable
Commit-ID: 3c0dab44e22782359a0a706cbce72de99a22aa75 Gitweb: https://git.kernel.org/tip/3c0dab44e22782359a0a706cbce72de99a22aa75 Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:29 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:53 +0200
x86/ftrace: Set trampoline pages as executable
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.
--- arch/x86/kernel/ftrace.c | 8 ++++++++ 1 file changed, 8 insertions(+)
arch/x86/kernel/ftrace.c +8 -0
@@ -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);
PATCH v5 10/23 x86/kprobes: Set instruction page as executable
In reply to Nadav AmitSet 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.
--- arch/x86/kernel/kprobes/core.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
arch/x86/kernel/kprobes/core.c +20 -4
@@ -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); }
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/kprobes: Set instruction page as executable
x86/kprobes: Set instruction page as executable
Commit-ID: 7298e24f904224fa79eb8fd7e0fbd78950ccf2db Gitweb: https://git.kernel.org/tip/7298e24f904224fa79eb8fd7e0fbd78950ccf2db Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:30 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:54 +0200
x86/kprobes: Set instruction page as executable
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.
--- arch/x86/kernel/kprobes/core.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
arch/x86/kernel/kprobes/core.c +20 -4
@@ -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); }
PATCH v5 11/23 x86/module: Avoid breaking W^X while loading modules
In reply to Nadav AmitWhen 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.
--- 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(-)
arch/x86/kernel/alternative.c +21 -7
@@ -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; }
arch/x86/kernel/module.c +1 -1
@@ -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);
include/linux/filter.h +1 -0
@@ -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)
kernel/module.c +5 -0
@@ -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)
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/modules: Avoid breaking W^X while loading modules
x86/modules: Avoid breaking W^X while loading modules
Commit-ID: f2c65fb3221adc6b73b0549fc7ba892022db9797 Gitweb: https://git.kernel.org/tip/f2c65fb3221adc6b73b0549fc7ba892022db9797 Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:31 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:55 +0200
x86/modules: Avoid breaking W^X while loading modules
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.
--- 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(-)
arch/x86/kernel/alternative.c +21 -7
@@ -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; }
arch/x86/kernel/module.c +1 -1
@@ -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);
include/linux/filter.h +1 -0
@@ -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)
kernel/module.c +5 -0
@@ -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)
PATCH v5 12/23 x86/jump-label: Remove support for custom poker
In reply to Nadav AmitThere 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.
--- arch/x86/kernel/jump_label.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-)
arch/x86/kernel/jump_label.c +10 -16
@@ -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 ); }
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/jump-label: Remove support for custom text poker
x86/jump-label: Remove support for custom text poker
Commit-ID: bb0a008d6a2c543efc11313b448d2f26f91dc4f8 Gitweb: https://git.kernel.org/tip/bb0a008d6a2c543efc11313b448d2f26f91dc4f8 Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:32 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:55 +0200
x86/jump-label: Remove support for custom text poker
There are only two types of text poking: early and breakpoint based. The use of a function pointer to perform text poking complicates the code and is probably inefficient due to the use of indirect branches.
--- arch/x86/kernel/jump_label.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-)
arch/x86/kernel/jump_label.c +10 -16
@@ -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 ); }
PATCH v5 13/23 x86/alternative: Remove the return value of text_poke_*()
In reply to Nadav AmitThe return value of text_poke_early() and text_poke_bp() is useless. Remove it.
--- arch/x86/include/asm/text-patching.h | 4 ++-- arch/x86/kernel/alternative.c | 11 ++++------- 2 files changed, 6 insertions(+), 9 deletions(-)
arch/x86/include/asm/text-patching.h +2 -2
@@ -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;
arch/x86/kernel/alternative.c +4 -7
@@ -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; }
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/alternatives: Remove the return value of text_poke_*()
x86/alternatives: Remove the return value of text_poke_*()
Commit-ID: 0a203df5cf0eb709be4f190314e262b72d7e5b76 Gitweb: https://git.kernel.org/tip/0a203df5cf0eb709be4f190314e262b72d7e5b76 Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:33 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:56 +0200
x86/alternatives: Remove the return value of text_poke_*()
The return value of text_poke_early() and text_poke_bp() is useless. Remove it.
--- arch/x86/include/asm/text-patching.h | 4 ++-- arch/x86/kernel/alternative.c | 11 ++++------- 2 files changed, 6 insertions(+), 9 deletions(-)
arch/x86/include/asm/text-patching.h +2 -2
@@ -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;
arch/x86/kernel/alternative.c +4 -7
@@ -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; }
PATCH v5 14/23 x86/mm/cpa: Add set_direct_map_ functions
In reply to Nadav AmitFrom: Rick Edgecombe <[email protected]>
Add two new functions set_direct_map_default_noflush() and set_direct_map_invalid_noflush() for setting the direct map alias for the page to its default valid permissions and to an invalid state that cannot be cached in a TLB, respectively. These functions do not flush the TLB.
Note, __kernel_map_pages() does something similar but flushes the TLB and doesn't reset the permission bits to default on all architectures.
Also add an ARCH config ARCH_HAS_SET_DIRECT_MAP for specifying whether these have an actual implementation or a default empty one.
--- arch/Kconfig | 4 ++++ arch/x86/Kconfig | 1 + arch/x86/include/asm/set_memory.h | 3 +++ arch/x86/mm/pageattr.c | 14 +++++++++++--- include/linux/set_memory.h | 11 +++++++++++ 5 files changed, 30 insertions(+), 3 deletions(-)
arch/Kconfig +4 -0
@@ -249,6 +249,10 @@ config ARCH_HAS_FORTIFY_SOURCE config ARCH_HAS_SET_MEMORY bool +# Select if arch has all set_direct_map_invalid/default() functions +config ARCH_HAS_SET_DIRECT_MAP + bool + # Select if arch init_task must go in the __init_task_data section config ARCH_TASK_STRUCT_ON_STACK bool
arch/x86/Kconfig +1 -0
@@ -66,6 +66,7 @@ config X86 select ARCH_HAS_UACCESS_FLUSHCACHE if X86_64 select ARCH_HAS_UACCESS_MCSAFE if X86_64 && X86_MCE select ARCH_HAS_SET_MEMORY + select ARCH_HAS_SET_DIRECT_MAP select ARCH_HAS_STRICT_KERNEL_RWX select ARCH_HAS_STRICT_MODULE_RWX select ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
arch/x86/include/asm/set_memory.h +3 -0
@@ -85,6 +85,9 @@ int set_pages_nx(struct page *page, int numpages);int set_pages_ro (struct page *page,int numpages);int set_pages_rw (struct page *page,int numpages); +int set_direct_map_invalid_noflush (struct page *page); +int set_direct_map_default_noflush (struct page *page); +extern int kernel_set_to_readonly;void set_kernel_text_rw (void );void set_kernel_text_ro (void );
arch/x86/mm/pageattr.c +11 -3
@@ -2209,8 +2209,6 @@ int set_pages_rw(struct page *page, int numpages)return set_memory_rw (addr, numpages); } -#ifdef CONFIG_DEBUG_PAGEALLOC -static int __set_pages_p (struct page *page,int numpages) {unsigned long tempaddr = (unsigned long )page_address (page);
@@ -2249,6 +2247,17 @@ static int __set_pages_np(struct page *page, int numpages)return __change_page_attr_set_clr (&cpa,0 ); } +int set_direct_map_invalid_noflush (struct page *page) +{ +return __set_pages_np (page,1 ); +} + +int set_direct_map_default_noflush (struct page *page) +{ +return __set_pages_p (page,1 ); +} + +#ifdef CONFIG_DEBUG_PAGEALLOC void __kernel_map_pages (struct page *page,int numpages,int enable) {if (PageHighMem (page))
@@ -2282,7 +2291,6 @@ void __kernel_map_pages(struct page *page, int numpages, int enable) }#ifdef CONFIG_HIBERNATION -bool kernel_page_present (struct page *page) {unsigned int level;
include/linux/set_memory.h +11 -0
@@ -17,6 +17,17 @@ static inline int set_memory_x(unsigned long addr, int numpages) { return 0; }static inline int set_memory_nx (unsigned long addr,int numpages) {return 0 ; }#endif +#ifndef CONFIG_ARCH_HAS_SET_DIRECT_MAP +static inline int set_direct_map_invalid_noflush (struct page *page) +{ +return 0 ; +} +static inline int set_direct_map_default_noflush (struct page *page) +{ +return 0 ; +} +#endif +#ifndef set_mce_nospecstatic inline int set_mce_nospec (unsigned long pfn) {
Signature · 1 line
2.17.1
Re: [PATCH v5 14/23] x86/mm/cpa: Add set_direct_map_ functions
Nadav,
I get
dmarc=fail (p=QUARANTINE sp=NONE dis=QUARANTINE) header.from=vmware.com
for these emails, because they lack the vmware DKIM signature.
It clearly did go through some vmware mail servers, but apparently not the *right* external vmware SMTP gateway.
Please check with vmware MIS what the right SMTP setup for git-send-email is.
Linus
Quoted 7 lines · Nadav Amit
From: Rick Edgecombe <[email protected]>
Add two new functions
set_direct_map_default_noflush()andset_direct_map_invalid_noflush()for setting the direct map alias for the page to its default valid permissions and to an invalid state that cannot be cached in a TLB, respectively. These functions do not flush the TLB.
...
Re: [PATCH v5 14/23] x86/mm/cpa: Add set_direct_map_ functions
Quoted 6 lines · Linus Torvalds
Nadav,
I get
Quoted 8 lines
dmarc=fail (p=QUARANTINE sp=NONE dis=QUARANTINE) header.from=vmware.com
for these emails, because they lack the vmware DKIM signature.
It clearly did go through some vmware mail servers, but apparently not the *right* external vmware SMTP gateway.
Please check with vmware MIS what the right SMTP setup for git-send-email is.
Err.. Sorry for that. For the time being I’ll use gmail instead.
tip-bot for Rick Edgecombe
merge bot[tip:x86/mm] x86/mm/cpa: Add set_direct_map_*() functions
In reply to Nadav Amitx86/mm/cpa: Add set_direct_map_*() functions
Commit-ID: d253ca0c3865a8d9a8c01143cf20425e0be4d0ce Gitweb: https://git.kernel.org/tip/d253ca0c3865a8d9a8c01143cf20425e0be4d0ce Author: Rick Edgecombe <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:34 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:56 +0200
x86/mm/cpa: Add set_direct_map_*() functions
Add two new functions set_direct_map_default_noflush() and set_direct_map_invalid_noflush() for setting the direct map alias for the page to its default valid permissions and to an invalid state that cannot be cached in a TLB, respectively. These functions do not flush the TLB.
Note, __kernel_map_pages() does something similar but flushes the TLB and doesn't reset the permission bits to default on all architectures.
Also add an ARCH config ARCH_HAS_SET_DIRECT_MAP for specifying whether these have an actual implementation or a default empty one.
--- arch/Kconfig | 4 ++++ arch/x86/Kconfig | 1 + arch/x86/include/asm/set_memory.h | 3 +++ arch/x86/mm/pageattr.c | 14 +++++++++++--- include/linux/set_memory.h | 11 +++++++++++ 5 files changed, 30 insertions(+), 3 deletions(-)
arch/Kconfig +4 -0
@@ -249,6 +249,10 @@ config ARCH_HAS_FORTIFY_SOURCE config ARCH_HAS_SET_MEMORY bool +# Select if arch has all set_direct_map_invalid/default() functions +config ARCH_HAS_SET_DIRECT_MAP + bool + # Select if arch init_task must go in the __init_task_data section config ARCH_TASK_STRUCT_ON_STACK bool
arch/x86/Kconfig +1 -0
@@ -65,6 +65,7 @@ config X86 select ARCH_HAS_UACCESS_FLUSHCACHE if X86_64 select ARCH_HAS_UACCESS_MCSAFE if X86_64 && X86_MCE select ARCH_HAS_SET_MEMORY + select ARCH_HAS_SET_DIRECT_MAP select ARCH_HAS_STRICT_KERNEL_RWX select ARCH_HAS_STRICT_MODULE_RWX select ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
arch/x86/include/asm/set_memory.h +3 -0
@@ -85,6 +85,9 @@ int set_pages_nx(struct page *page, int numpages);int set_pages_ro (struct page *page,int numpages);int set_pages_rw (struct page *page,int numpages); +int set_direct_map_invalid_noflush (struct page *page); +int set_direct_map_default_noflush (struct page *page); +extern int kernel_set_to_readonly;void set_kernel_text_rw (void );void set_kernel_text_ro (void );
arch/x86/mm/pageattr.c +11 -3
@@ -2209,8 +2209,6 @@ int set_pages_rw(struct page *page, int numpages)return set_memory_rw (addr, numpages); } -#ifdef CONFIG_DEBUG_PAGEALLOC -static int __set_pages_p (struct page *page,int numpages) {unsigned long tempaddr = (unsigned long )page_address (page);
@@ -2249,6 +2247,17 @@ static int __set_pages_np(struct page *page, int numpages)return __change_page_attr_set_clr (&cpa,0 ); } +int set_direct_map_invalid_noflush (struct page *page) +{ +return __set_pages_np (page,1 ); +} + +int set_direct_map_default_noflush (struct page *page) +{ +return __set_pages_p (page,1 ); +} + +#ifdef CONFIG_DEBUG_PAGEALLOC void __kernel_map_pages (struct page *page,int numpages,int enable) {if (PageHighMem (page))
@@ -2282,7 +2291,6 @@ void __kernel_map_pages(struct page *page, int numpages, int enable) }#ifdef CONFIG_HIBERNATION -bool kernel_page_present (struct page *page) {unsigned int level;
include/linux/set_memory.h +11 -0
@@ -17,6 +17,17 @@ static inline int set_memory_x(unsigned long addr, int numpages) { return 0; }static inline int set_memory_nx (unsigned long addr,int numpages) {return 0 ; }#endif +#ifndef CONFIG_ARCH_HAS_SET_DIRECT_MAP +static inline int set_direct_map_invalid_noflush (struct page *page) +{ +return 0 ; +} +static inline int set_direct_map_default_noflush (struct page *page) +{ +return 0 ; +} +#endif +#ifndef set_mce_nospecstatic inline int set_mce_nospec (unsigned long pfn) {
PATCH v5 15/23 mm: Make hibernate handle unmapped pages
In reply to Nadav AmitFrom: Rick Edgecombe <[email protected]>
Make hibernate handle unmapped pages on the direct map when CONFIG_ARCH_HAS_SET_ALIAS is set. These functions allow for setting pages to invalid configurations, so now hibernate should check if the pages have valid mappings and handle if they are unmapped when doing a hibernate save operation.
Previously this checking was already done when CONFIG_DEBUG_PAGEALLOC was configured. It does not appear to have a big hibernating performance impact. The speed of the saving operation before this change was measured as 819.02 MB/s, and after was measured at 813.32 MB/s.
Before:
[ 4.670938] PM: Wrote 171996 kbytes in 0.21 seconds (819.02 MB/s)
After:
[ 4.504714] PM: Wrote 178932 kbytes in 0.22 seconds (813.32 MB/s)
--- arch/x86/mm/pageattr.c | 4 ---- include/linux/mm.h | 18 ++++++------------ kernel/power/snapshot.c | 5 +++-- mm/page_alloc.c | 7 +++++-- 4 files changed, 14 insertions(+), 20 deletions(-)
arch/x86/mm/pageattr.c +0 -4
@@ -2257,7 +2257,6 @@ int set_direct_map_default_noflush(struct page *page)return __set_pages_p (page,1 ); } -#ifdef CONFIG_DEBUG_PAGEALLOC void __kernel_map_pages (struct page *page,int numpages,int enable) {if (PageHighMem (page))
@@ -2302,11 +2301,8 @@ bool kernel_page_present(struct page *page) pte =lookup_address ((unsigned long )page_address (page), &level); return (pte_val(*pte) & _PAGE_PRESENT); } -#endif /* CONFIG_HIBERNATION */ -#endif /* CONFIG_DEBUG_PAGEALLOC */ -int __init kernel_map_pages_in_pgd (pgd_t *pgd,u64 pfn,unsigned long address,unsigned numpages,unsigned long page_flags) {
include/linux/mm.h +6 -12
@@ -2610,37 +2610,31 @@ static inline void kernel_poison_pages(struct page *page, int numpages, int enable) { }#endif -#ifdef CONFIG_DEBUG_PAGEALLOC extern bool _debug_pagealloc_enabled; -extern void __kernel_map_pages (struct page *page,int numpages,int enable);static inline bool debug_pagealloc_enabled (void ) { -return _debug_pagealloc_enabled; +return IS_ENABLED (CONFIG_DEBUG_PAGEALLOC ) && _debug_pagealloc_enabled; } +#if defined(CONFIG_DEBUG_PAGEALLOC ) || defined(CONFIG_ARCH_HAS_SET_DIRECT_MAP ) +extern void __kernel_map_pages (struct page *page,int numpages,int enable); +static inline void kernel_map_pages (struct page *page,int numpages,int enable) { -if (!debug_pagealloc_enabled ()) -return ; -__kernel_map_pages (page, numpages, enable); }#ifdef CONFIG_HIBERNATION extern bool kernel_page_present (struct page *page);#endif /* CONFIG_HIBERNATION */ -#else /* CONFIG_DEBUG_PAGEALLOC */ +#else /* CONFIG_DEBUG_PAGEALLOC || CONFIG_ARCH_HAS_SET_DIRECT_MAP */ static inline void kernel_map_pages (struct page *page,int numpages,int enable) {}#ifdef CONFIG_HIBERNATION static inline bool kernel_page_present (struct page *page) {return true; }#endif /* CONFIG_HIBERNATION */ -static inline bool debug_pagealloc_enabled (void ) -{ -return false; -} -#endif /* CONFIG_DEBUG_PAGEALLOC */ +#endif /* CONFIG_DEBUG_PAGEALLOC || CONFIG_ARCH_HAS_SET_DIRECT_MAP */ #ifdef __HAVE_ARCH_GATE_AREAextern struct vm_area_struct *get_gate_vma (struct mm_struct *mm);
kernel/power/snapshot.c +3 -2
@@ -1342,8 +1342,9 @@ static inline void do_copy_page(long *dst, long *src)* safe_copy_page - Copy a page in a safe way. * * Check if the page we are going to copy is marked as present in the kernel -* page tables (this always is the case if CONFIG_DEBUG_PAGEALLOC is not set -* and in that case kernel_page_present() always returns 'true'). +* page tables. This always is the case if CONFIG_DEBUG_PAGEALLOC or +* CONFIG_ARCH_HAS_SET_DIRECT_MAP is not set. In that case kernel_page_present() +* always returns 'true'. */ static void safe_copy_page (void *dst,struct page *s_page) {
mm/page_alloc.c +5 -2
@@ -1131,7 +1131,9 @@ static __always_inline bool free_pages_prepare(struct page *page, }arch_free_page (page, order);kernel_poison_pages (page,1 << order,0 ); -kernel_map_pages (page,1 << order,0 ); +if (debug_pagealloc_enabled ()) +kernel_map_pages (page,1 << order,0 ); +kasan_free_nondeferred_pages (page, order);return true;
@@ -2001,7 +2003,8 @@ inline void post_alloc_hook(struct page *page, unsigned int order,set_page_refcounted (page);arch_alloc_page (page, order); -kernel_map_pages (page,1 << order,1 ); +if (debug_pagealloc_enabled ()) +kernel_map_pages (page,1 << order,1 );kasan_alloc_pages (page, order);kernel_poison_pages (page,1 << order,1 );set_page_owner (page, order, gfp_flags);
Signature · 1 line
2.17.1
tip-bot for Rick Edgecombe
merge bot[tip:x86/mm] mm/hibernation: Make hibernation handle unmapped pages
mm/hibernation: Make hibernation handle unmapped pages
Commit-ID: d63326928611600ad65baff54a70f53b02b3cdfe Gitweb: https://git.kernel.org/tip/d63326928611600ad65baff54a70f53b02b3cdfe Author: Rick Edgecombe <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:35 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:57 +0200
mm/hibernation: Make hibernation handle unmapped pages
Make hibernate handle unmapped pages on the direct map when CONFIG_ARCH_HAS_SET_ALIAS=y is set. These functions allow for setting pages to invalid configurations, so now hibernate should check if the pages have valid mappings and handle if they are unmapped when doing a hibernate save operation.
Previously this checking was already done when CONFIG_DEBUG_PAGEALLOC=y was configured. It does not appear to have a big hibernating performance impact. The speed of the saving operation before this change was measured as 819.02 MB/s, and after was measured at 813.32 MB/s.
Before:
[ 4.670938] PM: Wrote 171996 kbytes in 0.21 seconds (819.02 MB/s)
After:
[ 4.504714] PM: Wrote 178932 kbytes in 0.22 seconds (813.32 MB/s)
--- arch/x86/mm/pageattr.c | 4 ---- include/linux/mm.h | 18 ++++++------------ kernel/power/snapshot.c | 5 +++-- mm/page_alloc.c | 7 +++++-- 4 files changed, 14 insertions(+), 20 deletions(-)
arch/x86/mm/pageattr.c +0 -4
@@ -2257,7 +2257,6 @@ int set_direct_map_default_noflush(struct page *page)return __set_pages_p (page,1 ); } -#ifdef CONFIG_DEBUG_PAGEALLOC void __kernel_map_pages (struct page *page,int numpages,int enable) {if (PageHighMem (page))
@@ -2302,11 +2301,8 @@ bool kernel_page_present(struct page *page) pte =lookup_address ((unsigned long )page_address (page), &level); return (pte_val(*pte) & _PAGE_PRESENT); } -#endif /* CONFIG_HIBERNATION */ -#endif /* CONFIG_DEBUG_PAGEALLOC */ -int __init kernel_map_pages_in_pgd (pgd_t *pgd,u64 pfn,unsigned long address,unsigned numpages,unsigned long page_flags) {
include/linux/mm.h +6 -12
@@ -2610,37 +2610,31 @@ static inline void kernel_poison_pages(struct page *page, int numpages, int enable) { }#endif -#ifdef CONFIG_DEBUG_PAGEALLOC extern bool _debug_pagealloc_enabled; -extern void __kernel_map_pages (struct page *page,int numpages,int enable);static inline bool debug_pagealloc_enabled (void ) { -return _debug_pagealloc_enabled; +return IS_ENABLED (CONFIG_DEBUG_PAGEALLOC ) && _debug_pagealloc_enabled; } +#if defined(CONFIG_DEBUG_PAGEALLOC ) || defined(CONFIG_ARCH_HAS_SET_DIRECT_MAP ) +extern void __kernel_map_pages (struct page *page,int numpages,int enable); +static inline void kernel_map_pages (struct page *page,int numpages,int enable) { -if (!debug_pagealloc_enabled ()) -return ; -__kernel_map_pages (page, numpages, enable); }#ifdef CONFIG_HIBERNATION extern bool kernel_page_present (struct page *page);#endif /* CONFIG_HIBERNATION */ -#else /* CONFIG_DEBUG_PAGEALLOC */ +#else /* CONFIG_DEBUG_PAGEALLOC || CONFIG_ARCH_HAS_SET_DIRECT_MAP */ static inline void kernel_map_pages (struct page *page,int numpages,int enable) {}#ifdef CONFIG_HIBERNATION static inline bool kernel_page_present (struct page *page) {return true; }#endif /* CONFIG_HIBERNATION */ -static inline bool debug_pagealloc_enabled (void ) -{ -return false; -} -#endif /* CONFIG_DEBUG_PAGEALLOC */ +#endif /* CONFIG_DEBUG_PAGEALLOC || CONFIG_ARCH_HAS_SET_DIRECT_MAP */ #ifdef __HAVE_ARCH_GATE_AREAextern struct vm_area_struct *get_gate_vma (struct mm_struct *mm);
kernel/power/snapshot.c +3 -2
@@ -1342,8 +1342,9 @@ static inline void do_copy_page(long *dst, long *src)* safe_copy_page - Copy a page in a safe way. * * Check if the page we are going to copy is marked as present in the kernel -* page tables (this always is the case if CONFIG_DEBUG_PAGEALLOC is not set -* and in that case kernel_page_present() always returns 'true'). +* page tables. This always is the case if CONFIG_DEBUG_PAGEALLOC or +* CONFIG_ARCH_HAS_SET_DIRECT_MAP is not set. In that case kernel_page_present() +* always returns 'true'. */ static void safe_copy_page (void *dst,struct page *s_page) {
mm/page_alloc.c +5 -2
@@ -1144,7 +1144,9 @@ static __always_inline bool free_pages_prepare(struct page *page, }arch_free_page (page, order);kernel_poison_pages (page,1 << order,0 ); -kernel_map_pages (page,1 << order,0 ); +if (debug_pagealloc_enabled ()) +kernel_map_pages (page,1 << order,0 ); +kasan_free_nondeferred_pages (page, order);return true;
@@ -2014,7 +2016,8 @@ inline void post_alloc_hook(struct page *page, unsigned int order,set_page_refcounted (page);arch_alloc_page (page, order); -kernel_map_pages (page,1 << order,1 ); +if (debug_pagealloc_enabled ()) +kernel_map_pages (page,1 << order,1 );kasan_alloc_pages (page, order);kernel_poison_pages (page,1 << order,1 );set_page_owner (page, order, gfp_flags);
PATCH v5 16/23 vmalloc: Add flag for free of special permsissions
In reply to Nadav AmitFrom: Rick Edgecombe <[email protected]>
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.
--- include/linux/vmalloc.h | 15 ++++++ mm/vmalloc.c | 113 +++++++++++++++++++++++++++++++++------- 2 files changed, 109 insertions(+), 19 deletions(-)
include/linux/vmalloc.h +15 -0
@@ -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. */
mm/vmalloc.c +94 -19
151 lines not on this page · raw message
Signature · 1 line
2.17.1
tip-bot for Rick Edgecombe
merge bot[tip:x86/mm] mm/vmalloc: Add flag for freeing of special permsissions
mm/vmalloc: Add flag for freeing of special permsissions
Commit-ID: 868b104d7379e28013e9d48bdd2db25e0bdcf751 Gitweb: https://git.kernel.org/tip/868b104d7379e28013e9d48bdd2db25e0bdcf751 Author: Rick Edgecombe <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:36 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:58 +0200
mm/vmalloc: Add flag for freeing of special permsissions
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.
--- include/linux/vmalloc.h | 15 +++++++ mm/vmalloc.c | 113 ++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 109 insertions(+), 19 deletions(-)
include/linux/vmalloc.h +15 -0
@@ -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. */
mm/vmalloc.c +94 -19
151 lines not on this page · raw message
PATCH v5 17/23 modules: Use vmalloc special flag
In reply to Nadav AmitFrom: Rick Edgecombe <[email protected]>
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.
--- kernel/module.c | 77 +++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 38 deletions(-)
kernel/module.c +39 -38
@@ -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);
Signature · 1 line
2.17.1
tip-bot for Rick Edgecombe
merge bot[tip:x86/mm] modules: Use vmalloc special flag
modules: Use vmalloc special flag
Commit-ID: 1a7b7d9220819afe79d1ec5d759fe4349bd2453e Gitweb: https://git.kernel.org/tip/1a7b7d9220819afe79d1ec5d759fe4349bd2453e Author: Rick Edgecombe <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:37 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:58 +0200
modules: Use vmalloc special flag
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.
--- kernel/module.c | 77 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 39 insertions(+), 38 deletions(-)
kernel/module.c +39 -38
@@ -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);
PATCH v5 18/23 bpf: Use vmalloc special flag
In reply to Nadav AmitFrom: Rick Edgecombe <[email protected]>
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.
--- include/linux/filter.h | 17 +++-------------- kernel/bpf/core.c | 1 - 2 files changed, 3 insertions(+), 15 deletions(-)
include/linux/filter.h +3 -14
@@ -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); }
kernel/bpf/core.c +0 -1
@@ -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));
Signature · 1 line
2.17.1
tip-bot for Rick Edgecombe
merge bot[tip:x86/mm] bpf: Use vmalloc special flag
bpf: Use vmalloc special flag
Commit-ID: d53d2f78ceadba081fc7785570798c3c8d50a718 Gitweb: https://git.kernel.org/tip/d53d2f78ceadba081fc7785570798c3c8d50a718 Author: Rick Edgecombe <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:38 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:59 +0200
bpf: Use vmalloc special flag
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.
--- include/linux/filter.h | 17 +++-------------- kernel/bpf/core.c | 1 - 2 files changed, 3 insertions(+), 15 deletions(-)
include/linux/filter.h +3 -14
@@ -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); }
kernel/bpf/core.c +0 -1
@@ -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));
Re: [PATCH v5 18/23] bpf: Use vmalloc special flag
In reply to Nadav AmitHi,
Quoted 180 lines · Nadav Amit
From: Rick Edgecombe <[email protected]>
Use new flag
VM_FLUSH_RESET_PERMSfor 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 <[email protected]>Cc Alexei Starovoitov <[email protected]>Signed-off-by Rick Edgecombe <[email protected]>--- include/linux/filter.h | 17 +++-------------- kernel/bpf/core.c | 1 - 2 files changed, 3 insertions(+), 15 deletions(-)include/linux/filter.h +3
-14@@ -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); }kernel/bpf/core.c +0
-1@@ -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));Signature · 30 lines
2.17.1 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nadav Amit <[email protected]> Subject: [PATCH v5 18/23] bpf: Use vmalloc special flag Date: Thu, 25 Apr 2019 17:11:38 -0700 Message-ID: <[email protected]> In-Reply-To: <[email protected]> References: <[email protected]> MIME-Version: 1.0 Content-Type: text/plain To: Peter Zijlstra <[email protected]>, Borislav Petkov <[email protected]>, Andy Lutomirski <[email protected]>, Ingo Molnar <[email protected]> Cc: [email protected], [email protected], [email protected], Thomas Gleixner <[email protected]>, Nadav Amit <[email protected]>, Dave Hansen <[email protected]>, [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Rick Edgecombe <[email protected]>, Daniel Borkmann <[email protected]>, Alexei Starovoitov <[email protected]> List-ID: <kernel-hardening.lists.openwall.com> From: Rick Edgecombe <[email protected]> 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 <[email protected]> Cc: Alexei Starovoitov <[email protected]> Signed-off-by: Rick Edgecombe <[email protected]> --- include/linux/filter.h | 17 +++-------------- kernel/bpf/core.c | 1 - 2 files changed, 3 insertions(+), 15 deletions(-)include/linux/filter.h +3
-14@@ -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); }kernel/bpf/core.c +0
-0
kernel/bpf/core.c void __weak bpf_jit_free(struct bpf_prog *fp) · line 853
Quoted 7 lines
kernel/bpf/core.c +0
-1@@ -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));
There are issues with the TLB management on sparc64 (primarily sun4u) that were introduced by this patch. A typical backtrace after a crash looks like this:
[ 122.085803] Unable to handle kernel NULL pointer dereference
[ 122.160227] tsk->{mm,active_mm}->context = 000000000000009d
[ 122.233502] tsk->{mm,active_mm}->pgd = fff0000231d14000
[ 122.302118] \|/ ____ \|/
[ 122.302118] "@'/ .. \`@"
[ 122.302118] /_| \__/ |_\
[ 122.302118] \__U_/
[ 122.495420] systemd(1): Oops [#1]
[ 122.538874] CPU: 0 PID: 1 Comm: systemd Not tainted 5.2.0-3-sparc64 #1 Debian 5.2.17-1
[ 122.642957] TSTATE: 0000004411001601 TPC: 000000000061cd94 TNPC: 000000000061cd98 Y: 00000000 Not tainted
[ 122.772207] TPC: <vfs_getattr_nosec+0x34/0xc0>
[ 122.830529] g0: 0000000000000000 g1: 00000000000007ff g2: 0000000000000000 g3: 00000000000007df
[ 122.944902] g4: fff00002381771c0 g5: 0000000000000003 g6: fff0000238178000 g7: 0000000000000000
[ 123.059275] o0: fff000023817be18 o1: 0000000000000000 o2: 0000000000000000 o3: fff000023817be18
[ 123.173658] o4: 0000000000000000 o5: 0000000000000000 sp: fff000023817b341 ret_pc: 000000000061cd7c
[ 123.292611] RPC: <vfs_getattr_nosec+0x1c/0xc0>
[ 123.350933] l0: 0000010000204010 l1: fff0000101600e28 l2: e4e45b5b8ae44628 l3: 0000000000000000
[ 123.465311] l4: 0000000000000000 l5: 0000000000000000 l6: 0000000000000000 l7: fff0000100bff140
[ 123.579692] i0: fff000023817bd50 i1: fff000023817be18 i2: 0000000000000001 i3: 0000000000000900
[ 123.694060] i4: 0000000000000000 i5: fff00002320c1210 i6: fff000023817b3f1 i7: 000000000061ce48
[ 123.808439] I7: <vfs_getattr+0x28/0x40>
[ 123.858759] Call Trace:
[ 123.890785] [000000000061ce48] vfs_getattr+0x28/0x40
[ 123.957123] [000000000061cf64] vfs_statx+0x84/0xc0
[ 124.021173] [000000000061d918] sys_statx+0x38/0x60
[ 124.085226] [0000000000406154] linux_sparc_syscall+0x34/0x44
[ 124.160708] Disabling lock debugging due to kernel taint
[ 124.230481] Caller[000000000061ce48]: vfs_getattr+0x28/0x40
[ 124.303680] Caller[000000000061cf64]: vfs_statx+0x84/0xc0
[ 124.374593] Caller[000000000061d918]: sys_statx+0x38/0x60
[ 124.445503] Caller[0000000000406154]: linux_sparc_syscall+0x34/0x44
[ 124.527857] Caller[fff00001013fde40]: 0xfff00001013fde40
[ 124.597621] Instruction DUMP:
[ 124.597623] c2264000
[ 124.636505] 861027df
[ 124.667386] c45f6028
[ 124.698267] <c458a050>
[ 124.729148] 8408a401
[ 124.760031] 83789403
[ 124.790910] c2264000
[ 124.821801] c207600c
[ 124.852675] 80886800
[ 124.883556]
[ 124.954015] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
[ 125.054721] Press Stop-A (L1-A) from sun keyboard or send break
[ 125.054721] twice on console to return to the boot prom
[ 125.201103] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009 ]---I suspect that the main issue is to be found in the following patch which introduced VM_FLUSH_RESET_PERMS which may not work as expected on sun4u SPARC systems:
commit 868b104d7379e28013e9d48bdd2db25e0bdcf751
Author: Rick Edgecombe <[email protected]> Date: Thu Apr 25 17:11:36 2019 -0700
mm/vmalloc: Add flag for freeing of special permsissions
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.
The crash will always happen when support for transparent huge pages is enabled (CONFIG_TRANSPARENT_HUGEPAGE=y and CONFIG_TRANSPARENT_HUGEPAGE_ALWAYS=y) and in particular on sun4u machines (but not so much the more modern sun4v machines although I cannot rule out that the crashes sometimes happening on these machines is related to this bug).
With THP enabled, the crash can be delayed by either reverting d563d678aa0b or, for example, by this crude hack:
mm/vmalloc.c +2 -2
@@ -2948,8 +2948,8 @@ static void _vm_unmap_aliases(unsigned long start, unsigned long end, int flush) }free_purged_blocks (&purge_list); -if (!__purge_vmap_area_lazy (start, end, false) && flush) -flush_tlb_kernel_range (start, end); +// if (!__purge_vmap_area_lazy(start, end, false) && flush) +// flush_tlb_kernel_range(start, end); mutex_unlock (&vmap_purge_lock); }
Please see also the discussion in [1].
Thanks,
Adrian
Quoted 1 line
[1] https://lore.kernel.org/all/35f5ec4eda8a7dbeeb7df9ec0be5c0b062c509f7.camel@physik.fu-berlin.de/
Signature · 4 lines
.''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [PATCH v5 18/23] bpf: Use vmalloc special flag
Quoted 2 lines · John Paul Adrian Glaubitz
I suspect that the main issue is to be found in the following patch which introduced
VM_FLUSH_RESET_PERMSwhich may not work as expected on sun4u SPARC systems:
I think the problem we found with VM_FLUSH_RESET_PERMS was that the sparc64 kernel TLB flush implementation was broken. Since VM_FLUSH_RESET_PERMS caused kernel TLB flushes to happen sooner, it just showed up sooner. [0]
This other issue seems to be about userspace memory. So I wonder if these are two separate issues? Bisecting to the original VM_FLUSH_RESET_PERMS would have had the known sparc kernel range TLB flush issue. So to bisect the other issue you might need to apply this [1].
[0] https://marc.info/?l=linux-sparc&m=155915694304118&w=2
[1] https://lore.kernel.org/all/[email protected]/
Re: [PATCH v5 18/23] bpf: Use vmalloc special flag
Hi,
Quoted 4 lines · Rick P
Quoted 2 lines · John Paul Adrian Glaubitz
I suspect that the main issue is to be found in the following patch which introduced
VM_FLUSH_RESET_PERMSwhich may not work as expected on sun4u SPARC systems:
Quoted 8 lines
I think the problem we found with
VM_FLUSH_RESET_PERMSwas that the sparc64 kernel TLB flush implementation was broken. SinceVM_FLUSH_RESET_PERMScaused kernel TLB flushes to happen sooner, it just showed up sooner. [0]This other issue seems to be about userspace memory. So I wonder if these are two separate issues? Bisecting to the original
VM_FLUSH_RESET_PERMSwould have had the known sparc kernel range TLB flush issue. So to bisect the other issue you might need to apply this [1].
That could be true. I knew about the patch in [1] but I didn't think of applying it.
FWIW, the crashes we're seeing on recent kernel versions look like this:
[ 40.992851] \|/ ____ \|/ [ 40.992851] "@'/ .. \`@" [ 40.992851] /_| \__/ |_\ [ 40.992851] \__U_/ [ 41.186220] (udev-worker)(88): Kernel illegal instruction [#1] [ 41.262910] CPU: 0 UID: 0 PID: 88 Comm: (udev-worker) Tainted: G W 6.12.0+ #25 [ 41.376151] Tainted: [W]=WARN [ 41.415025] TSTATE: 0000004411001607 TPC: 00000000101c21c0 TNPC: 00000000101c21c4 Y: 00000000 Tainted: G W [ 41.563717] TPC: <ehci_init_driver+0x0/0x160 [ehci_hcd]> [ 41.633584] g0: 00000000012005b8 g1: 00000000100a1800 g2: 0000000010206000 g3: 00000000101de000 [ 41.747962] g4: fff000000a5af380 g5: 0000000000000000 g6: fff000000aac8000 g7: 0000000000000e7b [ 41.862338] o0: 0000000010060118 o1: 000000001020a000 o2: fff000000aa30ce0 o3: 0000000000000e7a [ 41.976728] o4: 00000000ff000000 o5: 00ff000000000000 sp: fff000000aacb091 ret_pc: 00000000101de028 [ 42.095768] RPC: <ehci_pci_init+0x28/0x2000 [ehci_pci]> [ 42.164394] l0: 0000000000000000 l1: 0000000100043fff l2: ffffffffff800000 l3: 0000000000800000 [ 42.278768] l4: fff00000001c8008 l5: 0000000000000000 l6: 00000000013358e0 l7: 0000000001002800 [ 42.393143] i0: ffffffffffffffed i1: 00000000004db8d8 i2: 0000000000000000 i3: fff000000aa304e0 [ 42.507517] i4: 0000000001127250 i5: 0000000010060000 i6: fff000000aacb141 i7: 0000000000427d90 [ 42.621893] I7: <do_one_initcall+0x30/0x200> [ 42.677931] Call Trace: [ 42.709953] [<0000000000427d90>] do_one_initcall+0x30/0x200 [ 42.783158] [<00000000004db908>] do_init_module+0x48/0x240 [ 42.855214] [<00000000004dd82c>] load_module+0x19cc/0x1f20 [ 42.927270] [<00000000004ddf8c>] init_module_from_file+0x6c/0xa0 [ 43.006189] [<00000000004de1e4>] sys_finit_module+0x1c4/0x2c0 [ 43.081677] [<0000000000406174>] linux_sparc_syscall+0x34/0x44 [ 43.158307] Disabling lock debugging due to kernel taint [ 43.228077] Caller[0000000000427d90]: do_one_initcall+0x30/0x200 [ 43.306995] Caller[00000000004db908]: do_init_module+0x48/0x240 [ 43.384772] Caller[00000000004dd82c]: load_module+0x19cc/0x1f20 [ 43.462544] Caller[00000000004ddf8c]: init_module_from_file+0x6c/0xa0 [ 43.547184] Caller[00000000004de1e4]: sys_finit_module+0x1c4/0x2c0 [ 43.628389] Caller[0000000000406174]: linux_sparc_syscall+0x34/0x44 [ 43.710741] Caller[fff000010480e2fc]: 0xfff000010480e2fc [ 43.780508] Instruction DUMP: [ 43.780511] 00000000 [ 43.819394] 00000000 [ 43.850273] 00000000 [ 43.881153] <00000000> [ 43.912036] 00000000 [ 43.942917] 00000000 [ 43.973797] 00000000 [ 44.004678] 00000000 [ 44.035561] 00000000 [ 44.066443]
Do you have any suggestion what to bisect?
Adrian
Signature · 4 lines
.''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [PATCH v5 18/23] bpf: Use vmalloc special flag
Quoted 9 lines · John Paul Adrian Glaubitz
That could be true. I knew about the patch in [1] but I didn't think of applying it.
FWIW, the crashes we're seeing on recent kernel versions look like this:
[ 40.992851] \|/ ____ \|/
[ 40.992851] "@'/ .. \`@"
[ 40.992851] /_| \__/ |_\
[ 40.992851] \__U_/
[ 41.186220] (udev-worker)(88): Kernel illegal instruction [#1]
Possibly re-using some stale TLB executable VA which's page now has other data in it.
Quoted 41 lines
[ 41.262910] CPU: 0 UID: 0 PID: 88 Comm: (udev-worker) Tainted: G W 6.12.0+ #25
[ 41.376151] Tainted: [W]=WARN
[ 41.415025] TSTATE: 0000004411001607 TPC:00000000101c21c0TNPC:00000000101c21c4Y: 00000000 Tainted: G W [ 41.563717] TPC: <ehci_init_driver+0x0/0x160[ehci_hcd]>
[ 41.633584] g0:00000000012005b8g1:00000000100a1800g2: 0000000010206000 g3:00000000101de000
[ 41.747962] g4:fff000000a5af380g5: 0000000000000000 g6:fff000000aac8000g7:0000000000000e7b
[ 41.862338] o0: 0000000010060118 o1:000000001020a000o2:fff000000aa30ce0o3:0000000000000e7a
[ 41.976728] o4:00000000ff000000o5:00ff000000000000sp:fff000000aacb091ret_pc:00000000101de028[ 42.095768] RPC: <ehci_pci_init+0x28/0x2000[ehci_pci]>
[ 42.164394] l0: 0000000000000000 l1:0000000100043fffl2:ffffffffff800000l3: 0000000000800000
[ 42.278768] l4:fff00000001c8008l5: 0000000000000000 l6:00000000013358e0l7: 0000000001002800
[ 42.393143] i0: ffffffffffffffed i1:00000000004db8d8i2: 0000000000000000 i3:fff000000aa304e0
[ 42.507517] i4: 0000000001127250 i5: 0000000010060000 i6:fff000000aacb141i7:0000000000427d90
[ 42.621893] I7: <do_one_initcall+0x30/0x200>
[ 42.677931] Call Trace:
[ 42.709953] [<0000000000427d90>]do_one_initcall+0x30/0x200
[ 42.783158] [<00000000004db908>]do_init_module+0x48/0x240
[ 42.855214] [<00000000004dd82c>]load_module+0x19cc/0x1f20
[ 42.927270] [<00000000004ddf8c>]init_module_from_file+0x6c/0xa0
[ 43.006189] [<00000000004de1e4>]sys_finit_module+0x1c4/0x2c0
[ 43.081677] [<0000000000406174>]linux_sparc_syscall+0x34/0x44
[ 43.158307] Disabling lock debugging due to kernel taint
[ 43.228077] Caller[0000000000427d90]:do_one_initcall+0x30/0x200
[ 43.306995] Caller[00000000004db908]:do_init_module+0x48/0x240
[ 43.384772] Caller[00000000004dd82c]:load_module+0x19cc/0x1f20
[ 43.462544] Caller[00000000004ddf8c]:init_module_from_file+0x6c/0xa0
[ 43.547184] Caller[00000000004de1e4]:sys_finit_module+0x1c4/0x2c0
[ 43.628389] Caller[0000000000406174]:linux_sparc_syscall+0x34/0x44
[ 43.710741] Caller[fff000010480e2fc]:0xfff000010480e2fc
[ 43.780508] Instruction DUMP:
[ 43.780511] 00000000
[ 43.819394] 00000000
[ 43.850273] 00000000
[ 43.881153] <00000000>
[ 43.912036] 00000000
[ 43.942917] 00000000
[ 43.973797] 00000000
[ 44.004678] 00000000
[ 44.035561] 00000000
[ 44.066443]
Quoted 1 line
Do you have any suggestion what to bisect?
This does look like kernel range TLB flush related. Not sure how it's related to userspace huge pages. Perhaps the userspace range TLB flush has issues to? Or the TLB flush asm needs to be fixed in this another sparc variant?
So far two issues were found with that patch and they were both rare architectures with broken kernel TLB flushes. Kernel TLB flushes can actually not be required for a long time, so probably the bug normally looked like unexplained crashes after days. The VM_FLUSH_RESET_PERMS just made them show up earlier in a bisectable way.
Re: [PATCH v5 18/23] bpf: Use vmalloc special flag
Quoted 5 lines · Rick P
Quoted 4 lines · John Paul Adrian Glaubitz
That could be true. I knew about the patch in [1] but I didn't think of applying it.
FWIW, the crashes we're seeing on recent kernel versions look like this:
Quoted 8 lines
Quoted 5 lines
[ 40.992851] \|/ ____ \|/
[ 40.992851] "@'/ .. \`@"
[ 40.992851] /_| \__/ |_\
[ 40.992851] \__U_/
[ 41.186220] (udev-worker)(88): Kernel illegal instruction [#1]Possibly re-using some stale TLB executable VA which's page now has other data in it.
Makes sense given the memory is actually zero'd out.
Quoted 41 lines
Quoted 41 lines
[ 41.262910] CPU: 0 UID: 0 PID: 88 Comm: (udev-worker) Tainted: G W 6.12.0+ #25
[ 41.376151] Tainted: [W]=WARN
[ 41.415025] TSTATE: 0000004411001607 TPC:00000000101c21c0TNPC:00000000101c21c4Y: 00000000 Tainted: G W [ 41.563717] TPC: <ehci_init_driver+0x0/0x160[ehci_hcd]>
[ 41.633584] g0:00000000012005b8g1:00000000100a1800g2: 0000000010206000 g3:00000000101de000
[ 41.747962] g4:fff000000a5af380g5: 0000000000000000 g6:fff000000aac8000g7:0000000000000e7b
[ 41.862338] o0: 0000000010060118 o1:000000001020a000o2:fff000000aa30ce0o3:0000000000000e7a
[ 41.976728] o4:00000000ff000000o5:00ff000000000000sp:fff000000aacb091ret_pc:00000000101de028[ 42.095768] RPC: <ehci_pci_init+0x28/0x2000[ehci_pci]>
[ 42.164394] l0: 0000000000000000 l1:0000000100043fffl2:ffffffffff800000l3: 0000000000800000
[ 42.278768] l4:fff00000001c8008l5: 0000000000000000 l6:00000000013358e0l7: 0000000001002800
[ 42.393143] i0: ffffffffffffffed i1:00000000004db8d8i2: 0000000000000000 i3:fff000000aa304e0
[ 42.507517] i4: 0000000001127250 i5: 0000000010060000 i6:fff000000aacb141i7:0000000000427d90
[ 42.621893] I7: <do_one_initcall+0x30/0x200>
[ 42.677931] Call Trace:
[ 42.709953] [<0000000000427d90>]do_one_initcall+0x30/0x200
[ 42.783158] [<00000000004db908>]do_init_module+0x48/0x240
[ 42.855214] [<00000000004dd82c>]load_module+0x19cc/0x1f20
[ 42.927270] [<00000000004ddf8c>]init_module_from_file+0x6c/0xa0
[ 43.006189] [<00000000004de1e4>]sys_finit_module+0x1c4/0x2c0
[ 43.081677] [<0000000000406174>]linux_sparc_syscall+0x34/0x44
[ 43.158307] Disabling lock debugging due to kernel taint
[ 43.228077] Caller[0000000000427d90]:do_one_initcall+0x30/0x200
[ 43.306995] Caller[00000000004db908]:do_init_module+0x48/0x240
[ 43.384772] Caller[00000000004dd82c]:load_module+0x19cc/0x1f20
[ 43.462544] Caller[00000000004ddf8c]:init_module_from_file+0x6c/0xa0
[ 43.547184] Caller[00000000004de1e4]:sys_finit_module+0x1c4/0x2c0
[ 43.628389] Caller[0000000000406174]:linux_sparc_syscall+0x34/0x44
[ 43.710741] Caller[fff000010480e2fc]:0xfff000010480e2fc
[ 43.780508] Instruction DUMP:
[ 43.780511] 00000000
[ 43.819394] 00000000
[ 43.850273] 00000000
[ 43.881153] <00000000>
[ 43.912036] 00000000
[ 43.942917] 00000000
[ 43.973797] 00000000
[ 44.004678] 00000000
[ 44.035561] 00000000
[ 44.066443]
Quoted 5 lines
Quoted 1 line
Do you have any suggestion what to bisect?
This does look like kernel range TLB flush related. Not sure how it's related to userspace huge pages. Perhaps the userspace range TLB flush has issues to? Or the TLB flush asm needs to be fixed in this another sparc variant?
The patch you previously linked actually fixed this particular SPARC variant which is sun4u, i.e. the non-hypervisor variant with sun4v being the hypervisor one.
I was already thinking that the fix in d3c976c14ad8 was possible incomplete.
Quoted 5 lines
So far two issues were found with that patch and they were both rare architectures with broken kernel TLB flushes. Kernel TLB flushes can actually not be required for a long time, so probably the bug normally looked like unexplained crashes after days. The
VM_FLUSH_RESET_PERMSjust made them show up earlier in a bisectable way.
Yeah, I think that could actually be the case.
I wonder whether I can revert both d3c976c14ad8 and a74ad5e660a9 on a current tree and see if that fixes the bug.
Adrian
Signature · 4 lines
.''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
PATCH v5 19/23 x86/ftrace: Use vmalloc special flag
In reply to Nadav AmitFrom: Rick Edgecombe <[email protected]>
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.
--- arch/x86/kernel/ftrace.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
arch/x86/kernel/ftrace.c +6 -8
@@ -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 ; }
Signature · 1 line
2.17.1
tip-bot for Rick Edgecombe
merge bot[tip:x86/mm] x86/ftrace: Use vmalloc special flag
x86/ftrace: Use vmalloc special flag
Commit-ID: 7fdfe1e40b225b1d163f9afed2fa3f04442dbaad Gitweb: https://git.kernel.org/tip/7fdfe1e40b225b1d163f9afed2fa3f04442dbaad Author: Rick Edgecombe <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:39 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:38:00 +0200
x86/ftrace: Use vmalloc special flag
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.
--- arch/x86/kernel/ftrace.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
arch/x86/kernel/ftrace.c +6 -8
@@ -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 ; }
PATCH v5 20/23 x86/kprobes: Use vmalloc special flag
In reply to Nadav AmitFrom: Rick Edgecombe <[email protected]>
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.
--- arch/x86/kernel/kprobes/core.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
arch/x86/kernel/kprobes/core.c +1 -6
@@ -434,6 +434,7 @@ void *alloc_insn_page(void)if (!page)return NULL ; +set_vm_flush_reset_perms (page);/* * First make the page read-only, and only then make it executable to * prevent it from being W+X in between.
@@ -452,12 +453,6 @@ void *alloc_insn_page(void)/* Recover page to RW mode before releasing it */ void free_insn_page (void *page) { -/* -* 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); }
Signature · 1 line
2.17.1
tip-bot for Rick Edgecombe
merge bot[tip:x86/mm] x86/kprobes: Use vmalloc special flag
x86/kprobes: Use vmalloc special flag
Commit-ID: 241a1f22380646bc4d1dd18e5bc246877513da68 Gitweb: https://git.kernel.org/tip/241a1f22380646bc4d1dd18e5bc246877513da68 Author: Rick Edgecombe <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:40 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:38:01 +0200
x86/kprobes: Use vmalloc special flag
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.
--- arch/x86/kernel/kprobes/core.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
arch/x86/kernel/kprobes/core.c +1 -6
@@ -434,6 +434,7 @@ void *alloc_insn_page(void)if (!page)return NULL ; +set_vm_flush_reset_perms (page);/* * First make the page read-only, and only then make it executable to * prevent it from being W+X in between.
@@ -452,12 +453,6 @@ void *alloc_insn_page(void)/* Recover page to RW mode before releasing it */ void free_insn_page (void *page) { -/* -* 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); }
PATCH v5 21/23 x86/alternative: Comment about module removal races
In reply to Nadav AmitAdd a comment to clarify that users of text_poke() must ensure that no races with module removal take place.
--- arch/x86/kernel/alternative.c | 5 +++++ 1 file changed, 5 insertions(+)
arch/x86/kernel/alternative.c +5 -0
@@ -810,6 +810,11 @@ static void *__text_poke(void *addr, const void *opcode, size_t len)* 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. +* +* Note that the caller must ensure that if the modified code is part of a +* module, the module would not be removed during poking. This can be achieved +* by registering a module notifier, and ordering module removal and patching +* trough a mutex. */ void *text_poke (void *addr,const void *opcode,size_t len) {
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] x86/alternatives: Add comment about module removal races
x86/alternatives: Add comment about module removal races
Commit-ID: 3950746d9d8ef981c1cb842384e0e86e8d1aad76 Gitweb: https://git.kernel.org/tip/3950746d9d8ef981c1cb842384e0e86e8d1aad76 Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:41 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:38:01 +0200
x86/alternatives: Add comment about module removal races
Add a comment to clarify that users of text_poke() must ensure that no races with module removal take place.
--- arch/x86/kernel/alternative.c | 5 +++++ 1 file changed, 5 insertions(+)
arch/x86/kernel/alternative.c +5 -0
@@ -810,6 +810,11 @@ static void *__text_poke(void *addr, const void *opcode, size_t len)* 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. +* +* Note that the caller must ensure that if the modified code is part of a +* module, the module would not be removed during poking. This can be achieved +* by registering a module notifier, and ordering module removal and patching +* trough a mutex. */ void *text_poke (void *addr,const void *opcode,size_t len) {
PATCH v5 22/23 mm/tlb: Provide default nmi_uaccess_okay()
In reply to Nadav Amitx86 has an nmi_uaccess_okay(), but other architectures do not. Arch-independent code might need to know whether access to user addresses is ok in an NMI context or in other code whose execution context is unknown. Specifically, this function is needed for bpf_probe_write_user().
Add a default implementation of nmi_uaccess_okay() for architectures that do not have such a function.
--- arch/x86/include/asm/tlbflush.h | 2 ++ include/asm-generic/tlb.h | 9 +++++++++ 2 files changed, 11 insertions(+)
arch/x86/include/asm/tlbflush.h +2 -0
@@ -274,6 +274,8 @@ static inline bool nmi_uaccess_okay(void)return true; } +#define nmi_uaccess_okay nmi_uaccess_okay +/* Initialize cr4 shadow for this CPU. */ static inline void cr4_init_shadow (void ) {
include/asm-generic/tlb.h +9 -0
@@ -21,6 +21,15 @@#include <asm/tlbflush.h> #include <asm/cacheflush.h> +/* +* Blindly accessing user memory from NMI context can be dangerous +* if we're in the middle of switching the current user task or switching +* the loaded mm. +*/ +#ifndef nmi_uaccess_okay +# define nmi_uaccess_okay () true +#endif +#ifdef CONFIG_MMU /*
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] mm/tlb: Provide default nmi_uaccess_okay()
mm/tlb: Provide default nmi_uaccess_okay()
Commit-ID: 5932c9fd19e6e5ac84756c5c32fe5155d9a6b458 Gitweb: https://git.kernel.org/tip/5932c9fd19e6e5ac84756c5c32fe5155d9a6b458 Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:42 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:48 +0200
mm/tlb: Provide default nmi_uaccess_okay()
x86 has an nmi_uaccess_okay(), but other architectures do not. Arch-independent code might need to know whether access to user addresses is ok in an NMI context or in other code whose execution context is unknown. Specifically, this function is needed for bpf_probe_write_user().
Add a default implementation of nmi_uaccess_okay() for architectures that do not have such a function.
--- arch/x86/include/asm/tlbflush.h | 2 ++ include/asm-generic/tlb.h | 9 +++++++++ 2 files changed, 11 insertions(+)
arch/x86/include/asm/tlbflush.h +2 -0
@@ -274,6 +274,8 @@ static inline bool nmi_uaccess_okay(void)return true; } +#define nmi_uaccess_okay nmi_uaccess_okay +/* Initialize cr4 shadow for this CPU. */ static inline void cr4_init_shadow (void ) {
include/asm-generic/tlb.h +9 -0
@@ -20,6 +20,15 @@#include <asm/pgalloc.h> #include <asm/tlbflush.h> +/* +* Blindly accessing user memory from NMI context can be dangerous +* if we're in the middle of switching the current user task or switching +* the loaded mm. +*/ +#ifndef nmi_uaccess_okay +# define nmi_uaccess_okay () true +#endif +#ifdef CONFIG_MMU #ifdef CONFIG_HAVE_RCU_TABLE_FREE
PATCH v5 23/23 bpf: Fail bpf_probe_write_user() while mm is switched
In reply to Nadav AmitWhen using a temporary mm, bpf_probe_write_user() should not be able to write to user memory, since user memory addresses may be used to map kernel memory. Detect these cases and fail bpf_probe_write_user() in such cases.
--- kernel/trace/bpf_trace.c | 8 ++++++++ 1 file changed, 8 insertions(+)
kernel/trace/bpf_trace.c +8 -0
@@ -14,6 +14,8 @@#include <linux/syscalls.h> #include <linux/error-injection.h> +#include <asm/tlb.h> +#include "trace_probe.h" #include "trace.h"
@@ -163,6 +165,10 @@ BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,* access_ok() should prevent writing to non-user memory, but in * some situations (nommu, temporary switch, etc) access_ok() does * not provide enough validation, hence the check on KERNEL_DS. +* +* nmi_uaccess_okay() ensures the probe is not run in an interim +* state, when the task or mm are switched. This is specifically +* required to prevent the use of temporary mm. */ if (unlikely(in_interrupt() ||
@@ -170,6 +176,8 @@ BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,return -EPERM ;if (unlikely (uaccess_kernel ()))return -EPERM ; +if (unlikely (!nmi_uaccess_okay ())) +return -EPERM ;if (!access_ok (unsafe_ptr, size))return -EPERM ;
Signature · 1 line
2.17.1
tip-bot for Nadav Amit
merge bot[tip:x86/mm] bpf: Fail bpf_probe_write_user() while mm is switched
bpf: Fail bpf_probe_write_user() while mm is switched
Commit-ID: c7b6f29b6257532792fc722b68fcc0e00b5a856c Gitweb: https://git.kernel.org/tip/c7b6f29b6257532792fc722b68fcc0e00b5a856c Author: Nadav Amit <[email protected]> AuthorDate: Thu, 25 Apr 2019 17:11:43 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 30 Apr 2019 12:37:48 +0200
bpf: Fail bpf_probe_write_user() while mm is switched
When using a temporary mm, bpf_probe_write_user() should not be able to write to user memory, since user memory addresses may be used to map kernel memory. Detect these cases and fail bpf_probe_write_user() in such cases.
--- kernel/trace/bpf_trace.c | 8 ++++++++ 1 file changed, 8 insertions(+)
kernel/trace/bpf_trace.c +8 -0
@@ -14,6 +14,8 @@#include <linux/syscalls.h> #include <linux/error-injection.h> +#include <asm/tlb.h> +#include "trace_probe.h" #include "trace.h"
@@ -163,6 +165,10 @@ BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,* access_ok() should prevent writing to non-user memory, but in * some situations (nommu, temporary switch, etc) access_ok() does * not provide enough validation, hence the check on KERNEL_DS. +* +* nmi_uaccess_okay() ensures the probe is not run in an interim +* state, when the task or mm are switched. This is specifically +* required to prevent the use of temporary mm. */ if (unlikely(in_interrupt() ||
@@ -170,6 +176,8 @@ BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,return -EPERM ;if (unlikely (uaccess_kernel ()))return -EPERM ; +if (unlikely (!nmi_uaccess_okay ())) +return -EPERM ;if (!access_ok (unsafe_ptr, size))return -EPERM ;
Peter Zijlstra
maintainerQuoted 2 lines · Nadav Amit
Yet another version, per PeterZ request, addressing the latest feedback.
Well, I would've been OK with just an updated to the one patch, but thanks. Let me see if I can get all that to apply :-)