linux-kernel - Re: [RFC 12/20] mm/tlb: save the VMA that is flushed during tlb_start

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: <YBfz8MMQ25xkjO7e@hirez.programming.kicks-ass.net>
Date:   Mon, 1 Feb 2021 13:28:32 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Nadav Amit <nadav.amit@...il.com>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Nadav Amit <namit@...are.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Will Deacon <will@...nel.org>, Yu Zhao <yuzhao@...gle.com>,
        Nick Piggin <npiggin@...il.com>, x86@...nel.org
Subject: Re: [RFC 12/20] mm/tlb: save the VMA that is flushed during
 tlb_start_vma()

On Sat, Jan 30, 2021 at 04:11:24PM -0800, Nadav Amit wrote:

> @@ -283,12 +290,6 @@ struct mmu_gather {
>  	unsigned int		cleared_puds : 1;
>  	unsigned int		cleared_p4ds : 1;
>  
> -	/*
> -	 * tracks VM_EXEC | VM_HUGETLB in tlb_start_vma
> -	 */
> -	unsigned int		vma_exec : 1;
> -	unsigned int		vma_huge : 1;
> -
>  	unsigned int		batch_count;
>  
>  #ifndef CONFIG_MMU_GATHER_NO_GATHER

> @@ -372,38 +369,20 @@ static inline void tlb_flush(struct mmu_gather *tlb)
>  	if (tlb->fullmm || tlb->need_flush_all) {
>  		flush_tlb_mm(tlb->mm);
>  	} else if (tlb->end) {
> -		struct vm_area_struct vma = {
> -			.vm_mm = tlb->mm,
> -			.vm_flags = (tlb->vma_exec ? VM_EXEC    : 0) |
> -				    (tlb->vma_huge ? VM_HUGETLB : 0),
> -		};
> -
> -		flush_tlb_range(&vma, tlb->start, tlb->end);
> +		VM_BUG_ON(!tlb->vma);
> +		flush_tlb_range(tlb->vma, tlb->start, tlb->end);
>  	}
>  }

I don't much like this, and I think this is a step in the wrong
direction.

The idea is to extend the tlb_{remove,flush}_*() API to provide the
needed information to do TLB flushing. In fact, I think
tlb_remove_huge*() is already sufficient to set the VM_EXEC 'hint'. We
just don't have anything that covers the EXEC thing.

(also, I suspect the page_size crud we have also covers that)

Constructing a fake vma very much ensures arch tlb routines don't go
about and look at anything else either.

> +tlb_update_vma(struct mmu_gather *tlb, struct vm_area_struct *vma)
>  {
> -	/*
> -	 * flush_tlb_range() implementations that look at VM_HUGETLB (tile,
> -	 * mips-4k) flush only large pages.
> -	 *
> -	 * flush_tlb_range() implementations that flush I-TLB also flush D-TLB
> -	 * (tile, xtensa, arm), so it's ok to just add VM_EXEC to an existing
> -	 * range.
> -	 *
> -	 * We rely on tlb_end_vma() to issue a flush, such that when we reset
> -	 * these values the batch is empty.
> -	 */
> -	tlb->vma_huge = is_vm_hugetlb_page(vma);
> -	tlb->vma_exec = !!(vma->vm_flags & VM_EXEC);
> +	tlb->vma = vma;
>  }

And you're also removing the useful information about arch tlb flush
functions.