Linux s390 Architecture development
 help / color / mirror / Atom feed
* [PATCH v2 0/4] s390: Reintroduce support for DCACHE_WORD_ACCESS
@ 2026-07-17  9:24 Heiko Carstens
  2026-07-17  9:24 ` [PATCH v2 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area Heiko Carstens
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Heiko Carstens @ 2026-07-17  9:24 UTC (permalink / raw)
  To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
	Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
	David Hildenbrand
  Cc: linux-s390, kvm, linux-kernel

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 (4):
  KVM: s390: pv: Use VM_SPARSE area for guest variable storage area
  s390/mm: Fix handling of secure storage access exceptions in vmalloc area
  s390/mm: Remove folio handling for secure storage access exceptions
  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                   | 10 ++--
 8 files changed, 118 insertions(+), 10 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area
  2026-07-17  9:24 [PATCH v2 0/4] s390: Reintroduce support for DCACHE_WORD_ACCESS Heiko Carstens
@ 2026-07-17  9:24 ` Heiko Carstens
  2026-07-17  9:40   ` sashiko-bot
  2026-07-17  9:24 ` [PATCH v2 2/4] s390/mm: Fix handling of secure storage access exceptions in vmalloc area Heiko Carstens
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Heiko Carstens @ 2026-07-17  9:24 UTC (permalink / raw)
  To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
	Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
	David Hildenbrand
  Cc: linux-s390, kvm, linux-kernel

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.

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/4] s390/mm: Fix handling of secure storage access exceptions in vmalloc area
  2026-07-17  9:24 [PATCH v2 0/4] s390: Reintroduce support for DCACHE_WORD_ACCESS Heiko Carstens
  2026-07-17  9:24 ` [PATCH v2 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area Heiko Carstens
@ 2026-07-17  9:24 ` Heiko Carstens
  2026-07-17  9:39   ` sashiko-bot
  2026-07-17  9:24 ` [PATCH v2 3/4] s390/mm: Remove folio handling for secure storage access exceptions Heiko Carstens
  2026-07-17  9:24 ` [PATCH v2 4/4] s390: Add support for DCACHE_WORD_ACCESS (again) Heiko Carstens
  3 siblings, 1 reply; 10+ messages in thread
From: Heiko Carstens @ 2026-07-17  9:24 UTC (permalink / raw)
  To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
	Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
	David Hildenbrand
  Cc: linux-s390, kvm, linux-kernel

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.

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 028aeb9c48d6..392e46742039 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/4] s390/mm: Remove folio handling for secure storage access exceptions
  2026-07-17  9:24 [PATCH v2 0/4] s390: Reintroduce support for DCACHE_WORD_ACCESS Heiko Carstens
  2026-07-17  9:24 ` [PATCH v2 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area Heiko Carstens
  2026-07-17  9:24 ` [PATCH v2 2/4] s390/mm: Fix handling of secure storage access exceptions in vmalloc area Heiko Carstens
@ 2026-07-17  9:24 ` Heiko Carstens
  2026-07-17  9:24 ` [PATCH v2 4/4] s390: Add support for DCACHE_WORD_ACCESS (again) Heiko Carstens
  3 siblings, 0 replies; 10+ messages in thread
From: Heiko Carstens @ 2026-07-17  9:24 UTC (permalink / raw)
  To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
	Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
	David Hildenbrand
  Cc: linux-s390, kvm, linux-kernel

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.

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 392e46742039..db77c8c0023e 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/4] s390: Add support for DCACHE_WORD_ACCESS (again)
  2026-07-17  9:24 [PATCH v2 0/4] s390: Reintroduce support for DCACHE_WORD_ACCESS Heiko Carstens
                   ` (2 preceding siblings ...)
  2026-07-17  9:24 ` [PATCH v2 3/4] s390/mm: Remove folio handling for secure storage access exceptions Heiko Carstens
@ 2026-07-17  9:24 ` Heiko Carstens
  2026-07-17  9:36   ` sashiko-bot
  3 siblings, 1 reply; 10+ messages in thread
From: Heiko Carstens @ 2026-07-17  9:24 UTC (permalink / raw)
  To: Alexander Gordeev, Sven Schnelle, Vasily Gorbik,
	Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
	David Hildenbrand
  Cc: linux-s390, kvm, linux-kernel

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.

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 4/4] s390: Add support for DCACHE_WORD_ACCESS (again)
  2026-07-17  9:24 ` [PATCH v2 4/4] s390: Add support for DCACHE_WORD_ACCESS (again) Heiko Carstens
@ 2026-07-17  9:36   ` sashiko-bot
  2026-07-17 10:20     ` Heiko Carstens
  0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-07-17  9:36 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: linux-s390, kvm, Vasily Gorbik, Alexander Gordeev, Heiko Carstens

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing earlyclobber constraint on the output register in `load_unaligned_zeropad` allows GCC to use the same register for input and output, leading to a kernel crash during secure storage access exception handling.
--

--- Patch [4]: [PATCH v2 4/4] s390: Add support for DCACHE_WORD_ACCESS (again) ---
commit 48318c86a5b130c4f8df66b5d5c16dda32e7448d
Author: Heiko Carstens <hca@linux.ibm.com>

s390: Add support for DCACHE_WORD_ACCESS (again)

This commit implements load_unaligned_zeropad() and enables
DCACHE_WORD_ACCESS to speed up string operations in fs/dcache.c and
fs/namei.c. It reinstates this support after addressing secure storage
access exception cases.

> diff --git a/arch/s390/include/asm/word-at-a-time.h b/arch/s390/include/asm/word-at-a-time.h
> --- a/arch/s390/include/asm/word-at-a-time.h
> +++ b/arch/s390/include/asm/word-at-a-time.h

[ ... ]

> +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)

[Severity: High]
Could GCC allocate the same register for both the [addr] input and the
[data] output since there is no earlyclobber modifier here?

If a page-crosser triggers a terminating secure storage access exception,
the hardware leaves the result register in an unpredictable state.

The exception handler ex_handler_zeropad() relies on reading the original
address from the register specified by [addr] to compute the valid bytes.
If GCC uses the exact same register for both input and output, the handler
will read a corrupted address from the clobbered register, dereference it,
and cause a kernel panic.

Should this use the earlyclobber constraint "=&d" for the data output to
ensure the address register is preserved during a terminating exception?

> +		: [addr] "a" (addr), "m" (*(unsigned long *)addr));
> +	return data;
> +}
> +
>  #endif /* _ASM_WORD_AT_A_TIME_H */

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/4] s390/mm: Fix handling of secure storage access exceptions in vmalloc area
  2026-07-17  9:24 ` [PATCH v2 2/4] s390/mm: Fix handling of secure storage access exceptions in vmalloc area Heiko Carstens
@ 2026-07-17  9:39   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-17  9:39 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Vasily Gorbik, Alexander Gordeev, kvm, Heiko Carstens, linux-s390

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

Pre-existing issues:
- [High] `do_secure_storage_access` uses `find_vma()` to locate the VMA but fails to verify that the faulting address is within the returned VMA's bounds, allowing an unprivileged user to reliably trigger a `WARN_ON_ONCE` via a race condition.
--

--- Patch [2]: [PATCH v2 2/4] s390/mm: Fix handling of secure storage access exceptions in vmalloc area ---
commit 09ae7bc2c3bf1f10251c33af84acf243e29af836
Author: Heiko Carstens <hca@linux.ibm.com>

s390/mm: Fix handling of secure storage access exceptions in vmalloc area

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
> index 028aeb9c48d6f..392e467420392 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;

[ ... ]

>  	} else {
>  		if (faulthandler_disabled())
>  			return handle_fault_error_nolock(regs, 0);
>  		mm = current->mm;
>  		mmap_read_lock(mm);
>  		vma = find_vma(mm, addr);
>  		if (!vma)
>  			return handle_fault_error(regs, SEGV_MAPERR);
>  		folio = folio_walk_start(&fw, vma, addr, 0);

[Severity: High]
This is a pre-existing issue, but does using find_vma() here allow a race
condition that could trigger a kernel warning in folio_walk_start()?

find_vma() returns the first VMA where vma->vm_end > addr, but it doesn't
guarantee that vma->vm_start <= addr. If a concurrent unmap happens and the
exact VMA is removed before mmap_read_lock() is acquired, find_vma() might
return the next VMA in the address space.

Passing an out-of-bounds VMA to folio_walk_start() violates its API contract
and triggers a WARN_ON_ONCE().

Would using vma_lookup() instead properly validate that the address falls
within the VMA's bounds?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area
  2026-07-17  9:24 ` [PATCH v2 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area Heiko Carstens
@ 2026-07-17  9:40   ` sashiko-bot
  2026-07-17 10:32     ` Heiko Carstens
  0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-07-17  9:40 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Alexander Gordeev, linux-s390, Vasily Gorbik, kvm, Heiko Carstens

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 kernel page table entries (PTEs) are cleared and TLBs are flushed, creating a dangerous Use-After-Free window.
- [High] The page table walker callback `uv_alloc_range_cb` performs a sleeping memory allocation (`GFP_KERNEL_ACCOUNT`) while holding a spinlock.
--

--- Patch [1]: [PATCH v2 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area ---
commit 3aa87ec52bc96aa3b06650c2d3924417b23be4ca
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. This commit changes the allocation to use
a VM_SPARSE area so that if the area is read via /proc/kcore,
vread_iter() returns zeroes instead of causing an exception.

> 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]
Does this create a window where the physical page is freed to the buddy
allocator but still mapped in the kernel page tables?

If another CPU reallocates the freed page before free_vm_area() unmaps and
flushes the TLB, could speculative execution or a stray kernel access to the
old virtual address read or corrupt the new owner's data?

> +	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);

[Severity: High]
Can this allocation sleep while holding a spinlock?

The apply_to_page_range() function loops over the page tables and acquires
the page table spinlock (ptl) via pte_alloc_map_lock() before invoking the
callback.

Since GFP_KERNEL_ACCOUNT implies __GFP_DIRECT_RECLAIM, which can sleep
and invoke the scheduler, could calling this while holding the page table
spinlock trigger a scheduling while atomic bug or deadlock?

> +	if (!page)
> +		return -ENOMEM;
> +	pte = __pte(page_to_phys(page) | pgprot_val(PAGE_KERNEL));
> +	set_pte(ptep, pte);
> +	return 0;
> +}

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 4/4] s390: Add support for DCACHE_WORD_ACCESS (again)
  2026-07-17  9:36   ` sashiko-bot
@ 2026-07-17 10:20     ` Heiko Carstens
  0 siblings, 0 replies; 10+ messages in thread
From: Heiko Carstens @ 2026-07-17 10:20 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-s390, kvm, Vasily Gorbik, Alexander Gordeev

On Fri, Jul 17, 2026 at 09:36:32AM +0000, sashiko-bot@kernel.org wrote:
> > +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)
> 
> [Severity: High]
> Could GCC allocate the same register for both the [addr] input and the
> [data] output since there is no earlyclobber modifier here?
> 
> If a page-crosser triggers a terminating secure storage access exception,
> the hardware leaves the result register in an unpredictable state.
> 
> The exception handler ex_handler_zeropad() relies on reading the original
> address from the register specified by [addr] to compute the valid bytes.
> If GCC uses the exact same register for both input and output, the handler
> will read a corrupted address from the clobbered register, dereference it,
> and cause a kernel panic.
> 
> Should this use the earlyclobber constraint "=&d" for the data output to
> ensure the address register is preserved during a terminating exception?

This doesn't make sense to me. Secure storage access exceptions are
nullifying per architecture, not terminating.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area
  2026-07-17  9:40   ` sashiko-bot
@ 2026-07-17 10:32     ` Heiko Carstens
  0 siblings, 0 replies; 10+ messages in thread
From: Heiko Carstens @ 2026-07-17 10:32 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Alexander Gordeev, linux-s390, Vasily Gorbik, kvm

On Fri, Jul 17, 2026 at 09:40:07AM +0000, sashiko-bot@kernel.org wrote:
> > +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]
> Does this create a window where the physical page is freed to the buddy
> allocator but still mapped in the kernel page tables?
> 
> If another CPU reallocates the freed page before free_vm_area() unmaps and
> flushes the TLB, could speculative execution or a stray kernel access to the
> old virtual address read or corrupt the new owner's data?

Intended per comment above.

> > +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: High]
> Can this allocation sleep while holding a spinlock?
> 
> The apply_to_page_range() function loops over the page tables and acquires
> the page table spinlock (ptl) via pte_alloc_map_lock() before invoking the
> callback.
> 
> Since GFP_KERNEL_ACCOUNT implies __GFP_DIRECT_RECLAIM, which can sleep
> and invoke the scheduler, could calling this while holding the page table
> spinlock trigger a scheduling while atomic bug or deadlock?

Odd... this is not true if apply_to_page_range() is called with init_mm.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-07-17 10:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17  9:24 [PATCH v2 0/4] s390: Reintroduce support for DCACHE_WORD_ACCESS Heiko Carstens
2026-07-17  9:24 ` [PATCH v2 1/4] KVM: s390: pv: Use VM_SPARSE area for guest variable storage area Heiko Carstens
2026-07-17  9:40   ` sashiko-bot
2026-07-17 10:32     ` Heiko Carstens
2026-07-17  9:24 ` [PATCH v2 2/4] s390/mm: Fix handling of secure storage access exceptions in vmalloc area Heiko Carstens
2026-07-17  9:39   ` sashiko-bot
2026-07-17  9:24 ` [PATCH v2 3/4] s390/mm: Remove folio handling for secure storage access exceptions Heiko Carstens
2026-07-17  9:24 ` [PATCH v2 4/4] s390: Add support for DCACHE_WORD_ACCESS (again) Heiko Carstens
2026-07-17  9:36   ` sashiko-bot
2026-07-17 10:20     ` Heiko Carstens

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox