| Message ID | 20210316151054.5405-16-yu-cheng.yu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Control-flow Enforcement: Shadow Stack | expand |
On Tue, Mar 16, 2021 at 08:10:41AM -0700, Yu-cheng Yu wrote: > When serving a page fault, maybe_mkwrite() makes a PTE writable if its vma > has VM_WRITE. > > A shadow stack vma has VM_SHSTK. Its PTEs have _PAGE_DIRTY, but not > _PAGE_WRITE. In fork(), _PAGE_DIRTY is cleared to effect copy-on-write, to cause > and in page fault, _PAGE_DIRTY is restored and the shadow stack page is in the page fault handler... > writable again. > > Update maybe_mkwrite() by introducing arch_maybe_mkwrite(), which sets > _PAGE_DIRTY for a shadow stack PTE. > > Apply the same changes to maybe_pmd_mkwrite(). > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > Reviewed-by: Kees Cook <keescook@chromium.org> > --- > arch/x86/Kconfig | 4 ++++ > arch/x86/mm/pgtable.c | 18 ++++++++++++++++++ > include/linux/mm.h | 2 ++ > include/linux/pgtable.h | 24 ++++++++++++++++++++++++ > mm/huge_memory.c | 2 ++ > 5 files changed, 50 insertions(+) Looks straightforward to me but I guess it needs a mm person's ack.
On Tue, Mar 16, 2021 at 08:10:41AM -0700, Yu-cheng Yu wrote: > diff --git a/include/linux/mm.h b/include/linux/mm.h > index a6c18c5752d6..af805ffde48e 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -997,6 +997,8 @@ static inline pte_t maybe_mkwrite(pte_t pte, struct vm_area_struct *vma) > { > if (likely(vma->vm_flags & VM_WRITE)) > pte = pte_mkwrite(pte); > + else > + pte = arch_maybe_mkwrite(pte, vma); > return pte; > } I think it would be cleaner to allow arch code to override maybe_mkwrite() and maybe_pmd_mkwrite() altogether. Wrap it into #ifndef maybe_mkwrite here and provide VM_SHSTK-aware version from <asm/pgtable.h>.
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index bdfecc17c2d3..102212025993 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1945,12 +1945,16 @@ config X86_SGX config ARCH_HAS_SHADOW_STACK def_bool n +config ARCH_MAYBE_MKWRITE + def_bool n + config X86_CET prompt "Intel Control-flow protection for user-mode" def_bool n depends on AS_WRUSS depends on ARCH_HAS_SHADOW_STACK select ARCH_USES_HIGH_VMA_FLAGS + select ARCH_MAYBE_MKWRITE help Control-flow protection is a set of hardware features which place additional restrictions on indirect branches. These help diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c index f6a9e2e36642..0f4fbf51a9fc 100644 --- a/arch/x86/mm/pgtable.c +++ b/arch/x86/mm/pgtable.c @@ -610,6 +610,24 @@ int pmdp_clear_flush_young(struct vm_area_struct *vma, } #endif +#ifdef CONFIG_ARCH_MAYBE_MKWRITE +pte_t arch_maybe_mkwrite(pte_t pte, struct vm_area_struct *vma) +{ + if (likely(vma->vm_flags & VM_SHSTK)) + pte = pte_mkwrite_shstk(pte); + return pte; +} + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +pmd_t arch_maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma) +{ + if (likely(vma->vm_flags & VM_SHSTK)) + pmd = pmd_mkwrite_shstk(pmd); + return pmd; +} +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */ +#endif /* CONFIG_ARCH_MAYBE_MKWRITE */ + /** * reserve_top_address - reserves a hole in the top of kernel address space * @reserve - size of hole to reserve diff --git a/include/linux/mm.h b/include/linux/mm.h index a6c18c5752d6..af805ffde48e 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -997,6 +997,8 @@ static inline pte_t maybe_mkwrite(pte_t pte, struct vm_area_struct *vma) { if (likely(vma->vm_flags & VM_WRITE)) pte = pte_mkwrite(pte); + else + pte = arch_maybe_mkwrite(pte, vma); return pte; } diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index 5e772392a379..cbd98484c4f1 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -1446,6 +1446,30 @@ static inline bool arch_has_pfn_modify_check(void) } #endif /* !_HAVE_ARCH_PFN_MODIFY_ALLOWED */ +#ifdef CONFIG_MMU +#ifdef CONFIG_ARCH_MAYBE_MKWRITE +pte_t arch_maybe_mkwrite(pte_t pte, struct vm_area_struct *vma); + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +pmd_t arch_maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma); +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */ + +#else /* !CONFIG_ARCH_MAYBE_MKWRITE */ +static inline pte_t arch_maybe_mkwrite(pte_t pte, struct vm_area_struct *vma) +{ + return pte; +} + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE +static inline pmd_t arch_maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma) +{ + return pmd; +} +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */ + +#endif /* CONFIG_ARCH_MAYBE_MKWRITE */ +#endif /* CONFIG_MMU */ + /* * Architecture PAGE_KERNEL_* fallbacks * diff --git a/mm/huge_memory.c b/mm/huge_memory.c index ae907a9c2050..e79c1220a349 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -482,6 +482,8 @@ pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma) { if (likely(vma->vm_flags & VM_WRITE)) pmd = pmd_mkwrite(pmd); + else + pmd = arch_maybe_pmd_mkwrite(pmd, vma); return pmd; }