linux-kernel - Re: [PATCH 2/5] mm,hugetlb: Document the reason to lock the folio in the faulting path

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: <44f0f1cc-307a-46e3-9e73-8b2061e4e938@redhat.com>
Date: Fri, 13 Jun 2025 15:56:15 +0200
From: David Hildenbrand <david@...hat.com>
To: Oscar Salvador <osalvador@...e.de>,
 Andrew Morton <akpm@...ux-foundation.org>
Cc: Muchun Song <muchun.song@...ux.dev>,
 James Houghton <jthoughton@...gle.com>, Peter Xu <peterx@...hat.com>,
 Gavin Guo <gavinguo@...lia.com>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/5] mm,hugetlb: Document the reason to lock the folio in
 the faulting path

On 12.06.25 15:46, Oscar Salvador wrote:
> Recent conversations showed that there was a misunderstanding about why we
> were locking the folio prior to calling hugetlb_wp().
> Document explicitly why we need to take the lock, explaining on the way that
> although the timespan for the locking of anonymous and file folios is different,
> it would require a major surgery to represent that difference with the current
> code.
> 
> Signed-off-by: Oscar Salvador <osalvador@...e.de>
> ---
>   mm/hugetlb.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 175edafeec67..dfa09fc3b2c6 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6537,6 +6537,10 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
>   			}
>   			new_pagecache_folio = true;
>   		} else {
> +			/*
> +			 * hugetlb_wp() expects the folio to be locked in order to
> +			 * check whether we can re-use this page exclusively for us.
> +			 */
>   			folio_lock(folio);
>   			anon_rmap = 1;
>   		}
> @@ -6801,7 +6805,19 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
>   		/* Fallthrough to CoW */
>   	}
>   
> -	/* hugetlb_wp() requires page locks of pte_page(vmf.orig_pte) */
> +	/*
> +	 * We need to lock the folio before calling hugetlb_wp().
> +	 * Either the folio is in the pagecache and we need to copy it over
> +	 * to another file, so it must remain stable throughout the operation,

But as discussed, why is that the case? We don't need that for ordinary 
pages, and existing folio mappings can already concurrently modify the page?

-- 
Cheers,

David / dhildenb