> On Apr 25, 2019, at 9:26 AM, Borislav Petkov <[email protected]> wrote: > > On Mon, Apr 22, 2019 at 11:57:45AM -0700, Rick Edgecombe wrote: >> From: 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 ] >> >> Cc: Kees Cook <[email protected]> >> Cc: Dave Hansen <[email protected]> >> Acked-by: Peter Zijlstra (Intel) <[email protected]> >> Reviewed-by: Masami Hiramatsu <[email protected]> >> Tested-by: Masami Hiramatsu <[email protected]> >> Signed-off-by: Andy Lutomirski <[email protected]> >> Signed-off-by: Nadav Amit <[email protected]> >> Signed-off-by: Rick Edgecombe <[email protected]> >> --- >> arch/x86/include/asm/mmu_context.h | 33 ++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/arch/x86/include/asm/mmu_context.h >> b/arch/x86/include/asm/mmu_context.h >> index 19d18fae6ec6..d684b954f3c0 100644 >> --- a/arch/x86/include/asm/mmu_context.h >> +++ b/arch/x86/include/asm/mmu_context.h >> @@ -356,4 +356,37 @@ static inline unsigned long __get_current_cr3_fast(void) >> return cr3; >> } >> >> +typedef struct { >> + struct mm_struct *prev; >> +} temp_mm_state_t; >> + >> +/* >> + * Using a temporary mm allows to set temporary mappings that are not >> accessible >> + * by other cores. Such mappings are needed to perform sensitive memory >> writes > > s/cores/CPUs/g > > Yeah, the concept of a thread of execution we call a CPU in the kernel, > I'd say. No matter if it is one of the hyperthreads or a single thread > in core. > >> + * 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 cores. > > Ditto. > >> Using temporary mm also allows to avoid TLB shootdowns when the > > Using a .. > >> + * mapping is torn down. >> + * > > Nice commenting. > >> + * 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 state; >> + >> + lockdep_assert_irqs_disabled(); >> + state.prev = this_cpu_read(cpu_tlbstate.loaded_mm); >> + switch_mm_irqs_off(NULL, mm, current); >> + return state; >> +} >> + >> +static inline void unuse_temporary_mm(temp_mm_state_t prev) >> +{ >> + lockdep_assert_irqs_disabled(); >> + switch_mm_irqs_off(NULL, prev.prev, current); > > I think this code would be more readable if you call that > temp_mm_state_t variable "temp_state" and the mm_struct pointer "mm" and > then you have: > > switch_mm_irqs_off(NULL, temp_state.mm, current); > > And above you'll have: > > temp_state.mm = ...
Andy, please let me know whether you are fine with this change and I’ll incorporate it.
