| Message ID | 20210713053113.4632-6-cmr@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Use per-CPU temporary mappings for patching on Radix MMU | expand |
| Related | show |
Le 13/07/2021 à 07:31, Christopher M. Riedl a écrit : > x86 supports the notion of a temporary mm which restricts access to > temporary PTEs to a single CPU. A temporary mm is useful for situations > where a CPU needs to perform sensitive operations (such as patching a > STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing > said mappings to other CPUs. Another benefit is that other CPU TLBs do > not need to be flushed when the temporary mm is torn down. > > Mappings in the temporary mm can be set in the userspace portion of the > address-space. > > Interrupts must be disabled while the temporary mm is in use. HW > breakpoints, which may have been set by userspace as watchpoints on > addresses now within the temporary mm, are saved and disabled when > loading the temporary mm. The HW breakpoints are restored when unloading > the temporary mm. All HW breakpoints are indiscriminately disabled while > the temporary mm is in use. Can you explain more about that breakpoint stuff ? Why is it a special case here at all ? Isn't it the same when you switch from one user task to another one ? x86 commit doesn't say anythink about breakpoints. > > Based on x86 implementation: > > commit cefa929c034e > ("x86/mm: Introduce temporary mm structs") > > Signed-off-by: Christopher M. Riedl <cmr@linux.ibm.com> > > --- > > v5: * Drop support for using a temporary mm on Book3s64 Hash MMU. > > v4: * Pass the prev mm instead of NULL to switch_mm_irqs_off() when > using/unusing the temp mm as suggested by Jann Horn to keep > the context.active counter in-sync on mm/nohash. > * Disable SLB preload in the temporary mm when initializing the > temp_mm struct. > * Include asm/debug.h header to fix build issue with > ppc44x_defconfig. > --- > arch/powerpc/include/asm/debug.h | 1 + > arch/powerpc/kernel/process.c | 5 +++ > arch/powerpc/lib/code-patching.c | 56 ++++++++++++++++++++++++++++++++ > 3 files changed, 62 insertions(+) > > diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h > index 86a14736c76c3..dfd82635ea8b3 100644 > --- a/arch/powerpc/include/asm/debug.h > +++ b/arch/powerpc/include/asm/debug.h > @@ -46,6 +46,7 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; } > #endif > > void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk); > +void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk); > bool ppc_breakpoint_available(void); > #ifdef CONFIG_PPC_ADV_DEBUG_REGS > extern void do_send_trap(struct pt_regs *regs, unsigned long address, > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c > index 185beb2905801..a0776200772e8 100644 > --- a/arch/powerpc/kernel/process.c > +++ b/arch/powerpc/kernel/process.c > @@ -865,6 +865,11 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk) > return 0; > } > > +void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk) > +{ > + memcpy(brk, this_cpu_ptr(¤t_brk[nr]), sizeof(*brk)); > +} > + > void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk) > { > memcpy(this_cpu_ptr(¤t_brk[nr]), brk, sizeof(*brk)); > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c > index 54b6157d44e95..3122d8e4cc013 100644 > --- a/arch/powerpc/lib/code-patching.c > +++ b/arch/powerpc/lib/code-patching.c > @@ -17,6 +17,9 @@ > #include <asm/code-patching.h> > #include <asm/setup.h> > #include <asm/inst.h> > +#include <asm/mmu_context.h> > +#include <asm/debug.h> > +#include <asm/tlb.h> > > static int __patch_instruction(u32 *exec_addr, struct ppc_inst instr, u32 *patch_addr) > { > @@ -45,6 +48,59 @@ int raw_patch_instruction(u32 *addr, struct ppc_inst instr) > } > > #ifdef CONFIG_STRICT_KERNEL_RWX > + > +struct temp_mm { > + struct mm_struct *temp; > + struct mm_struct *prev; > + struct arch_hw_breakpoint brk[HBP_NUM_MAX]; > +}; > + > +static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm) > +{ > + /* We currently only support temporary mm on the Book3s64 Radix MMU */ > + WARN_ON(!radix_enabled()); > + > + temp_mm->temp = mm; > + temp_mm->prev = NULL; > + memset(&temp_mm->brk, 0, sizeof(temp_mm->brk)); > +} > + > +static inline void use_temporary_mm(struct temp_mm *temp_mm) > +{ > + lockdep_assert_irqs_disabled(); > + > + temp_mm->prev = current->active_mm; > + switch_mm_irqs_off(temp_mm->prev, temp_mm->temp, current); > + > + WARN_ON(!mm_is_thread_local(temp_mm->temp)); > + > + if (ppc_breakpoint_available()) { > + struct arch_hw_breakpoint null_brk = {0}; > + int i = 0; > + > + for (; i < nr_wp_slots(); ++i) { > + __get_breakpoint(i, &temp_mm->brk[i]); > + if (temp_mm->brk[i].type != 0) > + __set_breakpoint(i, &null_brk); > + } > + } > +} > + > +static inline void unuse_temporary_mm(struct temp_mm *temp_mm) not sure about the naming. Maybe start_using_temp_mm() and stop_using_temp_mm() would be more explicit. > +{ > + lockdep_assert_irqs_disabled(); > + > + switch_mm_irqs_off(temp_mm->temp, temp_mm->prev, current); > + > + if (ppc_breakpoint_available()) { > + int i = 0; > + > + for (; i < nr_wp_slots(); ++i) > + if (temp_mm->brk[i].type != 0) > + __set_breakpoint(i, &temp_mm->brk[i]); > + } > +} > + > static DEFINE_PER_CPU(struct vm_struct *, text_poke_area); > > #if IS_BUILTIN(CONFIG_LKDTM) > You'll probably get a bisecting hasard with those unused 'static inline' functions in a .c file because that patch alone probably fails build.
On Thu Aug 5, 2021 at 4:27 AM CDT, Christophe Leroy wrote: > > > Le 13/07/2021 à 07:31, Christopher M. Riedl a écrit : > > x86 supports the notion of a temporary mm which restricts access to > > temporary PTEs to a single CPU. A temporary mm is useful for situations > > where a CPU needs to perform sensitive operations (such as patching a > > STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing > > said mappings to other CPUs. Another benefit is that other CPU TLBs do > > not need to be flushed when the temporary mm is torn down. > > > > Mappings in the temporary mm can be set in the userspace portion of the > > address-space. > > > > Interrupts must be disabled while the temporary mm is in use. HW > > breakpoints, which may have been set by userspace as watchpoints on > > addresses now within the temporary mm, are saved and disabled when > > loading the temporary mm. The HW breakpoints are restored when unloading > > the temporary mm. All HW breakpoints are indiscriminately disabled while > > the temporary mm is in use. > > Can you explain more about that breakpoint stuff ? Why is it a special > case here at all ? Isn't it > the same when you switch from one user task to another one ? x86 commit > doesn't say anythink about > breakpoints. > We do not check if the breakpoint is on a kernel address (perf can do this IIUC) and just disable all of them. I had to dig, but x86 has a comment with their implementation at arch/x86/kernel/alternative.c:743. I can reword that part of the commit message if it's unclear. > > > > Based on x86 implementation: > > > > commit cefa929c034e > > ("x86/mm: Introduce temporary mm structs") > > > > Signed-off-by: Christopher M. Riedl <cmr@linux.ibm.com> > > > > --- > > > > v5: * Drop support for using a temporary mm on Book3s64 Hash MMU. > > > > v4: * Pass the prev mm instead of NULL to switch_mm_irqs_off() when > > using/unusing the temp mm as suggested by Jann Horn to keep > > the context.active counter in-sync on mm/nohash. > > * Disable SLB preload in the temporary mm when initializing the > > temp_mm struct. > > * Include asm/debug.h header to fix build issue with > > ppc44x_defconfig. > > --- > > arch/powerpc/include/asm/debug.h | 1 + > > arch/powerpc/kernel/process.c | 5 +++ > > arch/powerpc/lib/code-patching.c | 56 ++++++++++++++++++++++++++++++++ > > 3 files changed, 62 insertions(+) > > > > diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h > > index 86a14736c76c3..dfd82635ea8b3 100644 > > --- a/arch/powerpc/include/asm/debug.h > > +++ b/arch/powerpc/include/asm/debug.h > > @@ -46,6 +46,7 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; } > > #endif > > > > void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk); > > +void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk); > > bool ppc_breakpoint_available(void); > > #ifdef CONFIG_PPC_ADV_DEBUG_REGS > > extern void do_send_trap(struct pt_regs *regs, unsigned long address, > > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c > > index 185beb2905801..a0776200772e8 100644 > > --- a/arch/powerpc/kernel/process.c > > +++ b/arch/powerpc/kernel/process.c > > @@ -865,6 +865,11 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk) > > return 0; > > } > > > > +void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk) > > +{ > > + memcpy(brk, this_cpu_ptr(¤t_brk[nr]), sizeof(*brk)); > > +} > > + > > void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk) > > { > > memcpy(this_cpu_ptr(¤t_brk[nr]), brk, sizeof(*brk)); > > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c > > index 54b6157d44e95..3122d8e4cc013 100644 > > --- a/arch/powerpc/lib/code-patching.c > > +++ b/arch/powerpc/lib/code-patching.c > > @@ -17,6 +17,9 @@ > > #include <asm/code-patching.h> > > #include <asm/setup.h> > > #include <asm/inst.h> > > +#include <asm/mmu_context.h> > > +#include <asm/debug.h> > > +#include <asm/tlb.h> > > > > static int __patch_instruction(u32 *exec_addr, struct ppc_inst instr, u32 *patch_addr) > > { > > @@ -45,6 +48,59 @@ int raw_patch_instruction(u32 *addr, struct ppc_inst instr) > > } > > > > #ifdef CONFIG_STRICT_KERNEL_RWX > > + > > +struct temp_mm { > > + struct mm_struct *temp; > > + struct mm_struct *prev; > > + struct arch_hw_breakpoint brk[HBP_NUM_MAX]; > > +}; > > + > > +static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm) > > +{ > > + /* We currently only support temporary mm on the Book3s64 Radix MMU */ > > + WARN_ON(!radix_enabled()); > > + > > + temp_mm->temp = mm; > > + temp_mm->prev = NULL; > > + memset(&temp_mm->brk, 0, sizeof(temp_mm->brk)); > > +} > > + > > +static inline void use_temporary_mm(struct temp_mm *temp_mm) > > +{ > > + lockdep_assert_irqs_disabled(); > > + > > + temp_mm->prev = current->active_mm; > > + switch_mm_irqs_off(temp_mm->prev, temp_mm->temp, current); > > + > > + WARN_ON(!mm_is_thread_local(temp_mm->temp)); > > + > > + if (ppc_breakpoint_available()) { > > + struct arch_hw_breakpoint null_brk = {0}; > > + int i = 0; > > + > > + for (; i < nr_wp_slots(); ++i) { > > + __get_breakpoint(i, &temp_mm->brk[i]); > > + if (temp_mm->brk[i].type != 0) > > + __set_breakpoint(i, &null_brk); > > + } > > + } > > +} > > + > > +static inline void unuse_temporary_mm(struct temp_mm *temp_mm) > > not sure about the naming. > > Maybe start_using_temp_mm() and stop_using_temp_mm() would be more > explicit. > Hehe I think we've discussed this before - naming things is hard :) I'll take your suggestions for the next spin. > > > +{ > > + lockdep_assert_irqs_disabled(); > > + > > + switch_mm_irqs_off(temp_mm->temp, temp_mm->prev, current); > > + > > + if (ppc_breakpoint_available()) { > > + int i = 0; > > + > > + for (; i < nr_wp_slots(); ++i) > > + if (temp_mm->brk[i].type != 0) > > + __set_breakpoint(i, &temp_mm->brk[i]); > > + } > > +} > > + > > static DEFINE_PER_CPU(struct vm_struct *, text_poke_area); > > > > #if IS_BUILTIN(CONFIG_LKDTM) > > > > You'll probably get a bisecting hasard with those unused 'static inline' > functions in a .c file > because that patch alone probably fails build. I just built the patch without any issue. The compiler only complains for unused 'static' (non-inline) functions right?
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h index 86a14736c76c3..dfd82635ea8b3 100644 --- a/arch/powerpc/include/asm/debug.h +++ b/arch/powerpc/include/asm/debug.h @@ -46,6 +46,7 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; } #endif void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk); +void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk); bool ppc_breakpoint_available(void); #ifdef CONFIG_PPC_ADV_DEBUG_REGS extern void do_send_trap(struct pt_regs *regs, unsigned long address, diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index 185beb2905801..a0776200772e8 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -865,6 +865,11 @@ static inline int set_breakpoint_8xx(struct arch_hw_breakpoint *brk) return 0; } +void __get_breakpoint(int nr, struct arch_hw_breakpoint *brk) +{ + memcpy(brk, this_cpu_ptr(¤t_brk[nr]), sizeof(*brk)); +} + void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk) { memcpy(this_cpu_ptr(¤t_brk[nr]), brk, sizeof(*brk)); diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c index 54b6157d44e95..3122d8e4cc013 100644 --- a/arch/powerpc/lib/code-patching.c +++ b/arch/powerpc/lib/code-patching.c @@ -17,6 +17,9 @@ #include <asm/code-patching.h> #include <asm/setup.h> #include <asm/inst.h> +#include <asm/mmu_context.h> +#include <asm/debug.h> +#include <asm/tlb.h> static int __patch_instruction(u32 *exec_addr, struct ppc_inst instr, u32 *patch_addr) { @@ -45,6 +48,59 @@ int raw_patch_instruction(u32 *addr, struct ppc_inst instr) } #ifdef CONFIG_STRICT_KERNEL_RWX + +struct temp_mm { + struct mm_struct *temp; + struct mm_struct *prev; + struct arch_hw_breakpoint brk[HBP_NUM_MAX]; +}; + +static inline void init_temp_mm(struct temp_mm *temp_mm, struct mm_struct *mm) +{ + /* We currently only support temporary mm on the Book3s64 Radix MMU */ + WARN_ON(!radix_enabled()); + + temp_mm->temp = mm; + temp_mm->prev = NULL; + memset(&temp_mm->brk, 0, sizeof(temp_mm->brk)); +} + +static inline void use_temporary_mm(struct temp_mm *temp_mm) +{ + lockdep_assert_irqs_disabled(); + + temp_mm->prev = current->active_mm; + switch_mm_irqs_off(temp_mm->prev, temp_mm->temp, current); + + WARN_ON(!mm_is_thread_local(temp_mm->temp)); + + if (ppc_breakpoint_available()) { + struct arch_hw_breakpoint null_brk = {0}; + int i = 0; + + for (; i < nr_wp_slots(); ++i) { + __get_breakpoint(i, &temp_mm->brk[i]); + if (temp_mm->brk[i].type != 0) + __set_breakpoint(i, &null_brk); + } + } +} + +static inline void unuse_temporary_mm(struct temp_mm *temp_mm) +{ + lockdep_assert_irqs_disabled(); + + switch_mm_irqs_off(temp_mm->temp, temp_mm->prev, current); + + if (ppc_breakpoint_available()) { + int i = 0; + + for (; i < nr_wp_slots(); ++i) + if (temp_mm->brk[i].type != 0) + __set_breakpoint(i, &temp_mm->brk[i]); + } +} + static DEFINE_PER_CPU(struct vm_struct *, text_poke_area); #if IS_BUILTIN(CONFIG_LKDTM)
x86 supports the notion of a temporary mm which restricts access to temporary PTEs to a single CPU. A temporary mm is useful for situations where a CPU needs to perform sensitive operations (such as patching a STRICT_KERNEL_RWX kernel) requiring temporary mappings without exposing said mappings to other CPUs. Another benefit is that other CPU TLBs do not need to be flushed when the temporary mm is torn down. Mappings in the temporary mm can be set in the userspace portion of the address-space. Interrupts must be disabled while the temporary mm is in use. HW breakpoints, which may have been set by userspace as watchpoints on addresses now within the temporary mm, are saved and disabled when loading the temporary mm. The HW breakpoints are restored when unloading the temporary mm. All HW breakpoints are indiscriminately disabled while the temporary mm is in use. Based on x86 implementation: commit cefa929c034e ("x86/mm: Introduce temporary mm structs") Signed-off-by: Christopher M. Riedl <cmr@linux.ibm.com> --- v5: * Drop support for using a temporary mm on Book3s64 Hash MMU. v4: * Pass the prev mm instead of NULL to switch_mm_irqs_off() when using/unusing the temp mm as suggested by Jann Horn to keep the context.active counter in-sync on mm/nohash. * Disable SLB preload in the temporary mm when initializing the temp_mm struct. * Include asm/debug.h header to fix build issue with ppc44x_defconfig. --- arch/powerpc/include/asm/debug.h | 1 + arch/powerpc/kernel/process.c | 5 +++ arch/powerpc/lib/code-patching.c | 56 ++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+)