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: <aFAlupvoJ_w7jCIU@localhost.localdomain>
Date: Mon, 16 Jun 2025 16:10:02 +0200
From: Oscar Salvador <osalvador@...e.de>
To: David Hildenbrand <david@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	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 Mon, Jun 16, 2025 at 11:22:43AM +0200, David Hildenbrand wrote:

> hugetlb_fault->hugetlb_no_page->hugetlb_wp
> 
> already *mapped* the pagecache page into the page table.
> 
> See
> 	if (anon_rmap)
> 		hugetlb_add_new_anon_rmap(folio, vma, vmf->address);
> 	else
> 		hugetlb_add_file_rmap(folio);
> 
> So at that point it would be "stable" unless I am missing something?
> 
> So once we are in hugetlb_wp(), that path much rather corresponds to
> do_wp_page()->wp_page_copy.

Yes, that's right.
That's something I've been thinking over the weekend.

E.g: do_cow_fault, first copies the page from the pagecache to a new one
and __then__ maps the that page into the page tables.
While in hugetlb_no_page->hugetlb_wp, the workflow is a bit different.

We first map it and then we copy it if we need to.

What do you mean by stable?
In the generic faulting path, we're not worried about the page going away
because we hold a reference, so I guess the lock must be to keep content stable?

I mean, yes, after we have mapped the page privately into the pagetables,
we don't have business about content-integrity anymore, so given this rule, yes,
I guess hugetlb_wp() wouldn't need the lock (for !anonymous) because we already
have mapped it privately at that point.

But there's something I don't fully understand and makes me feel uneasy.
If the lock in the generic faultin path is to keep content stable till we
have mapped it privately, wouldn't be more correct to also hold it
during the copy in hugetlb_wp, to kinda emulate that?

-- 
Oscar Salvador
SUSE Labs