linux-kernel - Re: [PATCH v1 3/9] mm/memory: further separate anon and pagecache folio handling in zap_present

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] [thread-next>] [day] [month] [year] [list]

Date: Tue, 30 Jan 2024 09:37:12 +0100
From: David Hildenbrand <david@...hat.com>
To: Ryan Roberts <ryan.roberts@....com>, linux-kernel@...r.kernel.org
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
 Matthew Wilcox <willy@...radead.org>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>,
 Nick Piggin <npiggin@...il.com>, Peter Zijlstra <peterz@...radead.org>,
 Michael Ellerman <mpe@...erman.id.au>,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>,
 Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
 Alexander Gordeev <agordeev@...ux.ibm.com>,
 Christian Borntraeger <borntraeger@...ux.ibm.com>,
 Sven Schnelle <svens@...ux.ibm.com>, Arnd Bergmann <arnd@...db.de>,
 linux-arch@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
 linux-s390@...r.kernel.org
Subject: Re: [PATCH v1 3/9] mm/memory: further separate anon and pagecache
 folio handling in zap_present_pte()

On 30.01.24 09:31, Ryan Roberts wrote:
> On 29/01/2024 14:32, David Hildenbrand wrote:
>> We don't need up-to-date accessed-dirty information for anon folios and can
>> simply work with the ptent we already have. Also, we know the RSS counter
>> we want to update.
>>
>> We can safely move arch_check_zapped_pte() + tlb_remove_tlb_entry() +
>> zap_install_uffd_wp_if_needed() after updating the folio and RSS.
>>
>> While at it, only call zap_install_uffd_wp_if_needed() if there is even
>> any chance that pte_install_uffd_wp_if_needed() would do *something*.
>> That is, just don't bother if uffd-wp does not apply.
>>
>> Signed-off-by: David Hildenbrand <david@...hat.com>
>> ---
>>   mm/memory.c | 16 +++++++++++-----
>>   1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 69502cdc0a7d..20bc13ab8db2 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -1552,12 +1552,9 @@ static inline void zap_present_pte(struct mmu_gather *tlb,
>>   	folio = page_folio(page);
>>   	if (unlikely(!should_zap_folio(details, folio)))
>>   		return;
>> -	ptent = ptep_get_and_clear_full(mm, addr, pte, tlb->fullmm);
>> -	arch_check_zapped_pte(vma, ptent);
>> -	tlb_remove_tlb_entry(tlb, pte, addr);
>> -	zap_install_uffd_wp_if_needed(vma, addr, pte, details, ptent);
>>   
>>   	if (!folio_test_anon(folio)) {
>> +		ptent = ptep_get_and_clear_full(mm, addr, pte, tlb->fullmm);
>>   		if (pte_dirty(ptent)) {
>>   			folio_mark_dirty(folio);
>>   			if (tlb_delay_rmap(tlb)) {
>> @@ -1567,8 +1564,17 @@ static inline void zap_present_pte(struct mmu_gather *tlb,
>>   		}
>>   		if (pte_young(ptent) && likely(vma_has_recency(vma)))
>>   			folio_mark_accessed(folio);
>> +		rss[mm_counter(folio)]--;
>> +	} else {
>> +		/* We don't need up-to-date accessed/dirty bits. */
>> +		ptep_get_and_clear_full(mm, addr, pte, tlb->fullmm);
>> +		rss[MM_ANONPAGES]--;
>>   	}
>> -	rss[mm_counter(folio)]--;
>> +	arch_check_zapped_pte(vma, ptent);
> 
> Isn't the x86 (only) implementation of this relying on the dirty bit? So doesn't
> that imply you still need get_and_clear for anon? (And in hindsight I think that
> logic would apply to the previous patch too?)

x86 uses the encoding !writable && dirty to indicate special shadow 
stacks. That is, the hw dirty bit is set by software (to create that 
combination), not by hardware.

So you don't have to sync against any hw changes of the hw dirty bit. 
What you had in the original PTE you read is sufficient.

-- 
Cheers,

David / dhildenb