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: <9e220213-0d4a-4e61-b8cc-45ea21b073a6@arm.com>
Date: Tue, 24 Jun 2025 13:20:42 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Gerald Schaefer <gerald.schaefer@...ux.ibm.com>,
 Andrew Morton <akpm@...ux-foundation.org>
Cc: Matthew Wilcox <willy@...radead.org>, David Hildenbrand
 <david@...hat.com>, LKML <linux-kernel@...r.kernel.org>,
 linux-mm <linux-mm@...ck.org>
Subject: Re: [RFC PATCH 0/1] mm/debug_vm_pgtable: Use a swp_entry_t input
 value for swap tests

Hello Gerald,

On 24/06/25 12:13 AM, Gerald Schaefer wrote:
> Hi,
> 
> currently working on enabling THP_SWAP and THP_MIGRATION support for s390,
> and stumbling over the WARN_ON(args->fixed_pmd_pfn != pmd_pfn(pmd)) in
> debug_vm_pgtable pmd_swap_tests(). The problem is that pmd_pfn() on s390
> will use different shift values for leaf (large) and non-leaf PMDs. And
> when used on swapped PMDs, for which pmd_leaf() will always return false
> because !pmd_present(), the result is not really well defined.

Just curious - pmd_pfn() would have otherwise worked on leaf PMD entries ?
Because the PMD swap entries are not leaf entries as pmd_present() returns
negative, pmd_pfn() does not work on those ?

> 
> I think that pmd_pfn() is not safe or ever meant to be called on swapped
> PMD entries, and it doesn't seem to be used in that way anywhere else but
> debug_vm_pgtable. Also, the whole logic to test the various swap helpers

But is not the pmd_pfn() called on pmd which is derived from the swap entry
first.

	pmd = pfn_pmd(args->fixed_pmd_pfn, args->page_prot);
	swp = __pmd_to_swp_entry(pmd);
	pmd = __swp_entry_to_pmd(swp);
	WARN_ON(args->fixed_pmd_pfn != pmd_pfn(pmd));

> on normal PTE/PMD entries seems wrong to me. It just works by chance,
> because e.g. __pmd_to_swp_entry() and __swp_entry_to_pmd() are just no-ops
> on other architectures (also on s390, but only for PTEs), and also

Hmm, basically it just tests pfn_pmd() and pmd_pfn() conversions ?

> pmd_pfn() does not have any dependency on leaf/non-leaf entries there.Could you please elaborate on that ? 

> 
> So, I started with a small patch to make pmd_swap_tests() use a proper
> swapped PMD entry as input value, similar to how it is already done in
> pte_swap_exclusive_tests(), and not use pmd_pfn() for compare but rather
> compare the whole entries, again similar to pte_swap_exclusive_tests().

Agreed, that will make sense as well.

> 
> But then I noticed that such a change would probably also make sense for
> the other swap tests, and also a small inconsistency in Documentation,
> where it says e.g.
> 
> __pte_to_swp_entry        | Creates a swapped entry (arch) from a mapped PTE
> 
> I think this is wrong, those helpers should never operate on present and
> mapped PTEs, and they certainly don't create any swapped entry from a
> mapped entry, given that they are just no-ops on most architectures.
> Instead, in this example, it just returns the arch-dependent
> representation of a swp_entry_t, which happens to be just the entry
> itself on most architectures. See also pte_to_swp_entry() /
> swp_entry_to_pte() in include/linux/swapops.h.

Alright.

> 
> Now it became a larger clean-up, and I hope it makes sense. This is all
> rather new common code for me, so maybe I got things wrong, feedback is
> welcome.

A quick ran on arm64 looks just fine, will keep looking into this.

> 
> Gerald Schaefer (1):
>   mm/debug_vm_pgtable: Use a swp_entry_t input value for swap tests
> 
>  Documentation/mm/arch_pgtable_helpers.rst |  8 ++--
>  mm/debug_vm_pgtable.c                     | 55 ++++++++++++++---------
>  2 files changed, 38 insertions(+), 25 deletions(-)
> 



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ