[PATCH v3 0/6] s390: Reintroduce support for DCACHE_WORD_ACCESS

[PATCH v3 0/6] s390: Reintroduce support for DCACHE_WORD_ACCESS

[Heiko Carstens]

v3:
- Fix two more pre-existing bugs reported by Sashiko
- Add Christian's Tested-by tag to four of the six patches

v2:
- Explicitely PAGE_ALIGN size in uv_alloc_stor_var() [Sashiko]
- Document that lazy PTE update / TLB flushing is intended [Sashiko]

v1:
Support for DCACHE_WORD_ACCESS was recently removed [1] since it caused
problems with the incomplete handling of secure storage access
exceptions. It looked like fixing the exception handler would be a larger
effort; therefore support for DCACHE_WORD_ACCESS was removed as a work
around.

Address the potential problems that exist with secure storage access
exceptions and add support for DCACHE_WORD_ACCESS again.

In particular address the following problems:

- Reading the guest variable storage area via the /proc/kcore interface
  results in short reads. Address this by using a VM_SPARSE area for the
  guest variable storage area. VM_SPARSE areas will be handled like
  memory holes (zeros will be read).

- Fix handling of secure storage access exceptions in vmalloc area.

- Remove folio handling for secure storage access exceptions to avoid
  potential data corruption.

[1] 37540b8c287f ("s390: Revert support for DCACHE_WORD_ACCESS")

Heiko Carstens (6):
  KVM: s390: pv: Use VM_SPARSE area for guest variable storage area
  s390/mm: Add missing mm check to do_secure_storage_access()
  s390/mm: Use lock_mm_and_find_vma() in do_secure_storage_access()
  s390/mm: Fix handling of vmalloc area in do_secure_storage_access()
  s390/mm: Remove folio handling for kernel faults in do_secure_storage_access()
  s390: Add support for DCACHE_WORD_ACCESS (again)

 arch/s390/Kconfig                      |  1 +
 arch/s390/include/asm/asm-extable.h    |  4 ++
 arch/s390/include/asm/uv.h             |  2 +
 arch/s390/include/asm/word-at-a-time.h | 22 +++++++++
 arch/s390/kernel/uv.c                  | 65 ++++++++++++++++++++++++++
 arch/s390/kvm/pv.c                     |  6 +--
 arch/s390/mm/extable.c                 | 18 +++++++
 arch/s390/mm/fault.c                   | 20 +++-----
 8 files changed, 122 insertions(+), 16 deletions(-)

-- 
2.53.0

[PATCH v3 1/6] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area

[Heiko Carstens]

The guest variable storage area is allocated with vmalloc and then
donated to the ultravisor. Any kernel access to that area will result
in a secure storage access exception (aka fault).

This is a problem if such a memory area is read via /proc/kcore. This
causes an exception via vread_iter() and results in an unexpected short
read. Avoid this by allocating a custom VM_SPARSE area. If such an area
is read, vread_iter() returns zeroes for the entire area.

Note that the function which frees the area does not update ptes. This
is intentional to allow for deferred / lazy pte updates and TLB flushing
like the generic vfree() code is doing that. See vunmap_pte_range().

This assumes that s390 will gain full support for lazy_mmu_mode_enable()
and lazy_mmu_mode_disable() in the future, since as of now the used
ptep_get_and_clear() in vunmap_pte_range() does indeed invalidate and
flush every single pte entry, but only for s390.

Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/uv.h |  2 ++
 arch/s390/kernel/uv.c      | 65 ++++++++++++++++++++++++++++++++++++++
 arch/s390/kvm/pv.c         |  6 ++--
 3 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index d919e69662f5..153fed63adda 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -635,6 +635,8 @@ int s390_wiggle_split_folio(struct mm_struct *mm, struct folio *folio);
 int __make_folio_secure(struct folio *folio, struct uv_cb_header *uvcb);
 int uv_convert_from_secure(unsigned long paddr);
 int uv_convert_from_secure_folio(struct folio *folio);
+void *uv_alloc_stor_var(unsigned long size);
+void uv_free_stor_var(void *stor_var);
 
 void setup_uv(void);
 
diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index a284f98d9716..d1cdc5ae5e27 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -16,6 +16,7 @@
 #include <linux/swap.h>
 #include <linux/pagewalk.h>
 #include <linux/backing-dev.h>
+#include <linux/vmalloc.h>
 #include <asm/facility.h>
 #include <asm/sections.h>
 #include <asm/uv.h>
@@ -209,6 +210,70 @@ int uv_convert_from_secure_pte(pte_t pte)
 	return uv_convert_from_secure_folio(pfn_folio(pte_pfn(pte)));
 }
 
+static int uv_free_range_cb(pte_t *ptep, unsigned long addr, void *data)
+{
+	pte_t pte = ptep_get(ptep);
+
+	if (!pte_present(pte))
+		return 0;
+	/*
+	 * Note: do not update the pte here, since there is no code which
+	 * accesses the memory range, besides bugs. The invalidation of ptes
+	 * and TLB flushing is deferred like for regular vfree() calls.
+	 */
+	__free_page(pte_page(pte));
+	return 0;
+}
+
+void uv_free_stor_var(void *stor_var)
+{
+	unsigned long addr, size;
+	struct vm_struct *area;
+
+	if (!stor_var)
+		return;
+	area = find_vm_area(stor_var);
+	if (WARN_ON_ONCE(!area || !(area->flags & VM_SPARSE)))
+		return;
+	size = get_vm_area_size(area);
+	addr = (unsigned long)area->addr;
+	apply_to_existing_page_range(&init_mm, addr, size, uv_free_range_cb, NULL);
+	free_vm_area(area);
+}
+EXPORT_SYMBOL_FOR_MODULES(uv_free_stor_var, "kvm");
+
+static int uv_alloc_range_cb(pte_t *ptep, unsigned long addr, void *data)
+{
+	struct page *page;
+	pte_t pte;
+
+	page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
+	if (!page)
+		return -ENOMEM;
+	pte = __pte(page_to_phys(page) | pgprot_val(PAGE_KERNEL));
+	set_pte(ptep, pte);
+	return 0;
+}
+
+void *uv_alloc_stor_var(unsigned long size)
+{
+	struct vm_struct *area;
+	unsigned long addr;
+
+	size = PAGE_ALIGN(size);
+	area = get_vm_area(size, VM_SPARSE);
+	if (!area)
+		return NULL;
+	addr = (unsigned long)area->addr;
+	if (apply_to_page_range(&init_mm, addr, size, uv_alloc_range_cb, NULL))
+		goto out;
+	return area->addr;
+out:
+	uv_free_stor_var(area->addr);
+	return NULL;
+}
+EXPORT_SYMBOL_FOR_MODULES(uv_alloc_stor_var, "kvm");
+
 /*
  * Calculate the expected ref_count for a folio that would otherwise have no
  * further pins. This was cribbed from similar functions in other places in
diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
index 1beacc841ca8..dc5ac29b4c31 100644
--- a/arch/s390/kvm/pv.c
+++ b/arch/s390/kvm/pv.c
@@ -337,7 +337,7 @@ int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc)
 /* only free resources when the destroy was successful */
 static void kvm_s390_pv_dealloc_vm(struct kvm *kvm)
 {
-	vfree(kvm->arch.pv.stor_var);
+	uv_free_stor_var(kvm->arch.pv.stor_var);
 	free_pages(kvm->arch.pv.stor_base,
 		   get_order(uv_info.guest_base_stor_len));
 	kvm_s390_clear_pv_state(kvm);
@@ -369,7 +369,7 @@ static int kvm_s390_pv_alloc_vm(struct kvm *kvm)
 	/* Allocate variable storage */
 	vlen = ALIGN(virt * ((npages * PAGE_SIZE) / HPAGE_SIZE), PAGE_SIZE);
 	vlen += uv_info.guest_virt_base_stor_len;
-	kvm->arch.pv.stor_var = vzalloc(vlen);
+	kvm->arch.pv.stor_var = uv_alloc_stor_var(vlen);
 	if (!kvm->arch.pv.stor_var)
 		goto out_err;
 	return 0;
@@ -414,7 +414,7 @@ static int kvm_s390_pv_dispose_one_leftover(struct kvm *kvm,
 	 */
 	free_pages(leftover->stor_base, get_order(uv_info.guest_base_stor_len));
 	free_pages(leftover->old_gmap_table, CRST_ALLOC_ORDER);
-	vfree(leftover->stor_var);
+	uv_free_stor_var(leftover->stor_var);
 done_fast:
 	atomic_dec(&kvm->mm->context.protected_count);
 	return 0;
-- 
2.53.0

[PATCH v3 2/6] s390/mm: Add missing mm check to do_secure_storage_access()

[Heiko Carstens]

Similar like the normal exception handler the secure storage access
exception handler needs to verify if mm is not NULL, which may happen
with kernel threads.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260717132345.539A11F00A3A@smtp.kernel.org/
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/mm/fault.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 028aeb9c48d6..2839d7a64401 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -409,9 +409,9 @@ void do_secure_storage_access(struct pt_regs *regs)
 {
 	union teid teid = { .val = regs->int_parm_long };
 	unsigned long addr = get_fault_address(regs);
+	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma;
 	struct folio_walk fw;
-	struct mm_struct *mm;
 	struct folio *folio;
 	int rc;
 
@@ -453,9 +453,8 @@ void do_secure_storage_access(struct pt_regs *regs)
 		if (rc)
 			return handle_fault_error_nolock(regs, 0);
 	} else {
-		if (faulthandler_disabled())
+		if (faulthandler_disabled() || !mm)
 			return handle_fault_error_nolock(regs, 0);
-		mm = current->mm;
 		mmap_read_lock(mm);
 		vma = find_vma(mm, addr);
 		if (!vma)
-- 
2.53.0

[PATCH v3 3/6] s390/mm: Use lock_mm_and_find_vma() in do_secure_storage_access()

[Heiko Carstens]

do_secure_storage_access() uses find_vma() without verifying that the
faulting address is within the returned vma. Add this missing check by
converting to lock_mm_and_find_vma().

This is not a critical fix, since the worst that could happen is a
WARN_ON_ONCE() in folio_walk_start().

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260717093904.E4A421F00A3E@smtp.kernel.org/
Link: https://lore.kernel.org/all/20260717093904.E4A421F00A3E@smtp.kernel.org/
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/mm/fault.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 2839d7a64401..f150d97cac2e 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -455,10 +455,9 @@ void do_secure_storage_access(struct pt_regs *regs)
 	} else {
 		if (faulthandler_disabled() || !mm)
 			return handle_fault_error_nolock(regs, 0);
-		mmap_read_lock(mm);
-		vma = find_vma(mm, addr);
+		vma = lock_mm_and_find_vma(mm, addr, regs);
 		if (!vma)
-			return handle_fault_error(regs, SEGV_MAPERR);
+			return handle_fault_error_nolock(regs, SEGV_MAPERR);
 		folio = folio_walk_start(&fw, vma, addr, 0);
 		if (!folio) {
 			mmap_read_unlock(mm);
-- 
2.53.0

[PATCH v3 4/6] s390/mm: Fix handling of vmalloc area in do_secure_storage_access()

[Heiko Carstens]

Secure storage access exceptions in vmalloc area need to be handled
separately since folio_try_get() or virt_to_phys() do not work on such
addresses. Given that there cannot be a valid access let such exceptions
fail immediately. The kernel will either fixup or crash.

Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/mm/fault.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index f150d97cac2e..e650cdfa2523 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -438,6 +438,8 @@ void do_secure_storage_access(struct pt_regs *regs)
 		panic("Unexpected PGM 0x3d with TEID bit 61=0");
 	}
 	if (is_kernel_fault(regs)) {
+		if (is_vmalloc_addr((void *)addr))
+			return handle_fault_error_nolock(regs, 0);
 		folio = virt_to_folio((void *)addr);
 		if (unlikely(!folio_try_get(folio)))
 			return;
-- 
2.53.0

[PATCH v3 5/6] s390/mm: Remove folio handling for kernel faults in do_secure_storage_access()

[Heiko Carstens]

Remove folio handling for secure storage access exceptions that happen
via the kernel mapping. Using folio_try_get() for exceptions via the
kernel mapping is racy: for example such an exception may happen on one
CPU, while a different CPU frees the corresponding page. Subsequently the
page can be part of a folio of different size, before the faulting CPU
executes folio_try_get(). This may lead to data corruption.

As of now this scenario does not seem to be possible, however with the
upcoming load_unaligned_zeropad() this is possible.

As a side effect PG_arch_1 is not cleared anymore for such faults via the
kernel mapping. However given that this bit is over-indicating it
shouldn't matter.

Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/mm/fault.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index e650cdfa2523..776549d55c94 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -440,13 +440,7 @@ void do_secure_storage_access(struct pt_regs *regs)
 	if (is_kernel_fault(regs)) {
 		if (is_vmalloc_addr((void *)addr))
 			return handle_fault_error_nolock(regs, 0);
-		folio = virt_to_folio((void *)addr);
-		if (unlikely(!folio_try_get(folio)))
-			return;
-		rc = uv_convert_from_secure(folio_to_phys(folio));
-		if (!rc)
-			clear_bit(PG_arch_1, &folio->flags.f);
-		folio_put(folio);
+		rc = uv_convert_from_secure(__pa(addr));
 		/*
 		 * There are some valid fixup types for kernel
 		 * accesses to donated secure memory. zeropad is one
-- 
2.53.0

[PATCH v3 6/6] s390: Add support for DCACHE_WORD_ACCESS (again)

[Heiko Carstens]

Implement load_unaligned_zeropad() and enable DCACHE_WORD_ACCESS to
speed up string operations in fs/dcache.c and fs/namei.c.

With the secure storage access exception cases addressed, add support
for DCACHE_WORD_ACCESS again.

Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/Kconfig                      |  1 +
 arch/s390/include/asm/asm-extable.h    |  4 ++++
 arch/s390/include/asm/word-at-a-time.h | 22 ++++++++++++++++++++++
 arch/s390/mm/extable.c                 | 18 ++++++++++++++++++
 4 files changed, 45 insertions(+)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 84404e6778d5..fe81c539ef2f 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -163,6 +163,7 @@ config S390
 	select ARCH_WANTS_THP_SWAP
 	select BUILDTIME_TABLE_SORT
 	select CLONE_BACKWARDS2
+	select DCACHE_WORD_ACCESS if !KMSAN
 	select DYNAMIC_FTRACE if FUNCTION_TRACER
 	select FUNCTION_ALIGNMENT_8B if CC_IS_GCC
 	select FUNCTION_ALIGNMENT_16B if !CC_IS_GCC
diff --git a/arch/s390/include/asm/asm-extable.h b/arch/s390/include/asm/asm-extable.h
index 99748c20e767..d23ea0c94e4e 100644
--- a/arch/s390/include/asm/asm-extable.h
+++ b/arch/s390/include/asm/asm-extable.h
@@ -12,6 +12,7 @@
 #define EX_TYPE_UA_FAULT	3
 #define EX_TYPE_UA_LOAD_REG	5
 #define EX_TYPE_UA_LOAD_REGPAIR	6
+#define EX_TYPE_ZEROPAD		7
 #define EX_TYPE_FPC		8
 #define EX_TYPE_UA_MVCOS_TO	9
 #define EX_TYPE_UA_MVCOS_FROM	10
@@ -79,6 +80,9 @@
 #define EX_TABLE_UA_LOAD_REGPAIR(_fault, _target, _regerr, _regzero)	\
 	__EX_TABLE(__ex_table, _fault, _target, EX_TYPE_UA_LOAD_REGPAIR, _regerr, _regzero, 0)
 
+#define EX_TABLE_ZEROPAD(_fault, _target, _regdata, _regaddr)		\
+	__EX_TABLE(__ex_table, _fault, _target, EX_TYPE_ZEROPAD, _regdata, _regaddr, 0)
+
 #define EX_TABLE_FPC(_fault, _target)					\
 	__EX_TABLE(__ex_table, _fault, _target, EX_TYPE_FPC, __stringify(%%r0), __stringify(%%r0), 0)
 
diff --git a/arch/s390/include/asm/word-at-a-time.h b/arch/s390/include/asm/word-at-a-time.h
index e9287036392d..eaa19dee7699 100644
--- a/arch/s390/include/asm/word-at-a-time.h
+++ b/arch/s390/include/asm/word-at-a-time.h
@@ -4,6 +4,7 @@
 
 #include <linux/bitops.h>
 #include <linux/wordpart.h>
+#include <asm/asm-extable.h>
 #include <asm/bitsperlong.h>
 
 struct word_at_a_time {
@@ -40,4 +41,25 @@ static inline unsigned long zero_bytemask(unsigned long data)
 	return ~1UL << data;
 }
 
+/*
+ * Load an unaligned word from kernel space.
+ *
+ * In the (very unlikely) case of the word being a page-crosser
+ * and the next page not being mapped, take the exception and
+ * return zeroes in the non-existing part.
+ */
+static inline unsigned long load_unaligned_zeropad(const void *addr)
+{
+	unsigned long data;
+
+	asm_inline volatile(
+		"0:	lg	%[data],0(%[addr])\n"
+		"1:	nopr	%%r7\n"
+		EX_TABLE_ZEROPAD(0b, 1b, %[data], %[addr])
+		EX_TABLE_ZEROPAD(1b, 1b, %[data], %[addr])
+		: [data] "=d" (data)
+		: [addr] "a" (addr), "m" (*(unsigned long *)addr));
+	return data;
+}
+
 #endif /* _ASM_WORD_AT_A_TIME_H */
diff --git a/arch/s390/mm/extable.c b/arch/s390/mm/extable.c
index 063b4346742d..7498e858c401 100644
--- a/arch/s390/mm/extable.c
+++ b/arch/s390/mm/extable.c
@@ -50,6 +50,22 @@ static bool ex_handler_ua_load_reg(const struct exception_table_entry *ex,
 	return true;
 }
 
+static bool ex_handler_zeropad(const struct exception_table_entry *ex, struct pt_regs *regs)
+{
+	unsigned int reg_addr = FIELD_GET(EX_DATA_REG_ADDR, ex->data);
+	unsigned int reg_data = FIELD_GET(EX_DATA_REG_ERR, ex->data);
+	unsigned long data, addr, offset;
+
+	addr = regs->gprs[reg_addr];
+	offset = addr & (sizeof(unsigned long) - 1);
+	addr &= ~(sizeof(unsigned long) - 1);
+	data = *(unsigned long *)addr;
+	data <<= BITS_PER_BYTE * offset;
+	regs->gprs[reg_data] = data;
+	regs->psw.addr = extable_fixup(ex);
+	return true;
+}
+
 static bool ex_handler_fpc(const struct exception_table_entry *ex, struct pt_regs *regs)
 {
 	fpu_sfpc(0);
@@ -118,6 +134,8 @@ bool fixup_exception(struct pt_regs *regs)
 		return ex_handler_ua_load_reg(ex, false, regs);
 	case EX_TYPE_UA_LOAD_REGPAIR:
 		return ex_handler_ua_load_reg(ex, true, regs);
+	case EX_TYPE_ZEROPAD:
+		return ex_handler_zeropad(ex, regs);
 	case EX_TYPE_FPC:
 		return ex_handler_fpc(ex, regs);
 	case EX_TYPE_UA_MVCOS_TO:
-- 
2.53.0

Re: [PATCH v3 3/6] s390/mm: Use lock_mm_and_find_vma() in do_secure_storage_access()

[sashiko-bot]

> do_secure_storage_access() uses find_vma() without verifying that the
> faulting address is within the returned vma. Add this missing check by
> converting to lock_mm_and_find_vma().
> 
> This is not a critical fix, since the worst that could happen is a
> WARN_ON_ONCE() in folio_walk_start().
> 
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/20260717093904.E4A421F00A3E@smtp.kernel.org/
> Link: https://lore.kernel.org/all/20260717093904.E4A421F00A3E@smtp.kernel.org/
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719130006.3882764-1-hca@linux.ibm.com?part=3

Re: [PATCH v3 6/6] s390: Add support for DCACHE_WORD_ACCESS (again)

[sashiko-bot]

> Implement load_unaligned_zeropad() and enable DCACHE_WORD_ACCESS to
> speed up string operations in fs/dcache.c and fs/namei.c.
> 
> With the secure storage access exception cases addressed, add support
> for DCACHE_WORD_ACCESS again.
> 
> Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719130006.3882764-1-hca@linux.ibm.com?part=6

Re: [PATCH v3 4/6] s390/mm: Fix handling of vmalloc area in do_secure_storage_access()

[sashiko-bot]

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

Pre-existing issues:
- [High] Returning from the trap handler without fixing the exception when `folio_try_get()` fails causes an infinite kernel loop.
- [High] Kernel-mode accesses (e.g., `copy_from_user`) that fail to convert a secure folio will infinite loop due to improper signal handling.
--

--- Patch [4]: [PATCH v3 4/6] s390/mm: Fix handling of vmalloc area in do_secure_storage_access() ---
commit 1759378fac324776e9a7cbd8390b8445f3bb360c
Author: Heiko Carstens <hca@linux.ibm.com>

s390/mm: Fix handling of vmalloc area in do_secure_storage_access()

Secure storage access exceptions in vmalloc area need to be handled
separately since folio_try_get() or virt_to_phys() do not work on such
addresses. Given that there cannot be a valid access let such exceptions
fail immediately. The kernel will either fixup or crash.

> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c

[ ... ]

> @@ -438,6 +438,8 @@ void do_secure_storage_access(struct pt_regs *regs)
>  		panic("Unexpected PGM 0x3d with TEID bit 61=0");
>  	}
>  	if (is_kernel_fault(regs)) {
> +		if (is_vmalloc_addr((void *)addr))
> +			return handle_fault_error_nolock(regs, 0);
>  		folio = virt_to_folio((void *)addr);
>  		if (unlikely(!folio_try_get(folio)))
>  			return;

[Severity: High]
This isn't a bug introduced by this patch, but returning directly here in
do_secure_storage_access() without fixing the exception state when
folio_try_get() fails seems problematic.

Since PGM 0x3d is a suppressing exception on s390, the instruction pointer
isn't advanced. Returning without modifying the page state or adjusting the
instruction pointer (e.g., via fixup_exception()) will cause the hardware
to immediately re-execute the faulting instruction upon returning to the
interrupted context.

Will this trigger the exact same exception repeatedly and result in a hard
kernel lockup? Should this fail gracefully by calling
handle_fault_error_nolock(regs, 0) instead, so that exception tables can
be processed or a proper Oops generated?

[ ... ]

> 		rc = arch_make_folio_accessible(folio);
> 		folio_put(folio);
> 		folio_walk_end(&fw, vma);
> 		if (rc)
> 			send_sig(SIGSEGV, current, 0);
> 		mmap_read_unlock(mm);

[Severity: High]
This is also a pre-existing issue, but does this error path in
do_secure_storage_access() correctly handle kernel-mode accesses that fail
to convert a secure folio?

For accesses to user-space memory, is_kernel_fault() returns false even if
the access was performed by the kernel (e.g., copy_from_user() using secondary
space). If arch_make_folio_accessible() fails, the code queues a signal and
returns.

Because the fault happened in kernel mode, returning doesn't immediately
deliver the signal to user space. It resumes kernel execution and re-executes
the faulting copy_from_user() instruction, hitting the secure page again and
triggering another PGM 0x3d exception.

Can this result in an infinite kernel loop repeatedly queuing SIGSEGVs without
making forward progress? Should this path check !user_mode(regs) and call
handle_fault_error_nolock() (or equivalent) to apply fixup_exception() for
kernel-mode accesses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719130006.3882764-1-hca@linux.ibm.com?part=4

Re: [PATCH v3 2/6] s390/mm: Add missing mm check to do_secure_storage_access()

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing VMA Bounds Check allows Denial of Service via WARN_ON_ONCE.
- [High] Invalid Memory Access / Kernel Crash via virt_to_folio() on non-direct-mapped addresses.
--

--- Patch [2]: [PATCH v3 2/6] s390/mm: Add missing mm check to do_secure_storage_access() ---
commit 55ed80680943e84fffd67209605311fedc0e28ae
Author: Heiko Carstens <hca@linux.ibm.com>

s390/mm: Add missing mm check to do_secure_storage_access()
    
Similar like the normal exception handler the secure storage access
exception handler needs to verify if mm is not NULL, which may happen
with kernel threads.
    
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260717132345.539A11F00A3A@smtp.kernel.org/

> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index 028aeb9c48d6f..2839d7a64401c 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -409,9 +409,9 @@ void do_secure_storage_access(struct pt_regs *regs)
>  {
>  	union teid teid = { .val = regs->int_parm_long };
>  	unsigned long addr = get_fault_address(regs);
> +	struct mm_struct *mm = current->mm;
>  	struct vm_area_struct *vma;
>  	struct folio_walk fw;
> -	struct mm_struct *mm;
>  	struct folio *folio;
>  	int rc;
>  

[ ... ]

> @@ -453,9 +453,8 @@ void do_secure_storage_access(struct pt_regs *regs)
>  		if (rc)
>  			return handle_fault_error_nolock(regs, 0);
>  	} else {
> -		if (faulthandler_disabled())
> +		if (faulthandler_disabled() || !mm)
>  			return handle_fault_error_nolock(regs, 0);
> -		mm = current->mm;
>  		mmap_read_lock(mm);
>  		vma = find_vma(mm, addr);
>  		if (!vma)

[Severity: High]
This isn't a bug introduced by this patch, but can this cause memory
corruption?

Earlier in this same function:

arch/s390/mm/fault.c:do_secure_storage_access() {
    ...
	if (is_kernel_fault(regs)) {
		folio = virt_to_folio((void *)addr);
    ...
}

When the kernel accesses a secure guest page through a vmalloc mapping,
is_kernel_fault(regs) evaluates to true. Then virt_to_folio() is
unconditionally applied to the faulting kernel address without validating that
it is a direct-mapped address.

Since virt_to_folio() subtracts the direct map offset, wouldn't it produce a
bogus PFN for vmalloc addresses, leading to dereferencing a garbage struct page
pointer?

[Severity: High]
This is also a pre-existing issue, but could this lead to a denial of service
via WARN_ON_ONCE?

Looking at do_secure_storage_access() in arch/s390/mm/fault.c, find_vma()
returns a VMA that may start after the faulting address, which is not verified
before use.

If a concurrent thread unmaps the VMA between the time the hardware generates
the secure storage exception and the handler acquires the mmap lock, could
find_vma() return the next adjacent VMA?

If so, passing this potentially unrelated VMA to folio_walk_start():

arch/s390/mm/fault.c:do_secure_storage_access() {
    ...
		vma = find_vma(mm, addr);
		if (!vma)
			return handle_fault_error(regs, SEGV_MAPERR);
		folio = folio_walk_start(&fw, vma, addr, 0);
    ...
}

would trigger the WARN_ON_ONCE(addr < vma->vm_start || addr >= vma->vm_end)
safety check. Could a malicious user-space program exploit this race by
repeatedly triggering secure storage accesses while concurrently unmapping
regions to crash the system if panic_on_warn is enabled?

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

Re: [PATCH v3 5/6] s390/mm: Remove folio handling for kernel faults in do_secure_storage_access()

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Kernel accesses with exception fixups (e.g., speculative reads or probe reads) unconditionally destroy active secure guest pages because uv_convert_from_secure() is executed before checking fixup_exception().
--

--- Patch [5]: [PATCH v3 5/6] s390/mm: Remove folio handling for kernel faults in do_secure_storage_access() ---
commit 475be2dbf4868b390a06f3b6bd78b394674a341a
Author: Heiko Carstens <hca@linux.ibm.com>

s390/mm: Remove folio handling for kernel faults in do_secure_storage_access()

This commit removes folio handling and reference counting for secure
storage access exceptions triggered via the kernel mapping. It replaces
the folio_to_phys() conversion with a direct __pa(addr) translation to
avoid data corruption from a known race condition.

> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index e650cdfa25231..776549d55c94a 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -440,13 +440,7 @@ void do_secure_storage_access(struct pt_regs *regs)
>  	if (is_kernel_fault(regs)) {
>  		if (is_vmalloc_addr((void *)addr))
>  			return handle_fault_error_nolock(regs, 0);
> -		folio = virt_to_folio((void *)addr);
> -		if (unlikely(!folio_try_get(folio)))
> -			return;
> -		rc = uv_convert_from_secure(folio_to_phys(folio));
> -		if (!rc)
> -			clear_bit(PG_arch_1, &folio->flags.f);
> -		folio_put(folio);
> +		rc = uv_convert_from_secure(__pa(addr));

[Severity: Critical]
This is a pre-existing issue, but does this unconditionally destroy active
secure guest pages on speculative reads?

Looking at do_secure_storage_access(), uv_convert_from_secure() is called
here before checking fixup_exception(). Because the Ultravisor allows the
host to reclaim active guest memory, uv_convert_from_secure() succeeds for
guest memory, scrubs the page, and returns 0.

Since the conversion succeeds, the code skips calling
handle_fault_error_nolock() where the fixup is actually evaluated:

arch/s390/mm/fault.c:do_secure_storage_access() {
        ...
        rc = uv_convert_from_secure(__pa(addr));
        /*
         * There are some valid fixup types for kernel
         * accesses to donated secure memory. zeropad is one
         * of them.
         */
        if (rc)
                return handle_fault_error_nolock(regs, 0);
        ...
}

If a speculative kernel read (like the mentioned load_unaligned_zeropad)
or an eBPF probe read targets an address in the direct map belonging to a
secure guest, wouldn't this silently destroy the guest's memory instead of
safely executing the fixup?

>  		/*
>  		 * There are some valid fixup types for kernel
>  		 * accesses to donated secure memory. zeropad is one

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719130006.3882764-1-hca@linux.ibm.com?part=5

Re: [PATCH v3 1/6] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Physical pages are freed to the buddy allocator before their page table entries (PTEs) are cleared, creating a Use-After-Free and memory corruption window.
- [Medium] A sleeping memory allocation is performed inside a pte_fn_t callback under lazy_mmu_mode_enable(), violating core MM API contracts.
--

--- Patch [1]: [PATCH v3 1/6] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area ---
commit 2a65969cbdc861d6caeb95bb64f45d4facecff0e
Author: Heiko Carstens <hca@linux.ibm.com>

KVM: s390: pv: Use VM_SPARSE area for guest variable storage area

The guest variable storage area is allocated with vmalloc and then
donated to the ultravisor. Any kernel access to that area will result
in a secure storage access exception (aka fault).

This is a problem if such a memory area is read via /proc/kcore. This
causes an exception via vread_iter() and results in an unexpected short
read. Avoid this by allocating a custom VM_SPARSE area.

> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index a284f98d97168..d1cdc5ae5e270 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
[ ... ]
> @@ -209,6 +210,70 @@ int uv_convert_from_secure_pte(pte_t pte)
>  	return uv_convert_from_secure_folio(pfn_folio(pte_pfn(pte)));
>  }
>  
> +static int uv_free_range_cb(pte_t *ptep, unsigned long addr, void *data)
> +{
> +	pte_t pte = ptep_get(ptep);
> +
> +	if (!pte_present(pte))
> +		return 0;
> +	/*
> +	 * Note: do not update the pte here, since there is no code which
> +	 * accesses the memory range, besides bugs. The invalidation of ptes
> +	 * and TLB flushing is deferred like for regular vfree() calls.
> +	 */
> +	__free_page(pte_page(pte));

[Severity: High]
Is it safe to free the physical page back to the buddy allocator while the
PTE remains fully present in the init_mm page tables?

The commit message notes that vfree() defers PTE updates, but vfree() actually
clears the PTEs via remove_vm_area() before freeing the underlying pages.
Here, the pages are freed first, and the PTEs are cleared later when
free_vm_area() is called in uv_free_stor_var().

Could this create a window where the physical page is reallocated to another
process, but the current CPU still has a valid, writable PTE pointing to it?

Any stray kernel access or speculative execution through the old vmalloc
address could then silently corrupt the new owner's memory without faulting.

> +	return 0;
> +}
> +
> +void uv_free_stor_var(void *stor_var)
> +{
[ ... ]
> +}
> +EXPORT_SYMBOL_FOR_MODULES(uv_free_stor_var, "kvm");
> +
> +static int uv_alloc_range_cb(pte_t *ptep, unsigned long addr, void *data)
> +{
> +	struct page *page;
> +	pte_t pte;
> +
> +	page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);

[Severity: Medium]
Does this violate the cross-architecture pte_fn_t API contract by performing
a sleeping memory allocation?

This callback is executed by apply_to_page_range(), which wraps the pte_fn_t
callbacks in a lazy MMU mode section. The MM subsystem guidelines dictate
that lazy MMU mode implies a possible atomic context (as it disables
preemption on some architectures), forbidding these callbacks from sleeping.

While lazy_mmu_mode_enable() is currently a no-op on s390, the commit message
states s390 expects to gain full support for lazy MMU mode in the future.
Will this GFP_KERNEL allocation then introduce a preemption-disabled deadlock?

> +	if (!page)
> +		return -ENOMEM;
[ ... ]

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