[PATCH v2 0/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[PATCH v2 0/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[Claudio Imbrenda]

Fix handling of _PAGE_UNUSED pte bit. The bit was lingering around
longer than it should have, causing issues.

This patch replaces "s390/pgtable: Unconditionally clear _PAGE_UNUSED"
which also solved the issue in practice, but in the wrong way.

v1->v2:
* Completely different approach, entirely new patch

Claudio Imbrenda (1):
  s390/mm: Fix handling of _PAGE_UNUSED pte bit

 arch/s390/mm/gmap_helpers.c | 4 ++--
 arch/s390/mm/pgtable.c      | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

-- 
2.54.0

[PATCH v2 1/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[Claudio Imbrenda]

The _PAGE_UNUSED softbit should not really be lying around. Its sole
purpose is to signal to try_to_unmap_one() and try_to_migrate_one()
that the page can be discarded instead of being moved / swapped.

KVM has no way to know why a page is being unmapped, so it sets the bit
on userspace ptes corresponding to unused guest pages every time they
get unmapped. KVM has no reasonable way to clear the bit once the page
is in use again.

Without appropriate cleanup, the _PAGE_UNUSED bit will linger around
and cause guest corruption when a used page is instead thrown out.

While set_ptes() checks and clears the bit, ptep_xchg_direct(),
ptep_xchg_lazy(), and ptep_modify_prot_commit() did not. This led to
used pages being thrown out as if they were unused, causing guest
corruption.

This patch fixes the issue by introducing the missing checks in the
above functions.

Also fix gmap_helper_try_set_pte_unused() to only set the bit if the
pte is present; the _PAGE_UNUSED bit is only defined for present ptes
and thus should not be set for non-present ptes.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Fixes: c98175b7917f ("KVM: s390: Add gmap_helper_set_unused()")
---
 arch/s390/mm/gmap_helpers.c | 4 ++--
 arch/s390/mm/pgtable.c      | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/s390/mm/gmap_helpers.c b/arch/s390/mm/gmap_helpers.c
index 1cfe4724fbe2..5a7d6b9790e2 100644
--- a/arch/s390/mm/gmap_helpers.c
+++ b/arch/s390/mm/gmap_helpers.c
@@ -180,8 +180,8 @@ void gmap_helper_try_set_pte_unused(struct mm_struct *mm, unsigned long vmaddr)
 	ptep = try_get_locked_pte(mm, vmaddr, &ptl);
 	if (IS_ERR_OR_NULL(ptep))
 		return;
-
-	__atomic64_or(_PAGE_UNUSED, (long *)ptep);
+	if (pte_present(*ptep))
+		__atomic64_or(_PAGE_UNUSED, (long *)ptep);
 	pte_unmap_unlock(ptep, ptl);
 }
 EXPORT_SYMBOL_GPL(gmap_helper_try_set_pte_unused);
diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
index 4acd8b140c4b..2acc79383e7d 100644
--- a/arch/s390/mm/pgtable.c
+++ b/arch/s390/mm/pgtable.c
@@ -122,6 +122,8 @@ pte_t ptep_xchg_direct(struct mm_struct *mm, unsigned long addr,
 
 	preempt_disable();
 	old = ptep_flush_direct(mm, addr, ptep, 1);
+	if (pte_present(new))
+		new = clear_pte_bit(new, __pgprot(_PAGE_UNUSED));
 	set_pte(ptep, new);
 	preempt_enable();
 	return old;
@@ -160,6 +162,8 @@ pte_t ptep_xchg_lazy(struct mm_struct *mm, unsigned long addr,
 
 	preempt_disable();
 	old = ptep_flush_lazy(mm, addr, ptep, 1);
+	if (pte_present(new))
+		new = clear_pte_bit(new, __pgprot(_PAGE_UNUSED));
 	set_pte(ptep, new);
 	preempt_enable();
 	return old;
@@ -175,6 +179,8 @@ pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr,
 void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
 			     pte_t *ptep, pte_t old_pte, pte_t pte)
 {
+	if (pte_present(pte))
+		pte = clear_pte_bit(pte, __pgprot(_PAGE_UNUSED));
 	set_pte(ptep, pte);
 }
 
-- 
2.54.0

Re: [PATCH v2 1/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[Heiko Carstens]

On Mon, Jun 15, 2026 at 11:17:41AM +0200, Claudio Imbrenda wrote:
> The _PAGE_UNUSED softbit should not really be lying around. Its sole
> purpose is to signal to try_to_unmap_one() and try_to_migrate_one()
> that the page can be discarded instead of being moved / swapped.
> 
> KVM has no way to know why a page is being unmapped, so it sets the bit
> on userspace ptes corresponding to unused guest pages every time they
> get unmapped. KVM has no reasonable way to clear the bit once the page
> is in use again.
> 
> Without appropriate cleanup, the _PAGE_UNUSED bit will linger around
> and cause guest corruption when a used page is instead thrown out.
> 
> While set_ptes() checks and clears the bit, ptep_xchg_direct(),
> ptep_xchg_lazy(), and ptep_modify_prot_commit() did not. This led to
> used pages being thrown out as if they were unused, causing guest
> corruption.
> 
> This patch fixes the issue by introducing the missing checks in the
> above functions.
> 
> Also fix gmap_helper_try_set_pte_unused() to only set the bit if the
> pte is present; the _PAGE_UNUSED bit is only defined for present ptes
> and thus should not be set for non-present ptes.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Fixes: c98175b7917f ("KVM: s390: Add gmap_helper_set_unused()")
> ---
>  arch/s390/mm/gmap_helpers.c | 4 ++--
>  arch/s390/mm/pgtable.c      | 6 ++++++
>  2 files changed, 8 insertions(+), 2 deletions(-)

...

> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
> index 4acd8b140c4b..2acc79383e7d 100644
> --- a/arch/s390/mm/pgtable.c
> +++ b/arch/s390/mm/pgtable.c
> @@ -122,6 +122,8 @@ pte_t ptep_xchg_direct(struct mm_struct *mm, unsigned long addr,
>  
>  	preempt_disable();
>  	old = ptep_flush_direct(mm, addr, ptep, 1);
> +	if (pte_present(new))
> +		new = clear_pte_bit(new, __pgprot(_PAGE_UNUSED));
>  	set_pte(ptep, new);
>  	preempt_enable();
>  	return old;
> @@ -160,6 +162,8 @@ pte_t ptep_xchg_lazy(struct mm_struct *mm, unsigned long addr,
>  
>  	preempt_disable();
>  	old = ptep_flush_lazy(mm, addr, ptep, 1);
> +	if (pte_present(new))
> +		new = clear_pte_bit(new, __pgprot(_PAGE_UNUSED));
>  	set_pte(ptep, new);
>  	preempt_enable();
>  	return old;
> @@ -175,6 +179,8 @@ pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr,
>  void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
>  			     pte_t *ptep, pte_t old_pte, pte_t pte)
>  {
> +	if (pte_present(pte))
> +		pte = clear_pte_bit(pte, __pgprot(_PAGE_UNUSED));
>  	set_pte(ptep, pte);

Can't we move the logic from set_ptes() to set_pte() instead? The above
approach remembers me of the open-coded removal of the no-exec bit at many
places we had, which became a maintenance mess until it was rewritten.

The compiler _might_ even be clever enough to move the removal of the bit
outside the loop within set_ptes().

Re: [PATCH v2 1/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[Claudio Imbrenda]

On Mon, 15 Jun 2026 11:43:00 +0200
Heiko Carstens <hca@linux.ibm.com> wrote:

[...]

> > @@ -175,6 +179,8 @@ pte_t ptep_modify_prot_start(struct
vm_area_struct *vma, unsigned long addr,
> >  void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
> >  			     pte_t *ptep, pte_t old_pte, pte_t pte)
> >  {
> > +	if (pte_present(pte))
> > +		pte = clear_pte_bit(pte, __pgprot(_PAGE_UNUSED));
> >  	set_pte(ptep, pte);  
> 
> Can't we move the logic from set_ptes() to set_pte() instead? The above

set_pte() is also used for things that are not ptes, and in those cases
we probably don't want to touch that bit, although technically it is
currently unused for present large pmds and puds.

> approach remembers me of the open-coded removal of the no-exec bit at many
> places we had, which became a maintenance mess until it was rewritten.
> 
> The compiler _might_ even be clever enough to move the removal of the bit
> outside the loop within set_ptes().

Re: [PATCH v2 1/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[Heiko Carstens]

On Mon, Jun 15, 2026 at 12:31:03PM +0200, Claudio Imbrenda wrote:
> On Mon, 15 Jun 2026 11:43:00 +0200
> Heiko Carstens <hca@linux.ibm.com> wrote:
> 
> [...]
> 
> > > @@ -175,6 +179,8 @@ pte_t ptep_modify_prot_start(struct
> vm_area_struct *vma, unsigned long addr,
> > >  void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
> > >  			     pte_t *ptep, pte_t old_pte, pte_t pte)
> > >  {
> > > +	if (pte_present(pte))
> > > +		pte = clear_pte_bit(pte, __pgprot(_PAGE_UNUSED));
> > >  	set_pte(ptep, pte);  
> > 
> > Can't we move the logic from set_ptes() to set_pte() instead? The above
> 
> set_pte() is also used for things that are not ptes, and in those cases
> we probably don't want to touch that bit, although technically it is
> currently unused for present large pmds and puds.

I can only see huge_pte_clear() for this.

If that's the only user I'd rather add a BUG_ON() there instead of starting to
sprinkle the logic around. This _will_ break sooner or later.

Re: [PATCH v2 1/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] `ptep_reset_dat_prot()` misses clearing the `_PAGE_UNUSED` bit, allowing used pages to be wrongly discarded.
- [Critical] Hardware Storage Key updates by guest accesses bypass host PTE modifications, allowing `_PAGE_UNUSED` to linger and cause guest corruption.
--

commit 4a7ab46235e9a4cd801253465aae645c11c76ebb
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

s390/mm: Fix handling of _PAGE_UNUSED pte bit

The patch introduces checks to clear the _PAGE_UNUSED bit in various ptep
exchange and modify functions to prevent used pages from being discarded.
It also ensures the bit is only set for present PTEs in the gmap helper.

> diff --git a/arch/s390/mm/gmap_helpers.c b/arch/s390/mm/gmap_helpers.c
> index 1cfe4724fbe2a..5a7d6b9790e22 100644
> --- a/arch/s390/mm/gmap_helpers.c
> +++ b/arch/s390/mm/gmap_helpers.c

[ ... ]

> @@ -180,8 +180,8 @@ void gmap_helper_try_set_pte_unused(struct mm_struct *mm, unsigned long vmaddr)
>  	ptep = try_get_locked_pte(mm, vmaddr, &ptl);
>  	if (IS_ERR_OR_NULL(ptep))
>  		return;
> -
> -	__atomic64_or(_PAGE_UNUSED, (long *)ptep);
> +	if (pte_present(*ptep))
> +		__atomic64_or(_PAGE_UNUSED, (long *)ptep);

[Severity: Critical]
This is a pre-existing issue, but does the approach of clearing _PAGE_UNUSED
during host PTE modifications handle hardware-driven updates?

In gmap_helper_try_set_pte_unused(), __atomic64_or() sets _PAGE_UNUSED
without clearing the present or writable bits in the host PTE. If the guest
subsequently writes to this page, the CPU hardware handles it entirely via
Dynamic Address Translation, updating the hardware Storage Key without ever
trapping into the host kernel or modifying the host PTE.

Because the host PTE is never modified, the clearance logic added in this
patch is completely bypassed.

If the host later reclaims this memory via a path that skips
folio_referenced() (such as madvise(MADV_PAGEOUT), which calls
reclaim_pages() with ignore_references = true), the PTE is passed to
try_to_unmap_one() with _PAGE_UNUSED still set:

mm/rmap.c:try_to_unmap_one() {
    ...
    } else if (likely(pte_present(pteval)) && pte_unused(pteval) &&
               !userfaultfd_armed(vma)) {
        dec_mm_counter(mm, mm_counter(folio));
    ...
}

Because try_to_unmap_one() evaluates pte_unused() before checking the
Storage Key's dirty state, won't this unconditionally discard the page and
permanently destroy the guest's modified data?

>  	pte_unmap_unlock(ptep, ptl);
>  }

[ ... ]

> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
> index 4acd8b140c4bd..2acc79383e7df 100644
> --- a/arch/s390/mm/pgtable.c
> +++ b/arch/s390/mm/pgtable.c

[ ... ]

> @@ -175,6 +179,8 @@ pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr,
>  void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
>  			     pte_t *ptep, pte_t old_pte, pte_t pte)
>  {
> +	if (pte_present(pte))
> +		pte = clear_pte_bit(pte, __pgprot(_PAGE_UNUSED));
>  	set_pte(ptep, pte);
>  }

[Severity: Critical]
This is a pre-existing issue, but it looks like the fix might be incomplete.
Does ptep_reset_dat_prot() also need to clear the _PAGE_UNUSED bit?

Unlike ptep_xchg_direct(), ptep_reset_dat_prot() directly writes the new
PTE without clearing _PAGE_UNUSED:

arch/s390/mm/pgtable.c:ptep_reset_dat_prot() {
    ...
    set_pte(ptep, new);
    ...
}

When a KVM guest unmaps a page, it is marked with _PAGE_UNUSED. If the guest
subsequently writes to this page (for example, an RO to RW transition on a
clean page), ptep_set_access_flags() is called by the host page fault handler.

Because the modification only affects software bits and _PAGE_PROTECT,
pte_allow_rdp() evaluates to true, delegating the PTE update to the
architecture-specific fast path:

arch/s390/include/asm/pgtable.h:ptep_set_access_flags() {
    ...
    if (cpu_has_rdp() && pte_allow_rdp(*ptep, entry))
        ptep_reset_dat_prot(vma->vm_mm, addr, ptep, entry);
    ...
}

Since the new PTE was constructed from the old one, it inherits the
_PAGE_UNUSED flag. Won't the now-dirty page retain the unused flag and be
erroneously discarded without writeback during memory reclaim by
try_to_unmap_one()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615091741.76724-2-imbrenda@linux.ibm.com?part=1

Re: [PATCH v2 1/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[Gerald Schaefer]

On Mon, 15 Jun 2026 13:50:00 +0200
Heiko Carstens <hca@linux.ibm.com> wrote:

> On Mon, Jun 15, 2026 at 12:31:03PM +0200, Claudio Imbrenda wrote:
> > On Mon, 15 Jun 2026 11:43:00 +0200
> > Heiko Carstens <hca@linux.ibm.com> wrote:
> > 
> > [...]
> > 
> > > > @@ -175,6 +179,8 @@ pte_t ptep_modify_prot_start(struct
> > vm_area_struct *vma, unsigned long addr,
> > > >  void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
> > > >  			     pte_t *ptep, pte_t old_pte, pte_t pte)
> > > >  {
> > > > +	if (pte_present(pte))
> > > > +		pte = clear_pte_bit(pte, __pgprot(_PAGE_UNUSED));
> > > >  	set_pte(ptep, pte);  
> > > 
> > > Can't we move the logic from set_ptes() to set_pte() instead? The above
> > 
> > set_pte() is also used for things that are not ptes, and in those cases
> > we probably don't want to touch that bit, although technically it is
> > currently unused for present large pmds and puds.
> 
> I can only see huge_pte_clear() for this.
> 
> If that's the only user I'd rather add a BUG_ON() there instead of starting to
> sprinkle the logic around. This _will_ break sooner or later.

There is also __set_huge_pte_at(), and there it could also be called for
swap PMDs/PUDs, where bit 56 is used. But they would not be present, and
ATM we use the same present bit for PTEs and PMDs/PUDs, so it should work.

Still feels a bit shaky, but in general I agree that it would be better
to have this logic in a single place, like set_pte().

Also wonder now why we use set_pte() an not set_pmd() / set_pud() for the
hugetlbfs "fake" PTEs. I think at least in s390 code we could switch to
the pmd/pud variants, and then be safe against modifications from set_pte().

BTW, wrt Sashiko report that just dropped in, I also wondered first about
ptep_reset_dat_prot() using set_pte(). Not 100% sure about the exact scenario
where/how the _PAGE_UNUSED bit gets mixed in, where it shouldn't. Maybe
the answer to that question might even show another fix option. But when
changing set_pte(), it should also be fine for ptep_reset_dat_prot(),
which requires that the PROTECT bit is the only HW bit getting changed,
and _PAGE_UNUSED is a SW bit.

Re: [PATCH v2 1/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[Alexander Gordeev]

On Mon, Jun 15, 2026 at 11:17:41AM +0200, Claudio Imbrenda wrote:
> @@ -122,6 +122,8 @@ pte_t ptep_xchg_direct(struct mm_struct *mm, unsigned long addr,
>  
>  	preempt_disable();
>  	old = ptep_flush_direct(mm, addr, ptep, 1);
> +	if (pte_present(new))
> +		new = clear_pte_bit(new, __pgprot(_PAGE_UNUSED));

Why not before preempt_disable()?

>  	set_pte(ptep, new);
>  	preempt_enable();
>  	return old;
> @@ -160,6 +162,8 @@ pte_t ptep_xchg_lazy(struct mm_struct *mm, unsigned long addr,
>  
>  	preempt_disable();
>  	old = ptep_flush_lazy(mm, addr, ptep, 1);
> +	if (pte_present(new))
> +		new = clear_pte_bit(new, __pgprot(_PAGE_UNUSED));

Same here.

>  	set_pte(ptep, new);
>  	preempt_enable();
>  	return old;

Re: [PATCH v2 1/1] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[Heiko Carstens]

On Mon, Jun 15, 2026 at 02:09:39PM +0200, Gerald Schaefer wrote:
> On Mon, 15 Jun 2026 13:50:00 +0200
> Heiko Carstens <hca@linux.ibm.com> wrote:
> > > set_pte() is also used for things that are not ptes, and in those cases
> > > we probably don't want to touch that bit, although technically it is
> > > currently unused for present large pmds and puds.
> > 
> > I can only see huge_pte_clear() for this.
> > 
> > If that's the only user I'd rather add a BUG_ON() there instead of starting to
> > sprinkle the logic around. This _will_ break sooner or later.
> 
> There is also __set_huge_pte_at(), and there it could also be called for
> swap PMDs/PUDs, where bit 56 is used. But they would not be present, and
> ATM we use the same present bit for PTEs and PMDs/PUDs, so it should work.
> 
> Still feels a bit shaky, but in general I agree that it would be better
> to have this logic in a single place, like set_pte().

Yes, let's do that please.

> Also wonder now why we use set_pte() an not set_pmd() / set_pud() for the
> hugetlbfs "fake" PTEs. I think at least in s390 code we could switch to
> the pmd/pud variants, and then be safe against modifications from set_pte().

I guess that would be nice cleanup. Could you provide a patch for
that, please?