[PATCH 0/3] KVM: s390: more fixes

[PATCH 0/3] KVM: s390: more fixes

[Christian Borntraeger]

3 more small fixes all over the place.

Christian Borntraeger (3):
  KVM: s390: remove user triggerable WARN_ON
  KVM: s390: zero initialize irq for inject_pfault_token
  KVM: s390: memory leak in guest debug handling

 arch/s390/kvm/guestdbg.c  | 1 +
 arch/s390/kvm/interrupt.c | 2 +-
 arch/s390/kvm/kvm-s390.c  | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.53.0

[PATCH 1/3] KVM: s390: remove user triggerable WARN_ON

[Christian Borntraeger]

pin_map_page() fails legitimately whenever the userspace mapping behind
the adapter route has gone away, e.g. when the VMM unmaps that memory.
As this can happen without a kernel programming error, remove the
WARN_ON.

Fixes: 8c0141b7c13c ("KVM: s390: Enable adapter_indicators_set to use mapped pages")
Cc: Douglas Freimuth <freimuth@linux.ibm.com>
Cc: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 8f24bcd1a6d3..23f0e0821474 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -3013,7 +3013,7 @@ static int adapter_indicators_set(struct kvm *kvm,
 	if (!summary_info) {
 		spin_unlock_irqrestore(&adapter->maps_lock, flags);
 		summary_page = pin_map_page(kvm, adapter_int->summary_addr, 0);
-		if (WARN_ON_ONCE(!summary_page))
+		if (!summary_page)
 			return -1;
 		idx = srcu_read_lock(&kvm->srcu);
 		map = page_address(summary_page);
-- 
2.53.0

[PATCH 2/3] KVM: s390: zero initialize irq for inject_pfault_token

[Christian Borntraeger]

__kvm_inject_pfault_token() only sets .type and .u.ext.ext_params2 of
the on-stack struct kvm_s390_irq but the full ext substructure is copied
into the cpu local variable on inject. ext_params and pad contain stale
stack values.

Interrupt delivery only uses ext_params2, so nothing leaks, but a host
user can use the migration ioctls to get to the data.

Fix by zero-initializing the irq struct.

Fixes: 383d0b050106 ("KVM: s390: handle pending local interrupts via bitmap")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 4a38de3d6758..447e6a61f886 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -4474,7 +4474,7 @@ static void __kvm_inject_pfault_token(struct kvm_vcpu *vcpu, bool start_token,
 				     unsigned long token)
 {
 	struct kvm_s390_interrupt inti;
-	struct kvm_s390_irq irq;
+	struct kvm_s390_irq irq = {};
 	struct kvm_s390_interrupt_info *inti_mem = NULL;
 	int ret = 0;
 
-- 
2.53.0

Re: [PATCH 1/3] KVM: s390: remove user triggerable WARN_ON

[sashiko-bot]

> pin_map_page() fails legitimately whenever the userspace mapping behind
> the adapter route has gone away, e.g. when the VMM unmaps that memory.
> As this can happen without a kernel programming error, remove the
> WARN_ON.
> 
> Fixes: 8c0141b7c13c ("KVM: s390: Enable adapter_indicators_set to use mapped pages")
> Cc: Douglas Freimuth <freimuth@linux.ibm.com>
> Cc: Matthew Rosato <mjrosato@linux.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>

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

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

Re: [PATCH 3/3] KVM: s390: memory leak in guest debug handling

[sashiko-bot]

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

Pre-existing issues:
- [High] In `kvm_s390_import_bp_data()`, if importing a hardware watchpoint fails on a subsequent iteration, the dynamically allocated `old_data` buffers from watchpoints imported in previous iterations are permanently leaked.
- [High] The KVM_SET_GUEST_DEBUG ioctl calls `read_guest_abs()` (which internally accesses KVM memslots) without holding the `kvm->srcu` lock, violating KVM memslot locking rules and risking a Use-After-Free.
- [High] Leak of hardware watchpoint and breakpoint structures upon vCPU destruction because `kvm_s390_clear_bp_data()` is never called in the vCPU teardown path.
--

--- Patch [3]: [PATCH 3/3] KVM: s390: memory leak in guest debug handling ---
commit 56d3bf3acb3de4323dcc94259c523dacec14841e
Author: Christian Borntraeger <borntraeger@linux.ibm.com>

KVM: s390: memory leak in guest debug handling

bp_data is freed only for the error case by kfree(bp_data).
Every successful KVM_SET_GUEST_DEBUG will leak bp_data.

Fixes: 27291e2165b6 ("KVM: s390: hardware support for guest debugging")
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>

> diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
> index 69835e1d4f207..4c02dbebb2eb3 100644
> --- a/arch/s390/kvm/guestdbg.c
> +++ b/arch/s390/kvm/guestdbg.c
> @@ -267,6 +267,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>  	vcpu->arch.guestdbg.hw_bp_info = bp_info;
>  	vcpu->arch.guestdbg.nr_hw_wp = nr_wp;
>  	vcpu->arch.guestdbg.hw_wp_info = wp_info;
> +	kfree(bp_data);
>  	return 0;
>  error:
>  	kfree(bp_data);
>  	kfree(wp_info);

[Severity: High]
This is a pre-existing issue, but do we also leak the dynamically allocated
old_data buffers for watchpoints on this error path?

When iterating over the breakpoints, if a later hardware watchpoint fails
validation, the function jumps to this error path. It frees the wp_info
array, but does not appear to loop over the already processed wp_info
elements to free the wp_info->old_data buffers that were allocated in
__import_wp_info().

Could an unprivileged user trigger this error path repeatedly to exhaust
memory?

>  	kfree(bp_info);
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but does read_guest_abs() get called without
holding the kvm->srcu lock when importing watchpoints?

arch/s390/kvm/guestdbg.c:__import_wp_info() {
    ...
    wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL_ACCOUNT);
    ...
    ret = read_guest_abs(vcpu, wp_info->phys_addr, wp_info->old_data,
                         wp_info->len);
    ...
}

The KVM_SET_GUEST_DEBUG ioctl takes vcpu->mutex but not kvm->srcu. The call
chain __import_wp_info() -> read_guest_abs() -> kvm_read_guest() ->
gfn_to_memslot() expects kvm->srcu to be held to prevent use-after-free if
a memslot is concurrently removed. Should the SRCU lock be acquired before
reading guest memory here?

[Severity: High]
This is a pre-existing issue, but do we leak the guest debug structures if a
vCPU is destroyed while debugging is still enabled?

arch/s390/kvm/guestdbg.c:kvm_s390_import_bp_data() {
    ...
    vcpu->arch.guestdbg.hw_bp_info = bp_info;
    vcpu->arch.guestdbg.nr_hw_wp = nr_wp;
    vcpu->arch.guestdbg.hw_wp_info = wp_info;
    ...
}

If the userspace VMM process exits or crashes, kvm_arch_vcpu_destroy() is
called. However, kvm_arch_vcpu_destroy() in arch/s390/kvm/kvm-s390.c does not
appear to call kvm_s390_clear_bp_data() to clean up these dynamically
allocated arrays.

Would it be possible for an attacker to exhaust host memory by repeatedly
spawning and destroying debug-enabled VMs?

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

Re: [PATCH 2/3] KVM: s390: zero initialize irq for inject_pfault_token

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Calling kfree() on a stack-allocated pointer during floating machine check reinjection.
- [High] Uninitialized kernel stack memory in the `fixed_logout` array is leaked to host userspace via the KVM_GET_IRQ_STATE ioctl.
--

--- Patch [2]: [PATCH 2/3] KVM: s390: zero initialize irq for inject_pfault_token ---
commit f6de66eb1335bc1e8dc54d73ed3ad8d7b39ac810
Author: Christian Borntraeger <borntraeger@linux.ibm.com>

KVM: s390: zero initialize irq for inject_pfault_token

This commit fixes a potential stack information leak to userspace via
migration ioctls. It zero-initializes the irq structure in
__kvm_inject_pfault_token() to ensure stale stack values are not copied.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 518a69c55e85c..4f4ac6a8d1ad3 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4475,7 +4475,7 @@ static void __kvm_inject_pfault_token(struct kvm_vcpu *vcpu, bool start_token,
>  				     unsigned long token)
>  {
>  	struct kvm_s390_interrupt inti;
> -	struct kvm_s390_irq irq;
> +	struct kvm_s390_irq irq = {};

[Severity: High]
This isn't a bug introduced by this patch, but while looking at stack memory 
leaks, does a similar uninitialized stack leak happen in 
kvm_s390_reinject_machine_check() in arch/s390/kvm/interrupt.c?

arch/s390/kvm/interrupt.c:kvm_s390_reinject_machine_check() {
    ...
    struct kvm_s390_irq irq;
    ...
}

If the machine check is injected into the vCPU (mci.ck is false), several
fields are populated, but the fixed_logout array might be left
uninitialized. 

When passed to kvm_s390_inject_vcpu() -> __inject_mchk(), the uninitialized
fixed_logout array is copied into the vCPU's pending interrupt state:

arch/s390/kvm/interrupt.c:__inject_mchk() {
    ...
    memcpy(&mchk->fixed_logout, &irq->u.mchk.fixed_logout,
           sizeof(mchk->fixed_logout));
    ...
}

Later, if a host user calls the KVM_S390_GET_IRQ_STATE ioctl, this state
could be copied to userspace:

arch/s390/kvm/interrupt.c:kvm_s390_get_irq_state() {
    ...
    if (copy_to_user(&buf[n], &irq, sizeof(irq)))
    ...
}

Could this leak kernel stack memory in the fixed_logout array to userspace?


[Severity: Critical]
This is another pre-existing issue, but while looking at 
kvm_s390_reinject_machine_check(), can a stack pointer be incorrectly freed?

It allocates struct kvm_s390_interrupt_info inti on the kernel stack:

arch/s390/kvm/interrupt.c:kvm_s390_reinject_machine_check() {
    ...
    struct kvm_s390_interrupt_info inti;
    ...
}

If mci.ck is true, it attempts to inject a floating machine check by passing
the stack-allocated struct by reference:

arch/s390/kvm/interrupt.c:kvm_s390_reinject_machine_check() {
    ...
    rc = __inject_vm(vcpu->kvm, &inti);
    ...
}

The injection routes KVM_S390_MCHK to __inject_float_mchk(), which
unconditionally calls kfree() on the passed pointer:

arch/s390/kvm/interrupt.c:__inject_float_mchk() {
    ...
    kfree(inti);
    ...
}

Would calling kfree() on this stack pointer cause a kernel panic?

>  	struct kvm_s390_interrupt_info *inti_mem = NULL;
>  	int ret = 0;

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

Re: [PATCH 1/3] KVM: s390: remove user triggerable WARN_ON

[Matthew Rosato]

On 8/4/26 8:06 AM, Christian Borntraeger wrote:
> pin_map_page() fails legitimately whenever the userspace mapping behind
> the adapter route has gone away, e.g. when the VMM unmaps that memory.
> As this can happen without a kernel programming error, remove the
> WARN_ON.
> 
> Fixes: 8c0141b7c13c ("KVM: s390: Enable adapter_indicators_set to use mapped pages")
> Cc: Douglas Freimuth <freimuth@linux.ibm.com>
> Cc: Matthew Rosato <mjrosato@linux.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>  arch/s390/kvm/interrupt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 8f24bcd1a6d3..23f0e0821474 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -3013,7 +3013,7 @@ static int adapter_indicators_set(struct kvm *kvm,
>  	if (!summary_info) {
>  		spin_unlock_irqrestore(&adapter->maps_lock, flags);
>  		summary_page = pin_map_page(kvm, adapter_int->summary_addr, 0);
> -		if (WARN_ON_ONCE(!summary_page))
> +		if (!summary_page)
>  			return -1;
>  		idx = srcu_read_lock(&kvm->srcu);
>  		map = page_address(summary_page);

Re: [PATCH 2/3] KVM: s390: zero initialize irq for inject_pfault_token

[Matthew Rosato]

On 8/4/26 8:06 AM, Christian Borntraeger wrote:
> __kvm_inject_pfault_token() only sets .type and .u.ext.ext_params2 of
> the on-stack struct kvm_s390_irq but the full ext substructure is copied
> into the cpu local variable on inject. ext_params and pad contain stale
> stack values.
> 
> Interrupt delivery only uses ext_params2, so nothing leaks, but a host
> user can use the migration ioctls to get to the data.
> 
> Fix by zero-initializing the irq struct.
> 
> Fixes: 383d0b050106 ("KVM: s390: handle pending local interrupts via bitmap")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>  arch/s390/kvm/kvm-s390.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 4a38de3d6758..447e6a61f886 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4474,7 +4474,7 @@ static void __kvm_inject_pfault_token(struct kvm_vcpu *vcpu, bool start_token,
>  				     unsigned long token)
>  {
>  	struct kvm_s390_interrupt inti;
> -	struct kvm_s390_irq irq;
> +	struct kvm_s390_irq irq = {};
>  	struct kvm_s390_interrupt_info *inti_mem = NULL;
>  	int ret = 0;
>  

Re: [PATCH 3/3] KVM: s390: memory leak in guest debug handling

[Matthew Rosato]

On 8/4/26 8:06 AM, Christian Borntraeger wrote:
> bp_data is freed only for the error case by kfree(bp_data).
> Every successful KVM_SET_GUEST_DEBUG will leak bp_data.
> 
> Fixes: 27291e2165b6 ("KVM: s390: hardware support for guest debugging")
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>

Maybe worth re-visiting for potential __free() usage as a follow on and
replace the goto logic.

But as a fix this LGTM.

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>


> ---
>  arch/s390/kvm/guestdbg.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
> index 69835e1d4f20..4c02dbebb2eb 100644
> --- a/arch/s390/kvm/guestdbg.c
> +++ b/arch/s390/kvm/guestdbg.c
> @@ -267,6 +267,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>  	vcpu->arch.guestdbg.hw_bp_info = bp_info;
>  	vcpu->arch.guestdbg.nr_hw_wp = nr_wp;
>  	vcpu->arch.guestdbg.hw_wp_info = wp_info;
> +	kfree(bp_data);
>  	return 0;
>  error:
>  	kfree(bp_data);