* [PATCH 1/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer
2026-09-02 6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
@ 2026-09-02 6:15 ` Mike Rapoport (Microsoft)
2026-09-02 6:19 ` sashiko-bot
2026-09-02 6:15 ` [PATCH 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer Mike Rapoport (Microsoft)
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Mike Rapoport (Microsoft) @ 2026-09-02 6:15 UTC (permalink / raw)
To: Christian Borntraeger, Claudio Imbrenda, Janosch Frank
Cc: Alexander Gordeev, David Hildenbrand, Heiko Carstens,
Mike Rapoport, Sven Schnelle, Vasily Gorbik, Vlastimil Babka, kvm,
linux-kernel, linux-mm, linux-s390
handle_sthyi() allocates the buffer that receives the STHYI response
block before it is copied to the guest.
This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.
Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.
For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.
Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/s390/kvm/s390/intercept.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/s390/kvm/s390/intercept.c b/arch/s390/kvm/s390/intercept.c
index ca1205dfac8b0..aaf9828d1f44d 100644
--- a/arch/s390/kvm/s390/intercept.c
+++ b/arch/s390/kvm/s390/intercept.c
@@ -11,6 +11,7 @@
#include <linux/kvm_host.h>
#include <linux/errno.h>
#include <linux/pagemap.h>
+#include <linux/slab.h>
#include <asm/asm-offsets.h>
#include <asm/irq.h>
@@ -438,13 +439,13 @@ int handle_sthyi(struct kvm_vcpu *vcpu)
if (!kvm_s390_pv_cpu_is_protected(vcpu) && (addr & ~PAGE_MASK))
return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
- sctns = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
+ sctns = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
if (!sctns)
return -ENOMEM;
cc = sthyi_fill(sctns, &rc);
if (cc < 0) {
- free_page((unsigned long)sctns);
+ kfree(sctns);
return cc;
}
out:
@@ -454,13 +455,13 @@ int handle_sthyi(struct kvm_vcpu *vcpu)
} else {
r = write_guest(vcpu, addr, reg2, sctns, PAGE_SIZE);
if (r) {
- free_page((unsigned long)sctns);
+ kfree(sctns);
return kvm_s390_inject_prog_cond(vcpu, r);
}
}
}
- free_page((unsigned long)sctns);
+ kfree(sctns);
vcpu->run->s.regs.gprs[reg2 + 1] = rc;
kvm_s390_set_psw_cc(vcpu, cc);
return r;
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer
2026-09-02 6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
2026-09-02 6:15 ` [PATCH 1/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer Mike Rapoport (Microsoft)
@ 2026-09-02 6:15 ` Mike Rapoport (Microsoft)
2026-09-02 6:22 ` sashiko-bot
2026-09-02 6:15 ` [PATCH 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB Mike Rapoport (Microsoft)
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Mike Rapoport (Microsoft) @ 2026-09-02 6:15 UTC (permalink / raw)
To: Christian Borntraeger, Claudio Imbrenda, Janosch Frank
Cc: Alexander Gordeev, David Hildenbrand, Heiko Carstens,
Mike Rapoport, Sven Schnelle, Vasily Gorbik, Vlastimil Babka, kvm,
linux-kernel, linux-mm, linux-s390
handle_stsi() allocates the buffer that receives the STSI response block
before it is copied to the guest.
This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.
Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.
For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.
While on it, make the buffer a void pointer to get rid of the casts.
Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/s390/kvm/s390/priv.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
index b3cb2c2c3aa71..856939acf012f 100644
--- a/arch/s390/kvm/s390/priv.c
+++ b/arch/s390/kvm/s390/priv.c
@@ -14,6 +14,7 @@
#include <linux/mm_types.h>
#include <linux/pgtable.h>
#include <linux/io.h>
+#include <linux/slab.h>
#include <asm/asm-offsets.h>
#include <asm/facility.h>
#include <asm/current.h>
@@ -869,7 +870,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
int fc = (vcpu->run->s.regs.gprs[0] & 0xf0000000) >> 28;
int sel1 = vcpu->run->s.regs.gprs[0] & 0xff;
int sel2 = vcpu->run->s.regs.gprs[1] & 0xffff;
- unsigned long mem = 0;
+ void *mem = NULL;
u64 operand2;
int rc = 0;
u8 ar;
@@ -911,19 +912,19 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
switch (fc) {
case 1: /* same handling for 1 and 2 */
case 2:
- mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
+ mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
if (!mem)
goto out_no_data;
- if (stsi((void *) mem, fc, sel1, sel2))
+ if (stsi(mem, fc, sel1, sel2))
goto out_no_data;
break;
case 3:
if (sel1 != 2 || sel2 != 2)
goto out_no_data;
- mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
+ mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
if (!mem)
goto out_no_data;
- handle_stsi_3_2_2(vcpu, (void *) mem);
+ handle_stsi_3_2_2(vcpu, mem);
break;
case 15: /* fc 15 is fully handled in userspace */
insert_stsi_usr_data(vcpu, operand2, ar, fc, sel1, sel2);
@@ -931,10 +932,10 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
return -EREMOTE;
}
if (kvm_s390_pv_cpu_is_protected(vcpu)) {
- memcpy(sida_addr(vcpu->arch.sie_block), (void *)mem, PAGE_SIZE);
+ memcpy(sida_addr(vcpu->arch.sie_block), mem, PAGE_SIZE);
rc = 0;
} else {
- rc = write_guest(vcpu, operand2, ar, (void *)mem, PAGE_SIZE);
+ rc = write_guest(vcpu, operand2, ar, mem, PAGE_SIZE);
}
if (rc) {
rc = kvm_s390_inject_prog_cond(vcpu, rc);
@@ -945,14 +946,14 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
rc = -EREMOTE;
}
trace_kvm_s390_handle_stsi(vcpu, fc, sel1, sel2, operand2);
- free_page(mem);
+ kfree(mem);
kvm_s390_set_psw_cc(vcpu, 0);
vcpu->run->s.regs.gprs[0] = 0;
return rc;
out_no_data:
kvm_s390_set_psw_cc(vcpu, 3);
out:
- free_page(mem);
+ kfree(mem);
return rc;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer
2026-09-02 6:15 ` [PATCH 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer Mike Rapoport (Microsoft)
@ 2026-09-02 6:22 ` sashiko-bot
2026-09-02 7:55 ` Mike Rapoport
0 siblings, 1 reply; 11+ messages in thread
From: sashiko-bot @ 2026-09-02 6:22 UTC (permalink / raw)
To: Mike Rapoport (Microsoft)
Cc: Heiko Carstens, Alexander Gordeev, Vasily Gorbik, linux-s390,
Christian Borntraeger, kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Replacing get_zeroed_page() with kzalloc() breaks the hardware 4K alignment requirement for the STSI buffer, creating a Guest-to-Host Denial of Service (DoS) vulnerability.
--
commit bcd4e89cdbdb3a1d43d3845b698f4de5d334121d
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>
KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer
This commit replaces get_zeroed_page() with kzalloc() for allocating
the STSI response block buffer in the s390 KVM subsystem.
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
> index b3cb2c2c3aa71..856939acf012f 100644
> --- a/arch/s390/kvm/s390/priv.c
> +++ b/arch/s390/kvm/s390/priv.c
[ ... ]
> @@ -911,19 +912,19 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
> switch (fc) {
> case 1: /* same handling for 1 and 2 */
> case 2:
> - mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
> + mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
> if (!mem)
> goto out_no_data;
> - if (stsi((void *) mem, fc, sel1, sel2))
> + if (stsi(mem, fc, sel1, sel2))
[Severity: Critical]
Does this allocation guarantee the 4K alignment required by the STSI hardware
instruction?
While kzalloc() for PAGE_SIZE might often return page-aligned memory, it is
not strictly guaranteed, especially if slab debugging (CONFIG_SLUB_DEBUG)
with redzoning or CONFIG_KASAN is enabled.
If the allocated memory is not 4K-aligned, the inline stsi() assembly will
trigger a specification exception (Program Check 6). Because there is no
exception table entry for this inline assembly, this will crash the host
kernel.
Since an unprivileged KVM guest can readily execute the STSI instruction
and trap to the host, could this allow a guest-to-host denial of service?
> goto out_no_data;
> break;
> case 3:
> if (sel1 != 2 || sel2 != 2)
> goto out_no_data;
> - mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
> + mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
> if (!mem)
> goto out_no_data;
> - handle_stsi_3_2_2(vcpu, (void *) mem);
> + handle_stsi_3_2_2(vcpu, mem);
[Severity: Critical]
Similarly, passing this potentially misaligned memory into handle_stsi_3_2_2()
can trigger the same specification exception when it executes the STSI
instruction internally.
Should these allocations continue to use get_zeroed_page() to ensure the
strict page alignment required by the hardware?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-s390-kvm-v1-0-3bc0986550b1@kernel.org?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer
2026-09-02 6:22 ` sashiko-bot
@ 2026-09-02 7:55 ` Mike Rapoport
0 siblings, 0 replies; 11+ messages in thread
From: Mike Rapoport @ 2026-09-02 7:55 UTC (permalink / raw)
To: sashiko-reviews
Cc: Heiko Carstens, Alexander Gordeev, Vasily Gorbik, linux-s390,
Christian Borntraeger, kvm
On Wed, Sep 02, 2026 at 06:22:22AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Replacing get_zeroed_page() with kzalloc() breaks the hardware 4K alignment requirement for the STSI buffer, creating a Guest-to-Host Denial of Service (DoS) vulnerability.
> --
>
> commit bcd4e89cdbdb3a1d43d3845b698f4de5d334121d
> Author: Mike Rapoport (Microsoft) <rppt@kernel.org>
>
> KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer
>
> This commit replaces get_zeroed_page() with kzalloc() for allocating
> the STSI response block buffer in the s390 KVM subsystem.
>
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
>
> > diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
> > index b3cb2c2c3aa71..856939acf012f 100644
> > --- a/arch/s390/kvm/s390/priv.c
> > +++ b/arch/s390/kvm/s390/priv.c
> [ ... ]
> > @@ -911,19 +912,19 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
> > switch (fc) {
> > case 1: /* same handling for 1 and 2 */
> > case 2:
> > - mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
> > + mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
> > if (!mem)
> > goto out_no_data;
> > - if (stsi((void *) mem, fc, sel1, sel2))
> > + if (stsi(mem, fc, sel1, sel2))
>
> [Severity: Critical]
> Does this allocation guarantee the 4K alignment required by the STSI hardware
> instruction?
>
> While kzalloc() for PAGE_SIZE might often return page-aligned memory, it is
> not strictly guaranteed, especially if slab debugging (CONFIG_SLUB_DEBUG)
> with redzoning or CONFIG_KASAN is enabled.
It's strictly guaranteed, see commit 59bb47985c1db ("mm, sl[aou]b:
guarantee natural alignment for kmalloc(power-of-two)")
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB
2026-09-02 6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
2026-09-02 6:15 ` [PATCH 1/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer Mike Rapoport (Microsoft)
2026-09-02 6:15 ` [PATCH 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer Mike Rapoport (Microsoft)
@ 2026-09-02 6:15 ` Mike Rapoport (Microsoft)
2026-09-02 6:28 ` sashiko-bot
2026-09-02 6:15 ` [PATCH 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA Mike Rapoport (Microsoft)
2026-09-02 11:06 ` [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Claudio Imbrenda
4 siblings, 1 reply; 11+ messages in thread
From: Mike Rapoport (Microsoft) @ 2026-09-02 6:15 UTC (permalink / raw)
To: Christian Borntraeger, Claudio Imbrenda, Janosch Frank
Cc: Alexander Gordeev, David Hildenbrand, Heiko Carstens,
Mike Rapoport, Sven Schnelle, Vasily Gorbik, Vlastimil Babka, kvm,
linux-kernel, linux-mm, linux-s390
kvm_s390_gib_init() allocates the guest information block (GIB). The GIB
is passed to the hardware as a physical address and must be page aligned.
kmalloc() guarantees that a power of two sized allocation is aligned to
its size, so a PAGE_SIZE allocation is page aligned as well.
This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.
Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.
For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.
Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/s390/kvm/s390/interrupt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/s390/kvm/s390/interrupt.c b/arch/s390/kvm/s390/interrupt.c
index 0381ae9817035..02065fb8d053e 100644
--- a/arch/s390/kvm/s390/interrupt.c
+++ b/arch/s390/kvm/s390/interrupt.c
@@ -3689,7 +3689,7 @@ void kvm_s390_gib_destroy(void)
}
chsc_sgib(0);
unregister_adapter_interrupt(&gib_alert_irq);
- free_page((unsigned long)gib);
+ kfree(gib);
gib = NULL;
}
@@ -3703,7 +3703,7 @@ int __init kvm_s390_gib_init(u8 nisc)
goto out;
}
- gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
+ gib = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT | GFP_DMA);
if (!gib) {
rc = -ENOMEM;
goto out;
@@ -3722,7 +3722,7 @@ int __init kvm_s390_gib_init(u8 nisc)
gib_origin = virt_to_phys(gib);
if (chsc_sgib(gib_origin)) {
pr_err("Associating the GIB with the AIV facility failed\n");
- free_page((unsigned long)gib);
+ kfree(gib);
gib = NULL;
rc = -EIO;
goto out_unreg_gal;
@@ -3742,7 +3742,7 @@ int __init kvm_s390_gib_init(u8 nisc)
out_unreg_gal:
unregister_adapter_interrupt(&gib_alert_irq);
out_free_gib:
- free_page((unsigned long)gib);
+ kfree(gib);
gib = NULL;
out:
return rc;
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB
2026-09-02 6:15 ` [PATCH 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB Mike Rapoport (Microsoft)
@ 2026-09-02 6:28 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-02 6:28 UTC (permalink / raw)
To: Mike Rapoport (Microsoft)
Cc: Alexander Gordeev, Heiko Carstens, Vasily Gorbik, kvm,
Christian Borntraeger, linux-s390
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Hardware Use-After-Free due to missing `chsc_sgib(0)` on `kvm_s390_pci_aen_init` failure path.
- [High] Race condition in `kvm_s390_gib_init()` error path leading to a NULL pointer dereference.
--
commit b7e0a2c7fc946fc151e2d875aa6df00a71151750
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>
Subject: KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB
This commit replaces the use of get_zeroed_page() with kzalloc() and
free_page() with kfree() for the guest information block (GIB) allocation.
It notes that kmalloc() guarantees page alignment for power-of-two sizes,
making it suitable for this buffer without requiring direct page allocator
access.
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> diff --git a/arch/s390/kvm/s390/interrupt.c b/arch/s390/kvm/s390/interrupt.c
> index 0381ae9817035..02065fb8d053e 100644
> --- a/arch/s390/kvm/s390/interrupt.c
> +++ b/arch/s390/kvm/s390/interrupt.c
[ ... ]
> @@ -3722,7 +3722,7 @@ int __init kvm_s390_gib_init(u8 nisc)
> gib_origin = virt_to_phys(gib);
> if (chsc_sgib(gib_origin)) {
> pr_err("Associating the GIB with the AIV facility failed\n");
> - free_page((unsigned long)gib);
> + kfree(gib);
> gib = NULL;
[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition in
this error path in kvm_s390_gib_init().
When chsc_sgib() fails, the code frees gib and sets it to NULL. However,
this happens before calling unregister_adapter_interrupt(&gib_alert_irq) at
the out_unreg_gal label.
Since the interrupt handler is still registered and active, if another device
sharing the same ISC triggers an interrupt during this window, could
the gib_alert_irq_handler execute and call process_gib_alert_list()?
If it does, it looks like it will unconditionally dereference the NULL gib
pointer.
> rc = -EIO;
> goto out_unreg_gal;
[ ... ]
> @@ -3742,7 +3742,7 @@ int __init kvm_s390_gib_init(u8 nisc)
> out_unreg_gal:
> unregister_adapter_interrupt(&gib_alert_irq);
> out_free_gib:
> - free_page((unsigned long)gib);
> + kfree(gib);
[Severity: Critical]
This is also a pre-existing issue, but does this cleanup path result in a
hardware use-after-free if kvm_s390_pci_aen_init() fails?
In kvm_s390_gib_init(), if the GIB is successfully registered with the
hardware via chsc_sgib(gib_origin), but the subsequent call to
kvm_s390_pci_aen_init() fails, the code jumps to out_unreg_gal:
if (kvm_s390_pci_interp_allowed()) {
if (kvm_s390_pci_aen_init(nisc)) {
pr_err("Initializing AEN for PCI failed\n");
rc = -EIO;
goto out_unreg_gal;
}
}
This skips calling chsc_sgib(0) to tear down the hardware registration.
When we reach out_free_gib and free the memory backing the GIB, doesn't
the hardware AIV facility still retain the physical pointer to the freed
memory?
> gib = NULL;
> out:
> return rc;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-s390-kvm-v1-0-3bc0986550b1@kernel.org?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA
2026-09-02 6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
` (2 preceding siblings ...)
2026-09-02 6:15 ` [PATCH 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB Mike Rapoport (Microsoft)
@ 2026-09-02 6:15 ` Mike Rapoport (Microsoft)
2026-09-02 6:26 ` sashiko-bot
2026-09-02 11:06 ` [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Claudio Imbrenda
4 siblings, 1 reply; 11+ messages in thread
From: Mike Rapoport (Microsoft) @ 2026-09-02 6:15 UTC (permalink / raw)
To: Christian Borntraeger, Claudio Imbrenda, Janosch Frank
Cc: Alexander Gordeev, David Hildenbrand, Heiko Carstens,
Mike Rapoport, Sven Schnelle, Vasily Gorbik, Vlastimil Babka, kvm,
linux-kernel, linux-mm, linux-s390
kvm_arch_init_vm() allocates struct sie_page2 that holds the facility
list and the crypto control block of a VM and kvm_s390_vcpu_setup_cmma()
allocates the CMMA collaborative memory management block referenced by
the SIE control block. Both are passed to the hardware as physical
addresses and must be page aligned.
kmalloc() guarantees that a power of two sized allocation is aligned to
its size, so a PAGE_SIZE allocation is page aligned as well.
These buffers can be allocated with kmalloc() as there's nothing special
about them to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.
Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.
For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.
The sie_page allocated in kvm_arch_vcpu_create() is left alone because it
is mapped to userspace with virt_to_page() in kvm_arch_vcpu_fault() and
therefore really needs a struct page.
Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/s390/kvm/s390/s390.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/s390/kvm/s390/s390.c b/arch/s390/kvm/s390/s390.c
index b0839e887221e..8ec505060f8c0 100644
--- a/arch/s390/kvm/s390/s390.c
+++ b/arch/s390/kvm/s390/s390.c
@@ -3273,8 +3273,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
goto out_err;
BUILD_BUG_ON(sizeof(struct sie_page2) != 4096);
- kvm->arch.sie_page2 =
- (struct sie_page2 *) get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
+ kvm->arch.sie_page2 = kzalloc_obj(*kvm->arch.sie_page2,
+ GFP_KERNEL_ACCOUNT | GFP_DMA);
if (!kvm->arch.sie_page2)
goto out_err;
@@ -3369,7 +3369,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
return 0;
out_err:
- free_page((unsigned long)kvm->arch.sie_page2);
+ kfree(kvm->arch.sie_page2);
debug_unregister(kvm->arch.dbf);
sca_dispose(kvm);
KVM_EVENT(3, "creation of vm failed: %d", rc);
@@ -3427,7 +3427,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
mmu_notifier_unregister(&kvm->arch.pv.mmu_notifier, kvm->mm);
debug_unregister(kvm->arch.dbf);
- free_page((unsigned long)kvm->arch.sie_page2);
+ kfree(kvm->arch.sie_page2);
kvm_s390_destroy_adapters(kvm);
kvm_s390_clear_float_irqs(kvm);
kvm_s390_vsie_destroy(kvm);
@@ -3657,13 +3657,13 @@ static void kvm_s390_vcpu_crypto_setup(struct kvm_vcpu *vcpu)
void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
{
if (vcpu->arch.sie_block->cbrlo)
- free_page((unsigned long)phys_to_virt(vcpu->arch.sie_block->cbrlo));
+ kfree(phys_to_virt(vcpu->arch.sie_block->cbrlo));
vcpu->arch.sie_block->cbrlo = 0;
}
int kvm_s390_vcpu_setup_cmma(struct kvm_vcpu *vcpu)
{
- void *cbrlo_page = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
+ void *cbrlo_page = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
if (!cbrlo_page)
return -ENOMEM;
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA
2026-09-02 6:15 ` [PATCH 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA Mike Rapoport (Microsoft)
@ 2026-09-02 6:26 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-02 6:26 UTC (permalink / raw)
To: Mike Rapoport (Microsoft)
Cc: linux-s390, Alexander Gordeev, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Invalid free in SLUB allocator due to passing an unaligned interior pointer containing hardware-injected offset bits.
--
commit 7888b46f989fd0725fe9f7dbcee943460aa8c059
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>
KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA
This commit replaces usages of get_zeroed_page() with kzalloc() and
free_page() with kfree() for allocating sie_page2 and CMMA blocks in the
s390 KVM subsystem. It aims to simplify the API usage since power-of-two
sized kmalloc() allocations are aligned to their size.
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> diff --git a/arch/s390/kvm/s390/s390.c b/arch/s390/kvm/s390/s390.c
> index b0839e887221e..8ec505060f8c0 100644
> --- a/arch/s390/kvm/s390/s390.c
> +++ b/arch/s390/kvm/s390/s390.c
[ ... ]
> @@ -3657,13 +3657,13 @@ static void kvm_s390_vcpu_crypto_setup(struct kvm_vcpu *vcpu)
> void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
> {
> if (vcpu->arch.sie_block->cbrlo)
> - free_page((unsigned long)phys_to_virt(vcpu->arch.sie_block->cbrlo));
> + kfree(phys_to_virt(vcpu->arch.sie_block->cbrlo));
[Severity: High]
Does this code pass an unaligned interior pointer to the SLUB allocator?
During execution, the SIE hardware modifies the lower 12 bits of cbrlo to
store the Next CBR Entry Offset (NCEO). On VM teardown, if the hardware
has updated this count and it wasn't explicitly reset, the unmasked cbrlo
value is passed to kfree().
The previous free_page() implementation safely dropped these offset bits
because virt_to_page() implicitly applies PAGE_SHIFT. However, kfree()
requires the exact, unmodified pointer originally returned by the allocator.
Passing an interior pointer can lead to immediate freelist corruption and a
subsequent kernel panic.
> vcpu->arch.sie_block->cbrlo = 0;
> }
>
> int kvm_s390_vcpu_setup_cmma(struct kvm_vcpu *vcpu)
> {
> - void *cbrlo_page = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
> + void *cbrlo_page = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
>
> if (!cbrlo_page)
> return -ENOMEM;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-s390-kvm-v1-0-3bc0986550b1@kernel.org?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc()
2026-09-02 6:15 [PATCH 0/4] KVM: s390: replace page allocator calls with kzalloc() Mike Rapoport (Microsoft)
` (3 preceding siblings ...)
2026-09-02 6:15 ` [PATCH 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA Mike Rapoport (Microsoft)
@ 2026-09-02 11:06 ` Claudio Imbrenda
4 siblings, 0 replies; 11+ messages in thread
From: Claudio Imbrenda @ 2026-09-02 11:06 UTC (permalink / raw)
To: Mike Rapoport (Microsoft)
Cc: Christian Borntraeger, Janosch Frank, Alexander Gordeev,
David Hildenbrand, Heiko Carstens, Sven Schnelle, Vasily Gorbik,
Vlastimil Babka, kvm, linux-kernel, linux-mm, linux-s390
On Wed, 02 Sep 2026 09:15:12 +0300
"Mike Rapoport (Microsoft)" <rppt@kernel.org> wrote:
> This is a (small) part of larger work of replacing page allocator calls
> with kmalloc.
>
> My initial intention a few month ago was to remove ugly casts [1], but then
> willy pointed out that Linus objected to something like this [2] and it
> looks like more than a decade old technical debt.
>
> Largely, anything that doesn't need struct page (or a memdesc in the
> future) should just use kmalloc() or kvmalloc() to allocate memory.
> kmalloc() guarantees alignment, physical contiguity and working
> virt_to_phys() and beside nicer API that returns void * on alloc and
> doesn't require to know the allocation size on free, kmalloc() provides
> better debugging capabilities than page allocator.
>
> Another thing is that touching these allocation sites gives the reviewers
> opportunity to see if a PAGE_SIZE buffer is actually needed or maybe
> another size is appropriate.
>
> For larger allocations that don't need physically contiguous memory
> kvmalloc() can be a better option that __get_free_pages() because under
> memory pressure it's is easier to allocate several order-0 pages than a
> physically contiguous chunk with the same number of pages.
>
> And last, but not least, removing needless calls to page allocator should
> help with memdesc (aka project folio) conversion. There will be way less
> places to audit to see if the user was actually using struct page.
I have some objections to this series, but not because of what you are
trying to do (which is actually nice).
I understand that you probably wanted to touch as little code as
possible, but now since you're rewriting the allocations to use
kmalloc.... I'd like them to be converted to use the __free(kvmalloc)
system. It will make the code smaller, easier to read and understand,
less prone to future errors, etc.
In some places the whole code flow can be simplified a lot.
> Also in git:
> https://git.kernel.org/pub/scm/linux/kernel/git/rppt/linux.git gfp-to-kmalloc/s390-kvm
>
> [1] https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/
> [2] https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/
>
> ---
> Mike Rapoport (Microsoft) (4):
> KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer
> KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer
> KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB
> KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA
>
> arch/s390/kvm/s390/intercept.c | 9 +++++----
> arch/s390/kvm/s390/interrupt.c | 8 ++++----
> arch/s390/kvm/s390/priv.c | 19 ++++++++++---------
> arch/s390/kvm/s390/s390.c | 12 ++++++------
> 4 files changed, 25 insertions(+), 23 deletions(-)
> ---
> base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
> change-id: 20260828-s390-kvm-2015b0d777dc
>
> --
> Sincerely yours,
> Mike.
>
^ permalink raw reply [flat|nested] 11+ messages in thread