linux-kernel - Re: TLB flushes on fixmap changes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALCETrW+nx7kRWov_+hCAxJRUSsUUJuVqvRP7KoVUxvFFTqi5Q@mail.gmail.com>
Date:   Mon, 27 Aug 2018 12:58:22 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Nadav Amit <nadav.amit@...il.com>
Cc:     Andy Lutomirski <luto@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Kees Cook <keescook@...omium.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Jiri Kosina <jkosina@...e.cz>,
        Will Deacon <will.deacon@....com>,
        Benjamin Herrenschmidt <benh@....ibm.com>,
        Nick Piggin <npiggin@...il.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        Rik van Riel <riel@...riel.com>,
        Jann Horn <jannh@...gle.com>,
        Adin Scannell <ascannell@...gle.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>,
        David Miller <davem@...emloft.net>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Michael Ellerman <mpe@...erman.id.au>
Subject: Re: TLB flushes on fixmap changes

On Mon, Aug 27, 2018 at 12:43 PM, Nadav Amit <nadav.amit@...il.com> wrote:
> at 12:10 PM, Nadav Amit <nadav.amit@...il.com> wrote:
>
>> at 11:58 AM, Andy Lutomirski <luto@...nel.org> wrote:
>>
>>> On Mon, Aug 27, 2018 at 11:54 AM, Nadav Amit <nadav.amit@...il.com> wrote:
>>>>> On Mon, Aug 27, 2018 at 10:34 AM, Nadav Amit <nadav.amit@...il.com> wrote:
>>>>> What do you all think?
>>>>
>>>> I agree in general. But I think that current->mm would need to be loaded, as
>>>> otherwise I am afraid it would break switch_mm_irqs_off().
>>>
>>> What breaks?
>>
>> Actually nothing. I just saw the IBPB stuff regarding tsk, but it should not
>> matter.
>
> So here is what I got. It certainly needs some cleanup, but it boots.
>
> Let me know how crappy you find it...
>
>
> diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
> index bbc796eb0a3b..336779650a41 100644
> --- a/arch/x86/include/asm/mmu_context.h
> +++ b/arch/x86/include/asm/mmu_context.h
> @@ -343,4 +343,24 @@ static inline unsigned long __get_current_cr3_fast(void)
>         return cr3;
>  }
>
> +typedef struct {
> +       struct mm_struct *prev;
> +} temporary_mm_state_t;
> +
> +static inline temporary_mm_state_t use_temporary_mm(struct mm_struct *mm)
> +{
> +       temporary_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(temporary_mm_state_t prev)
> +{
> +       lockdep_assert_irqs_disabled();
> +       switch_mm_irqs_off(NULL, prev.prev, current);
> +}
> +
>  #endif /* _ASM_X86_MMU_CONTEXT_H */
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 5715647fc4fe..ef62af9a0ef7 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -976,6 +976,10 @@ static inline void __meminit init_trampoline_default(void)
>         /* Default trampoline pgd value */
>         trampoline_pgd_entry = init_top_pgt[pgd_index(__PAGE_OFFSET)];
>  }
> +
> +void __init patching_mm_init(void);
> +#define patching_mm_init patching_mm_init
> +
>  # ifdef CONFIG_RANDOMIZE_MEMORY
>  void __meminit init_trampoline(void);
>  # else
> diff --git a/arch/x86/include/asm/pgtable_64_types.h b/arch/x86/include/asm/pgtable_64_types.h
> index 054765ab2da2..9f44262abde0 100644
> --- a/arch/x86/include/asm/pgtable_64_types.h
> +++ b/arch/x86/include/asm/pgtable_64_types.h
> @@ -116,6 +116,9 @@ extern unsigned int ptrs_per_p4d;
>  #define LDT_PGD_ENTRY          (pgtable_l5_enabled() ? LDT_PGD_ENTRY_L5 : LDT_PGD_ENTRY_L4)
>  #define LDT_BASE_ADDR          (LDT_PGD_ENTRY << PGDIR_SHIFT)
>
> +#define TEXT_POKE_PGD_ENTRY    -5UL
> +#define TEXT_POKE_ADDR         (TEXT_POKE_PGD_ENTRY << PGDIR_SHIFT)
> +
>  #define __VMALLOC_BASE_L4      0xffffc90000000000UL
>  #define __VMALLOC_BASE_L5      0xffa0000000000000UL
>
> diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
> index 99fff853c944..840c72ec8c4f 100644
> --- a/arch/x86/include/asm/pgtable_types.h
> +++ b/arch/x86/include/asm/pgtable_types.h
> @@ -505,6 +505,9 @@ pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
>  /* Install a pte for a particular vaddr in kernel space. */
>  void set_pte_vaddr(unsigned long vaddr, pte_t pte);
>
> +struct mm_struct;
> +void set_mm_pte_vaddr(struct mm_struct *mm, unsigned long vaddr, pte_t pte);
> +
>  #ifdef CONFIG_X86_32
>  extern void native_pagetable_init(void);
>  #else
> diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
> index 2ecd34e2d46c..cb364ea5b19d 100644
> --- a/arch/x86/include/asm/text-patching.h
> +++ b/arch/x86/include/asm/text-patching.h
> @@ -38,4 +38,6 @@ extern void *text_poke(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 struct mm_struct *patching_mm;
> +
>  #endif /* _ASM_X86_TEXT_PATCHING_H */
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index a481763a3776..fd8a950b0d62 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -11,6 +11,7 @@
>  #include <linux/stop_machine.h>
>  #include <linux/slab.h>
>  #include <linux/kdebug.h>
> +#include <linux/mmu_context.h>
>  #include <asm/text-patching.h>
>  #include <asm/alternative.h>
>  #include <asm/sections.h>
> @@ -701,8 +702,36 @@ void *text_poke(void *addr, const void *opcode, size_t len)
>                 WARN_ON(!PageReserved(pages[0]));
>                 pages[1] = virt_to_page(addr + PAGE_SIZE);
>         }
> -       BUG_ON(!pages[0]);
> +
>         local_irq_save(flags);
> +       BUG_ON(!pages[0]);
> +
> +       /*
> +        * During initial boot, it is hard to initialize patching_mm due to
> +        * dependencies in boot order.
> +        */
> +       if (patching_mm) {
> +               pte_t pte;
> +               temporary_mm_state_t prev;
> +
> +               prev = use_temporary_mm(patching_mm);
> +               pte = mk_pte(pages[0], PAGE_KERNEL);
> +               set_mm_pte_vaddr(patching_mm, TEXT_POKE_ADDR, pte);
> +               pte = mk_pte(pages[1], PAGE_KERNEL);
> +               set_mm_pte_vaddr(patching_mm, TEXT_POKE_ADDR + PAGE_SIZE, pte);
> +
> +               memcpy((void *)(TEXT_POKE_ADDR | ((unsigned long)addr & ~PAGE_MASK)),
> +                      opcode, len);
> +
> +               set_mm_pte_vaddr(patching_mm, TEXT_POKE_ADDR, __pte(0));
> +               set_mm_pte_vaddr(patching_mm, TEXT_POKE_ADDR + PAGE_SIZE, __pte(0));
> +               local_flush_tlb();

Hmm.  This is stuff busted on SMP, and it's IMO more complicated than
needed.  How about getting rid of all the weird TLB flushing stuff and
instead putting the mapping at vaddr - __START_KERNEL_map or whatever
it is?  You *might* need to flush_tlb_mm_range() on module unload, but
that's it.

> +               sync_core();

I can't think of any case where sync_core() is needed.  The mm switch
serializes.

Also, is there any circumstance in which any of this is used before at
least jump table init?  All the early stuff is text_poke_early(),
right?