| Message ID | 0a6caf8305409173b5d41dccb6ecd46460cf9c1c.1734526570.git.zhengqi.arch@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | move pagetable_*_dtor() to __tlb_remove_table() | expand |
On Wed, Dec 18, 2024 at 09:04:46PM +0800, Qi Zheng wrote: > In s390, the pagetable_dtor() of PTE has long been moved to > __tlb_remove_table(). Well, not pagetable_dtor(), but rather pagetable_pte_dtor() and not to __tlb_remove_table(), but to pagetable_pte_dtor_free() ;) > So similarly, also move the pagetable_dtor() of > PMD|PUD|P4D to __tlb_remove_table(). This prevents the use-after-free > problem where the ptlock is freed immediately but the page table pages > is freed later via RCU. > > By the way, rename pagetable_pte_dtor_free() to pagetable_dtor_free(). This is not just a random rename, but rather a result of unifying PxD and PTE TLB free paths. Could you please come up with a better wording concerning the above? > Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> > Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: linux-s390@vger.kernel.org > --- > arch/s390/include/asm/tlb.h | 3 --- > arch/s390/mm/pgalloc.c | 14 ++++---------- > 2 files changed, 4 insertions(+), 13 deletions(-) You also did not CC linux-s390@vger.kernel.org for other patches that affect s390 sources. Just CC the whole series, please. Thanks!
On 2024/12/20 16:42, Alexander Gordeev wrote: > On Wed, Dec 18, 2024 at 09:04:46PM +0800, Qi Zheng wrote: >> In s390, the pagetable_dtor() of PTE has long been moved to >> __tlb_remove_table(). > > Well, not pagetable_dtor(), but rather pagetable_pte_dtor() and > not to __tlb_remove_table(), but to pagetable_pte_dtor_free() ;) > >> So similarly, also move the pagetable_dtor() of >> PMD|PUD|P4D to __tlb_remove_table(). This prevents the use-after-free >> problem where the ptlock is freed immediately but the page table pages >> is freed later via RCU. >> >> By the way, rename pagetable_pte_dtor_free() to pagetable_dtor_free(). > > This is not just a random rename, but rather a result of unifying > PxD and PTE TLB free paths. > > Could you please come up with a better wording concerning the above? Sorry about that. Will modify it as you said above. > >> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> >> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org> >> Cc: linux-s390@vger.kernel.org >> --- >> arch/s390/include/asm/tlb.h | 3 --- >> arch/s390/mm/pgalloc.c | 14 ++++---------- >> 2 files changed, 4 insertions(+), 13 deletions(-) > > You also did not CC linux-s390@vger.kernel.org for other patches > that affect s390 sources. Just CC the whole series, please. Got it, will CC linux-s390@vger.kernel.org for the whole series in v3. Thanks! > > Thanks!
diff --git a/arch/s390/include/asm/tlb.h b/arch/s390/include/asm/tlb.h index 74b6fba4c2ee3..79df7c0932c56 100644 --- a/arch/s390/include/asm/tlb.h +++ b/arch/s390/include/asm/tlb.h @@ -102,7 +102,6 @@ static inline void pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd, { if (mm_pmd_folded(tlb->mm)) return; - pagetable_dtor(virt_to_ptdesc(pmd)); __tlb_adjust_range(tlb, address, PAGE_SIZE); tlb->mm->context.flush_mm = 1; tlb->freed_tables = 1; @@ -122,7 +121,6 @@ static inline void pud_free_tlb(struct mmu_gather *tlb, pud_t *pud, { if (mm_pud_folded(tlb->mm)) return; - pagetable_dtor(virt_to_ptdesc(pud)); tlb->mm->context.flush_mm = 1; tlb->freed_tables = 1; tlb->cleared_p4ds = 1; @@ -141,7 +139,6 @@ static inline void p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d, { if (mm_p4d_folded(tlb->mm)) return; - pagetable_dtor(virt_to_ptdesc(p4d)); __tlb_adjust_range(tlb, address, PAGE_SIZE); tlb->mm->context.flush_mm = 1; tlb->freed_tables = 1; diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c index 569de24d33761..c73b89811a264 100644 --- a/arch/s390/mm/pgalloc.c +++ b/arch/s390/mm/pgalloc.c @@ -180,7 +180,7 @@ unsigned long *page_table_alloc(struct mm_struct *mm) return table; } -static void pagetable_pte_dtor_free(struct ptdesc *ptdesc) +static void pagetable_dtor_free(struct ptdesc *ptdesc) { pagetable_dtor(ptdesc); pagetable_free(ptdesc); @@ -190,20 +190,14 @@ void page_table_free(struct mm_struct *mm, unsigned long *table) { struct ptdesc *ptdesc = virt_to_ptdesc(table); - pagetable_pte_dtor_free(ptdesc); + pagetable_dtor_free(ptdesc); } void __tlb_remove_table(void *table) { struct ptdesc *ptdesc = virt_to_ptdesc(table); - struct page *page = ptdesc_page(ptdesc); - if (compound_order(page) == CRST_ALLOC_ORDER) { - /* pmd, pud, or p4d */ - pagetable_free(ptdesc); - return; - } - pagetable_pte_dtor_free(ptdesc); + pagetable_dtor_free(ptdesc); } #ifdef CONFIG_TRANSPARENT_HUGEPAGE @@ -211,7 +205,7 @@ static void pte_free_now(struct rcu_head *head) { struct ptdesc *ptdesc = container_of(head, struct ptdesc, pt_rcu_head); - pagetable_pte_dtor_free(ptdesc); + pagetable_dtor_free(ptdesc); } void pte_free_defer(struct mm_struct *mm, pgtable_t pgtable)
In s390, the pagetable_dtor() of PTE has long been moved to __tlb_remove_table(). So similarly, also move the pagetable_dtor() of PMD|PUD|P4D to __tlb_remove_table(). This prevents the use-after-free problem where the ptlock is freed immediately but the page table pages is freed later via RCU. By the way, rename pagetable_pte_dtor_free() to pagetable_dtor_free(). Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: linux-s390@vger.kernel.org --- arch/s390/include/asm/tlb.h | 3 --- arch/s390/mm/pgalloc.c | 14 ++++---------- 2 files changed, 4 insertions(+), 13 deletions(-)