linux-kernel - Re: [GIT PULL] arm64 updates for 6.13-rc1

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

Message-ID: <Z0h6Qtleb-znhX3u@arm.com>
Date: Thu, 28 Nov 2024 14:12:18 +0000
From: Catalin Marinas <catalin.marinas@....com>
To: Yang Shi <yang@...amperecomputing.com>
Cc: Sasha Levin <sashal@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Will Deacon <will@...nel.org>, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, David Hildenbrand <david@...hat.com>
Subject: Re: [GIT PULL] arm64 updates for 6.13-rc1

On Wed, Nov 27, 2024 at 05:21:37PM -0800, Yang Shi wrote:
> > > diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> > > index 87b3f1a25535..ef303a2262c5 100644
> > > --- a/arch/arm64/mm/copypage.c
> > > +++ b/arch/arm64/mm/copypage.c
> > > @@ -30,9 +30,9 @@ void copy_highpage(struct page *to, struct page *from)
> > >    	if (!system_supports_mte())
> > >    		return;
> > > -	if (folio_test_hugetlb(src) &&
> > > -	    folio_test_hugetlb_mte_tagged(src)) {
> > > -		if (!folio_try_hugetlb_mte_tagging(dst))
> > > +	if (folio_test_hugetlb(src)) {
> > > +		if (!folio_test_hugetlb_mte_tagged(src) ||
> > > +		    !folio_try_hugetlb_mte_tagging(dst))
> > >    			return;
> > >    		/*
> > I wonder why we had a 'return' here originally rather than a
> > WARN_ON_ONCE() as we do further down for the page case. Do you seen any
> > issue with the hunk below? Destination should be a new folio and not
> > tagged yet:
> 
> Yes, I did see problem. Because we copy tags for all sub pages then set
> folio mte tagged when copying the data for the first subpage. The warning
> will be triggered when we copy the second subpage.

Ah, good point, copy_highpage() will be called multiple times for each
subpage but we only do the copying once for the folio.

Now, I wonder whether we should actually defer the tag copying until
copy_page() is called on the head page. This way we can keep the warning
for consistency with the non-compound page case:

diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
index 87b3f1a25535..a86c897017df 100644
--- a/arch/arm64/mm/copypage.c
+++ b/arch/arm64/mm/copypage.c
@@ -30,11 +30,13 @@ void copy_highpage(struct page *to, struct page *from)
 	if (!system_supports_mte())
 		return;
 
-	if (folio_test_hugetlb(src) &&
-	    folio_test_hugetlb_mte_tagged(src)) {
-		if (!folio_try_hugetlb_mte_tagging(dst))
+	if (folio_test_hugetlb(src)) {
+		if (!folio_test_hugetlb_mte_tagged(src) ||
+		    from != folio_page(src, 0))
 			return;
 
+		WARN_ON_ONCE(!folio_try_hugetlb_mte_tagging(dst));
+
 		/*
 		 * Populate tags for all subpages.
 		 *

-- 
Catalin