linux-kernel - Re: [PATCH v1 2/2] mm/hugetlb: support write-faults in shared mappings

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: <Yu2i2K421LgaMyxp@monkey>
Date:   Fri, 5 Aug 2022 16:08:08 -0700
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Peter Xu <peterx@...hat.com>
Cc:     David Hildenbrand <david@...hat.com>, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
        Muchun Song <songmuchun@...edance.com>,
        Peter Feiner <pfeiner@...gle.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCH v1 2/2] mm/hugetlb: support write-faults in shared
 mappings

On 08/05/22 14:12, Peter Xu wrote:
> On Fri, Aug 05, 2022 at 01:03:29PM +0200, David Hildenbrand wrote:
> > Let's add a safety net if we ever get (again) a write-fault on a R/O-mapped
> > page in a shared mapping, in which case we simply have to map the
> > page writable.
> > 

> > +		if (WARN_ON_ONCE(!(vma->vm_flags & VM_WRITE)))
> > +			return VM_FAULT_SIGSEGV;
> 
> I had a feeling that you just want to double check we have write
> permission, but IIUC this should be checked far earlier or we'll have
> problem.

Back when I was exploring hugetlb softdirty support, this patch handled
this condition by not calling into hugetlb_wp (was hugetlb_cow).

https://lore.kernel.org/linux-mm/20210211000322.159437-3-mike.kravetz@oracle.com/

Here is a quickly updated version that was only tested with David's program.
-- 
Mike Kravetz

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index a23f1cffaf49..86d38d41fddf 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5758,9 +5758,10 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 	 * spinlock. For private mappings, we also lookup the pagecache
 	 * page now as it is used to determine if a reservation has been
 	 * consumed.
+	 * Only private/non-shared mappings are sent to hugetlb_wp.
 	 */
 	if ((flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) &&
-	    !huge_pte_write(entry)) {
+	    !huge_pte_write(entry) && !(vma->vm_flags & VM_MAYSHARE)) {
 		if (vma_needs_reservation(h, vma, haddr) < 0) {
 			ret = VM_FAULT_OOM;
 			goto out_mutex;
@@ -5768,8 +5769,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 		/* Just decrements count, does not deallocate */
 		vma_end_reservation(h, vma, haddr);
 
-		if (!(vma->vm_flags & VM_MAYSHARE))
-			pagecache_page = hugetlbfs_pagecache_page(h,
+		pagecache_page = hugetlbfs_pagecache_page(h,
 								vma, haddr);
 	}
 
@@ -5815,9 +5815,14 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 
 	if (flags & (FAULT_FLAG_WRITE|FAULT_FLAG_UNSHARE)) {
 		if (!huge_pte_write(entry)) {
-			ret = hugetlb_wp(mm, vma, address, ptep, flags,
+			if (!(vma->vm_flags & VM_MAYSHARE)) {
+				ret = hugetlb_wp(mm, vma, address, ptep, flags,
 					 pagecache_page, ptl);
-			goto out_put_page;
+				goto out_put_page;
+			}
+
+			entry = huge_pte_mkwrite(entry);
+			entry = huge_pte_mkdirty(entry);
 		} else if (likely(flags & FAULT_FLAG_WRITE)) {
 			entry = huge_pte_mkdirty(entry);
 		}