Messages in this thread | | | From | David Hildenbrand <> | Subject | Re: [RFC PATCH 4/4] x86/mm: write protect (most) page tables | Date | Wed, 25 Aug 2021 10:38:22 +0200 |
| |
On 24.08.21 01:50, Dave Hansen wrote: > On 8/23/21 6:25 AM, Mike Rapoport wrote: >> void ___pte_free_tlb(struct mmu_gather *tlb, struct page *pte) >> { >> + enable_pgtable_write(page_address(pte)); >> pgtable_pte_page_dtor(pte); >> paravirt_release_pte(page_to_pfn(pte)); >> paravirt_tlb_remove_table(tlb, pte); >> @@ -69,6 +73,7 @@ void ___pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd) >> #ifdef CONFIG_X86_PAE >> tlb->need_flush_all = 1; >> #endif >> + enable_pgtable_write(pmd); >> pgtable_pmd_page_dtor(page); >> paravirt_tlb_remove_table(tlb, page); >> } > > I would expected this to have leveraged the pte_offset_map/unmap() code > to enable/disable write access. Granted, it would enable write access > even when only a read is needed, but that could be trivially fixed with > having a variant like:
For write access you actually want pte_offset_map_locked(), but it's also used for stable read access sometimes (exclude any writers).
> > pte_offset_map_write() > pte_offset_unmap_write() > > in addition to the existing (presumably read-only) versions: > > pte_offset_map() > pte_offset_unmap()
These should mostly only be read access, you'd need other ways of making sure nobody else messes with that entry. I think it even holds for khugepaged collapsing code.
I find these hidden PMD entry modifications (e.g., without holding the PMD lock) deep down in arch code quite concerning. Read: horribly ugly and a nightmare to debug.
-- Thanks,
David / dhildenb
| |