linux-kernel - Re: [PATCH v4 6/6] mm,thp: avoid writes to file with THP in pagecache

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c29e2daf2e3c9c8acbdfae62ba8090f572d88345.camel@fb.com>
Date:   Thu, 20 Jun 2019 17:42:02 +0000
From:   Rik van Riel <riel@...com>
To:     Song Liu <songliubraving@...com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "matthew.wilcox@...cle.com" <matthew.wilcox@...cle.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "Kernel Team" <Kernel-team@...com>,
        "william.kucharski@...cle.com" <william.kucharski@...cle.com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>
Subject: Re: [PATCH v4 6/6] mm,thp: avoid writes to file with THP in pagecache

On Thu, 2019-06-20 at 10:27 -0700, Song Liu wrote:

> +++ b/mm/mmap.c
> @@ -3088,6 +3088,18 @@ int vm_brk(unsigned long addr, unsigned long
> len)
>  }
>  EXPORT_SYMBOL(vm_brk);
>  
> +static inline void release_file_thp(struct vm_area_struct *vma)
> +{
> +#ifdef CONFIG_READ_ONLY_THP_FOR_FS
> +	struct file *file = vma->vm_file;
> +
> +	if (file && (vma->vm_flags & VM_DENYWRITE) &&
> +	    atomic_read(&file_inode(file)->i_writecount) == 0 &&
> +	    filemap_nr_thps(file_inode(file)->i_mapping))
> +		truncate_pagecache(file_inode(file), 0);
> +#endif
> +}
> +
>  /* Release all mmaps. */
>  void exit_mmap(struct mm_struct *mm)
>  {
> @@ -3153,6 +3165,8 @@ void exit_mmap(struct mm_struct *mm)
>  	while (vma) {
>  		if (vma->vm_flags & VM_ACCOUNT)
>  			nr_accounted += vma_pages(vma);
> +
> +		release_file_thp(vma);
>  		vma = remove_vma(vma);
>  	}
>  	vm_unacct_memory(nr_accounted);

I like how you make the file accessible again to other
users, but am somewhat unsure about the mechanism used.

First, if multiple processes have the same file mmapped,
do you really want to blow away the page cache?

Secondly, by hooking into exit_mmap, you miss making
files writable again that get unmapped through munmap.

Would it be better to blow away the page cache when
the last mmap user unmaps it?

The page->mapping->i_mmap interval tree will be empty
when nobody has the file mmap()d.

Alternatively, open() could check whether the file is
currently mmaped, and blow away the page cache then.
That would leave the page cache intact if the same file 
gets execve()d several times in a row without any writes
in-between, which seems like it might be a relatively
common case.