[PATCH v2 0/7] KVM: s390: more fixes

[PATCH v2 0/7] KVM: s390: more fixes

[Christian Borntraeger]

v1->v2: several sashiko reports added
- machine check stack exposure
- extend zeroing to pfault done interrupts
- old_data leak
- srcu protection missing
- guest debug allocations not freed on destroy
- uppercase patch title

The comment about double free in kvm_s390_reinject_machine_check from
sashiko is technically not wrong, but this needs further rework.  We do
not want to forward channel subsystem damage to our guests and we
actually do not expect to see those in the wild. 

Christian Borntraeger (7):
  KVM: s390: Remove user triggerable WARN_ON
  KVM: s390: Zero initialize data structures for inject_pfault_token
  KVM: s390: Zero initialize irq in reinject_machine_check
  KVM: s390: Fix memory leak in guest debug handling
  KVM: s390: Fix old_data leak in guest debug error path
  KVM: s390: Take srcu when importing watchpoint data
  KVM: s390: Free guest debug data on vcpu destroy

 arch/s390/kvm/guestdbg.c  |  7 ++++++-
 arch/s390/kvm/interrupt.c |  4 ++--
 arch/s390/kvm/kvm-s390.c  | 11 +++++++----
 3 files changed, 15 insertions(+), 7 deletions(-)

-- 
2.53.0

[PATCH v2 1/7] 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: 1e95e3bc6b05 ("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);
-- 
2.53.0

Re: [PATCH v2 1/7] 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: 1e95e3bc6b05 ("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/20260804162435.28617-1-borntraeger@linux.ibm.com?part=1

Re: [PATCH v2 1/7] KVM: s390: Remove user triggerable WARN_ON

[Douglas Freimuth]

On 8/4/26 12:24 PM, 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: 1e95e3bc6b05 ("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>

Reviewed-by: Douglas Freimuth <freimuth@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);

[PATCH v2 2/7] KVM: s390: Zero initialize data structures 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 to the guest,
but a host user can use the migration ioctls to get to the data.

Fix by zero-initializing the irq struct.
Do the same for the inti data structure.

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 4a38de3d6758..c47c3a930db7 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -4473,8 +4473,8 @@ int kvm_s390_try_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clo
 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_interrupt inti = {};
+	struct kvm_s390_irq irq = {};
 	struct kvm_s390_interrupt_info *inti_mem = NULL;
 	int ret = 0;
 
-- 
2.53.0

Re: [PATCH v2 2/7] KVM: s390: Zero initialize data structures 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 variable when reinjecting floating machine checks leads to a guest-triggerable kernel panic.
- [High] Uninitialized stack memory in `struct kvm_s390_irq` is leaked to the guest and userspace during machine check reinjection.
--

--- Patch [2]: [PATCH v2 2/7] KVM: s390: Zero initialize data structures for inject_pfault_token ---
commit 2b81aa54a540c9ab3f152dc0b18d8b86e716252d
Author: Christian Borntraeger <borntraeger@linux.ibm.com>

KVM: s390: Zero initialize data structures for inject_pfault_token

This commit fixes an issue where __kvm_inject_pfault_token() leaves fields
in the on-stack struct kvm_s390_irq uninitialized. These stale stack values
are copied into the cpu local variable, allowing a host user to access them
via migration ioctls. The fix zero-initializes the irq and inti structures.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 518a69c55e85c..611dcb5342101 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4474,8 +4474,8 @@ int kvm_s390_try_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clo
>  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_interrupt inti = {};
> +	struct kvm_s390_irq irq = {};

[Severity: Critical]
This is a pre-existing issue, but while looking at these stack variables,
I noticed a similar pattern in arch/s390/kvm/interrupt.c that might lead to a
kernel panic.

In kvm_s390_reinject_machine_check(), the interrupt structure is allocated on
the kernel stack:

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

If a malicious L1 guest crafts a nested vSIE control block with mcic set to
include the mci.ck bit, __inject_vm() delegates to __inject_float_mchk(),
which then unconditionally frees the stack pointer:

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

Could this allow an untrusted, unprivileged guest to trigger an immediate
kernel panic by corrupting the SLUB allocator?

[Severity: High]
This is a pre-existing issue, but there appears to be another uninitialized
stack leak in arch/s390/kvm/interrupt.c.

Also in kvm_s390_reinject_machine_check(), the irq structure is allocated on
the kernel stack without zero-initialization:

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

If triggered via vSIE without the mci.ck bit, the fixed_logout field remains
uninitialized. When __inject_mchk() is called, it blindly copies this memory:

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

This uninitialized host kernel stack data is then written directly to guest
memory via __write_machine_check(), or leaked via KVM_GET_IRQ_STATE:

arch/s390/kvm/interrupt.c:__write_machine_check() {
    ...
    rc |= write_guest_lc(vcpu, __LC_PSW_SAVE_AREA, &mchk->fixed_logout,
                         sizeof(mchk->fixed_logout));
    ...
}

Could this allow an untrusted guest to repeatedly dump 16 bytes of
uninitialized host kernel stack memory, potentially bypassing KASLR or leaking
sensitive information?

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

[PATCH v2 3/7] KVM: s390: Zero initialize irq in reinject_machine_check

[Christian Borntraeger]

kvm_s390_reinject_machine_check() fills cr14, mcic, ext_damage_code and
failing_storage_address of the on-stack struct kvm_s390_irq, but struct
kvm_s390_mchk_info also has a pad word and a 16 byte fixed_logout array.
struct mcck_volatile_info carries no logout data, so there is nothing to
copy there and both stay whatever was on the stack.

__inject_mchk() then memcpy()s fixed_logout into the vcpu local
interrupt state unconditionally. This will reach the guest during
deliver and userspace during migration.

Reflecting zeroes is the correct behaviour here, as KVM has no logout
data for a reinjected machine check.

This needs a host machine check while the cpu is in SIE so not trivial
to trigger.

Fixes: 4d62fcc0b692 ("KVM: s390: Inject machine check into the guest")
Cc: stable@vger.kernel.org
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 23f0e0821474..009d6a845d59 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -3109,7 +3109,7 @@ void kvm_s390_reinject_machine_check(struct kvm_vcpu *vcpu,
 				     struct mcck_volatile_info *mcck_info)
 {
 	struct kvm_s390_interrupt_info inti;
-	struct kvm_s390_irq irq;
+	struct kvm_s390_irq irq = {};
 	struct kvm_s390_mchk_info *mchk;
 	union mci mci;
 	__u64 cr14 = 0;         /* upper bits are not used */
-- 
2.53.0

Re: [PATCH v2 3/7] KVM: s390: Zero initialize irq in reinject_machine_check

[Matthew Rosato]

On 8/4/26 12:24 PM, Christian Borntraeger wrote:
> kvm_s390_reinject_machine_check() fills cr14, mcic, ext_damage_code and
> failing_storage_address of the on-stack struct kvm_s390_irq, but struct
> kvm_s390_mchk_info also has a pad word and a 16 byte fixed_logout array.
> struct mcck_volatile_info carries no logout data, so there is nothing to
> copy there and both stay whatever was on the stack.
> 
> __inject_mchk() then memcpy()s fixed_logout into the vcpu local
> interrupt state unconditionally. This will reach the guest during
> deliver and userspace during migration.
> 
> Reflecting zeroes is the correct behaviour here, as KVM has no logout
> data for a reinjected machine check.
> 
> This needs a host machine check while the cpu is in SIE so not trivial
> to trigger.
> 
> Fixes: 4d62fcc0b692 ("KVM: s390: Inject machine check into the guest")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>

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

Re: [PATCH v2 3/7] KVM: s390: Zero initialize irq in reinject_machine_check

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Injecting a floating machine check causes a host kernel panic because kfree() is unconditionally called on a stack-allocated variable.
- [High] Memory leak in kvm_s390_get_io_int() when I/O interrupt reinjection fails.
--

--- Patch [3]: [PATCH v2 3/7] KVM: s390: Zero initialize irq in reinject_machine_check ---
commit c87188fae3993586a5800e7440b273a7c1c85398
Author: Christian Borntraeger <borntraeger@linux.ibm.com>
Subject: KVM: s390: Zero initialize irq in reinject_machine_check

This commit zeroes out the stack-allocated kvm_s390_irq structure in
kvm_s390_reinject_machine_check() to prevent leaking uninitialized stack
data via the fixed_logout array.

> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 23f0e0821474a..009d6a845d598 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -3109,7 +3109,7 @@ void kvm_s390_reinject_machine_check(struct kvm_vcpu *vcpu,
>  				     struct mcck_volatile_info *mcck_info)
>  {
>  	struct kvm_s390_interrupt_info inti;

[Severity: Critical]
This is a pre-existing issue, but does this lead to a kernel panic when
injecting a floating machine check?

If a hardware machine check occurs while the CPU is in SIE and it is a
floating machine check, we pass the address of this stack-allocated inti
variable into __inject_vm():

kvm_s390_reinject_machine_check() {
    ...
    if (mci.ck) {
        inti.type = KVM_S390_MCHK;
        rc = __inject_vm(vcpu->kvm, &inti);
    ...
}

Inside __inject_vm(), the call chain reaches __inject_float_mchk(), which
unconditionally frees the pointer:

__inject_float_mchk() {
    ...
    kfree(inti);
}

Could this result in a host crash due to freeing a stack address?

> -	struct kvm_s390_irq irq;
> +	struct kvm_s390_irq irq = {};
>  	struct kvm_s390_mchk_info *mchk;
>  	union mci mci;
>  	__u64 cr14 = 0;         /* upper bits are not used */


While reviewing the interrupt code, I noticed another pre-existing issue in
a different path.

[Severity: High]
This isn't a bug introduced by this patch, but does kvm_s390_get_io_int()
leak memory when I/O interrupt reinjection fails?

When kvm_s390_get_io_int() dequeues a classical I/O interrupt, it takes
ownership of a dynamically allocated struct kvm_s390_interrupt_info object.
If it detects a higher-priority GISA interrupt concurrently, it attempts to
reinject it:

kvm_s390_get_io_int() {
    ...
    kvm_s390_reinject_io_int(kvm, inti);
    ...
}

If the floating interrupt queue is full, __inject_io() returns -EBUSY:

__inject_io() {
    ...
    if (fi->counters[FIRQ_CNTR_IO] >= KVM_S390_MAX_FLOAT_IRQS)
        return -EBUSY;
    ...
}

The return value of the reinjection is ignored in kvm_s390_get_io_int(), and
the original pointer is overwritten immediately after without freeing it on
error, which appears to leak the object.

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

[PATCH v2 4/7] KVM: s390: Fix memory leak in guest debug handling

[Christian Borntraeger]

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")
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/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);
-- 
2.53.0

Re: [PATCH v2 4/7] KVM: s390: Fix 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:
- [Critical] Missing SRCU read lock protection in KVM guest debug memory access leading to Use-After-Free risk.
- [High] Hardware breakpoint and watchpoint data structures are permanently leaked when a VCPU is destroyed.
- [High] Integer truncation of `bp_data->len` bypasses bounds checking and allows maliciously large memory allocations.
--

--- Patch [4]: [PATCH v2 4/7] KVM: s390: Fix memory leak in guest debug handling ---
commit ed2dab291478e4d3632d170d668feb5781798aba
Author: Christian Borntraeger <borntraeger@linux.ibm.com>

KVM: s390: Fix memory leak in guest debug handling

This commit fixes a memory leak in the s390 guest debug handling. It ensures
that the bp_data array allocated during KVM_SET_GUEST_DEBUG is properly freed
on successful paths, rather than only in the error case.

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

[Severity: Critical]
This is a pre-existing issue, but since this ioctl path is being modified,
is there a missing SRCU read lock when accessing guest memory?

When a user issues KVM_SET_GUEST_DEBUG, the ioctl executes the following path:
kvm_arch_vcpu_ioctl_set_guest_debug() -> kvm_s390_import_bp_data() ->
__import_wp_info() -> read_guest_abs()

In arch/s390/kvm/guestdbg.c:__import_wp_info():
	ret = read_guest_abs(vcpu, wp_info->phys_addr, wp_info->old_data,
			     wp_info->len);

The ioctl path only holds vcpu->mutex, but read_guest_abs() translates the
guest physical address using gfn_to_memslot(). Without holding kvm->srcu, could
concurrent memslot updates (like KVM_SET_USER_MEMORY_REGION) free the memslot
while it is being accessed, leading to a use-after-free?


[Severity: High]
This isn't a bug introduced by this patch, but does the guest debug length
check properly prevent large memory allocations?

In arch/s390/kvm/guestdbg.c:__import_wp_info():
	wp_info->len = bp_data->len;
	wp_info->addr = bp_data->addr;
	wp_info->phys_addr = bp_data->phys_addr;
	wp_info->old_data = NULL;

	if (wp_info->len < 0 || wp_info->len > MAX_WP_SIZE)
		return -EINVAL;

	wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL_ACCOUNT);

The user-controlled bp_data->len is a 64-bit value, but wp_info->len is a
32-bit integer. If a malicious user passes a carefully crafted 64-bit length
such as 4294967301, could it truncate to 5 when assigned to wp_info->len,
bypass the MAX_WP_SIZE check, and then trigger an oversized allocation when
the original 64-bit bp_data->len is passed to kmalloc()?


[Severity: High]
This is a pre-existing issue, but does VCPU destruction completely clean up
the guest debug allocations?

When userspace creates a VCPU and sets hardware breakpoints via
KVM_SET_GUEST_DEBUG, arrays are dynamically allocated and attached to
vcpu->arch.guestdbg (hw_bp_info, hw_wp_info, and old_data).

In arch/s390/kvm/kvm-s390.c:kvm_arch_vcpu_destroy():
	free_page((unsigned long)(vcpu->arch.sie_block));
	kvm_s390_free_mmu_cache(vcpu->arch.mc);

It appears kvm_arch_vcpu_destroy() is missing a call to
kvm_s390_clear_bp_data(). Could this allow unprivileged users to leak kernel
memory by repeatedly creating and destroying VCPUs?

>  	return 0;
>  error:
>  	kfree(bp_data);

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

[PATCH v2 5/7] KVM: s390: Fix old_data leak in guest debug error path

[Christian Borntraeger]

__import_wp_info() allocates a per-watchpoint old_data buffer to back up
the original guest memory contents. If a later watchpoint of the same
KVM_SET_GUEST_DEBUG request fails to import, kvm_s390_import_bp_data()
jumps to the error label, which frees the wp_info array but not the
old_data buffers of the entries that were imported successfully. Up to
MAX_BP_COUNT - 1 buffers of up to MAX_WP_SIZE bytes are leaked per failed
request, and the request can be repeated.

Create error handling for cleaning up all created old_data memory
areas.

Fixes: 27291e2165b6 ("KVM: s390: hardware support for guest debugging")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/guestdbg.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
index 4c02dbebb2eb..f7c94d54efbe 100644
--- a/arch/s390/kvm/guestdbg.c
+++ b/arch/s390/kvm/guestdbg.c
@@ -252,7 +252,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
 			ret = __import_wp_info(vcpu, &bp_data[i],
 					       &wp_info[nr_wp]);
 			if (ret)
-				goto error;
+				goto error_wp;
 			nr_wp++;
 			break;
 		case KVM_HW_BP:
@@ -269,6 +269,10 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
 	vcpu->arch.guestdbg.hw_wp_info = wp_info;
 	kfree(bp_data);
 	return 0;
+
+error_wp:
+	while (nr_wp--)
+		kfree(wp_info[nr_wp].old_data);
 error:
 	kfree(bp_data);
 	kfree(wp_info);
-- 
2.53.0

Re: [PATCH v2 5/7] KVM: s390: Fix old_data leak in guest debug error path

[Matthew Rosato]

On 8/4/26 12:24 PM, Christian Borntraeger wrote:
> __import_wp_info() allocates a per-watchpoint old_data buffer to back up
> the original guest memory contents. If a later watchpoint of the same
> KVM_SET_GUEST_DEBUG request fails to import, kvm_s390_import_bp_data()
> jumps to the error label, which frees the wp_info array but not the
> old_data buffers of the entries that were imported successfully. Up to
> MAX_BP_COUNT - 1 buffers of up to MAX_WP_SIZE bytes are leaked per failed
> request, and the request can be repeated.
> 
> Create error handling for cleaning up all created old_data memory
> areas.
> 
> Fixes: 27291e2165b6 ("KVM: s390: hardware support for guest debugging")
> 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/guestdbg.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
> index 4c02dbebb2eb..f7c94d54efbe 100644
> --- a/arch/s390/kvm/guestdbg.c
> +++ b/arch/s390/kvm/guestdbg.c
> @@ -252,7 +252,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>  			ret = __import_wp_info(vcpu, &bp_data[i],
>  					       &wp_info[nr_wp]);
>  			if (ret)
> -				goto error;
> +				goto error_wp;
>  			nr_wp++;
>  			break;
>  		case KVM_HW_BP:
> @@ -269,6 +269,10 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>  	vcpu->arch.guestdbg.hw_wp_info = wp_info;
>  	kfree(bp_data);
>  	return 0;
> +
> +error_wp:
> +	while (nr_wp--)
> +		kfree(wp_info[nr_wp].old_data);
>  error:
>  	kfree(bp_data);
>  	kfree(wp_info);

Re: [PATCH v2 5/7] KVM: s390: Fix old_data leak in guest debug error path

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Missing SRCU read lock around KVM memslot access during KVM_SET_GUEST_DEBUG causes a Use-After-Free vulnerability.
- [High] Pre-existing memory leak of guest debug arrays (`hw_wp_info`, `hw_bp_info`) and per-watchpoint `old_data` buffers on vCPU destruction.
- [High] Integer truncation of `bp_data->len` allows unconstrained memory allocation, leading to a kernel WARNING and potential Denial of Service.
--

--- Patch [5]: [PATCH v2 5/7] KVM: s390: Fix old_data leak in guest debug error path ---
commit a3583776c5b24b471704343b4d20b599ed4ab8ca
Author: Christian Borntraeger <borntraeger@linux.ibm.com>

KVM: s390: Fix old_data leak in guest debug error path

__import_wp_info() allocates a per-watchpoint old_data buffer to back up
the original guest memory contents. If a later watchpoint of the same
KVM_SET_GUEST_DEBUG request fails to import, kvm_s390_import_bp_data()
jumps to the error label, which frees the wp_info array but not the
old_data buffers of the entries that were imported successfully.

Cc: stable@vger.kernel.org

> diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c
> index 4c02dbebb2eb3..f7c94d54efbe2 100644
> --- a/arch/s390/kvm/guestdbg.c
> +++ b/arch/s390/kvm/guestdbg.c
> @@ -252,7 +252,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>  			ret = __import_wp_info(vcpu, &bp_data[i],
>  					       &wp_info[nr_wp]);

[Severity: Critical]
This is a pre-existing issue, but does this call path lack SRCU protection?

When KVM_SET_GUEST_DEBUG calls __import_wp_info(), it eventually invokes
read_guest_abs() which dereferences the KVM memslots array via
gfn_to_memslot().

Neither kvm_vcpu_ioctl() nor kvm_arch_vcpu_ioctl_set_guest_debug() appears
to acquire the kvm->srcu read lock. Can a concurrent
KVM_SET_USER_MEMORY_REGION ioctl free the memslot structure while it is
being accessed, triggering a use-after-free?


[Severity: High]
This is a pre-existing issue, but is there an integer truncation bug inside
__import_wp_info() for the length parameter?

bp_data->len is a 64-bit user-controlled value, which is assigned to
wp_info->len (a 32-bit signed int):

arch/s390/kvm/guestdbg.c:__import_wp_info() {
    ...
    wp_info->len = bp_data->len;
    ...
    if (wp_info->len < 0 || wp_info->len > MAX_WP_SIZE)
        return -EINVAL;

    wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL_ACCOUNT);
    ...
}

By providing a large 64-bit value (e.g., 0x100000005), the lower 32 bits
would pass the bounds check (5 <= MAX_WP_SIZE), but the untruncated 64-bit
value is then passed to kmalloc().

Can this lead to an unbounded memory allocation exceeding MAX_PAGE_ORDER,
triggering a kernel warning?

>  			if (ret)
> -				goto error;
> +				goto error_wp;
>  			nr_wp++;
>  			break;

[ ... ]

> @@ -269,6 +269,10 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu,
>  	vcpu->arch.guestdbg.hw_wp_info = wp_info;
>  	kfree(bp_data);
>  	return 0;
> +
> +error_wp:
> +	while (nr_wp--)
> +		kfree(wp_info[nr_wp].old_data);
>  error:
>  	kfree(bp_data);

[Severity: High]
This is a pre-existing issue, but does kvm_arch_vcpu_destroy() leak these
dynamically allocated buffers when tearing down a vCPU?

If the VM is destroyed while guest debugging is active,
kvm_arch_vcpu_destroy() seems to skip calling kvm_s390_clear_bp_data()
to free hw_wp_info, hw_bp_info, and the old_data buffers:

arch/s390/kvm/kvm-s390.c:kvm_arch_vcpu_destroy() {
    ...
    VCPU_EVENT(vcpu, 3, "%s", "free cpu");
    ...
}

Could a host userspace process repeatedly create and destroy vCPUs with
guest debug enabled to exhaust host memory?

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

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

[PATCH v2 6/7] KVM: s390: Take srcu when importing watchpoint data

[Christian Borntraeger]

__import_wp_info() backs up the original guest memory contents of a
watchpoint with read_guest_abs(), which is kvm_read_guest() and therefore
resolves the memslot via __kvm_memslots(). That requires kvm->srcu (or
kvm->slots_lock) to be held, otherwise a concurrent memslot update can
free the memslots array under us once its SRCU grace period has elapsed.

As this is not fast path, following lock ordering (mutex first, then
srcu) take the big hammer and hold the srcu for the full import.

Fixes: 27291e2165b6 ("KVM: s390: hardware support for guest debugging")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index c47c3a930db7..e0548926dc49 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -4247,8 +4247,10 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 		/* enforce guest PER */
 		kvm_s390_set_cpuflags(vcpu, CPUSTAT_P);
 
-		if (dbg->control & KVM_GUESTDBG_USE_HW_BP)
-			rc = kvm_s390_import_bp_data(vcpu, dbg);
+		if (dbg->control & KVM_GUESTDBG_USE_HW_BP) {
+			scoped_guard(srcu, &vcpu->kvm->srcu)
+				rc = kvm_s390_import_bp_data(vcpu, dbg);
+		}
 	} else {
 		kvm_s390_clear_cpuflags(vcpu, CPUSTAT_P);
 		vcpu->arch.guestdbg.last_bp = 0;
-- 
2.53.0

Re: [PATCH v2 6/7] KVM: s390: Take srcu when importing watchpoint data

[sashiko-bot]

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

New issues:
- [High] Holding kvm->srcu across a copy_from_user() operation introduces a userfaultfd deadlock vulnerability.
- [Low] Mixing goto-based error handling and scope-based cleanup helpers in the same function.

Pre-existing issues:
- [High] Hardware breakpoint and watchpoint data allocated via KVM_SET_GUEST_DEBUG is leaked upon VCPU destruction.
- [High] Integer truncation bypasses length validation, allowing unbounded kmalloc requests and kernel log spam.
--

--- Patch [6]: [PATCH v2 6/7] KVM: s390: Take srcu when importing watchpoint data ---
commit f52f981282afa4c844e3d3336d83110ab98f5741
Author: Christian Borntraeger <borntraeger@linux.ibm.com>

KVM: s390: Take srcu when importing watchpoint data

This commit wraps the call to kvm_s390_import_bp_data() in an SRCU read-side
critical section using scoped_guard(). This is done to ensure that the
memslots array is not freed concurrently during the watchpoint import
process.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 611dcb5342101..0a197e055cad0 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c

[ ... ]

> @@ -4248,8 +4248,10 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>  		/* enforce guest PER */
>  		kvm_s390_set_cpuflags(vcpu, CPUSTAT_P);
>  
> -		if (dbg->control & KVM_GUESTDBG_USE_HW_BP)
> -			rc = kvm_s390_import_bp_data(vcpu, dbg);
> +		if (dbg->control & KVM_GUESTDBG_USE_HW_BP) {
> +			scoped_guard(srcu, &vcpu->kvm->srcu)

[Severity: Low]
This isn't a bug, but does mixing scoped_guard() with the existing goto-based
error handling in this function violate the cleanup guidelines?

According to include/linux/cleanup.h, mixing goto-based error handling with
scope-based cleanup helpers in the same function can create confusing
ownership semantics. The guidelines suggest converting all resources that
need a goto cleanup to scope-based cleanup, or converting none of them. In
this function, vcpu_put(vcpu) is still handled via a goto out; label.

> +				rc = kvm_s390_import_bp_data(vcpu, dbg);

[Severity: High]
Could holding the SRCU read lock across this call introduce a userfaultfd
deadlock?

Inside kvm_s390_import_bp_data(), the very first operation is:

arch/s390/kvm/guestdbg.c:kvm_s390_import_bp_data() {
    ...
    bp_data = memdup_array_user(dbg->arch.hw_bp, dbg->arch.nr_hw_bp,
                                sizeof(*bp_data));
    ...
}

If user space maps this array using userfaultfd, the VCPU thread will block
in copy_from_user() while holding the kvm->srcu read lock. If the user space
userfaultfd handler attempts to execute a VM-wide KVM ioctl that modifies
memslots (such as KVM_SET_USER_MEMORY_REGION), that ioctl will call
synchronize_srcu(&kvm->srcu). The VCPU thread would wait for the page fault
to be resolved, and the userfaultfd handler would wait for the SRCU lock to
be released.

[Severity: High]
This isn't a bug introduced by this patch, but does kvm_s390_import_bp_data()
leak the dynamically allocated debug information upon VCPU destruction?

kvm_s390_import_bp_data() allocates memory for wp_info, bp_info, and old_data,
and assigns them to vcpu->arch.guestdbg. While these are freed during a
subsequent KVM_SET_GUEST_DEBUG ioctl via kvm_s390_clear_bp_data(vcpu), it
appears kvm_s390_clear_bp_data() is never invoked during VCPU destruction:

arch/s390/kvm/kvm-s390.c:kvm_arch_vcpu_destroy() {
    ...
    // No call to kvm_s390_clear_bp_data()
    ...
}

Could an unprivileged user who can create VCPUs repeatedly set hardware
breakpoints and then close the VCPU file descriptor to leak this memory?

[Severity: High]
This is a pre-existing issue, but can an integer truncation in
__import_wp_info() bypass length validation and cause unbounded kmalloc
requests?

When kvm_s390_import_bp_data() processes watchpoints, it calls
__import_wp_info(), where the user-provided __u64 length is truncated to an
int for the bounds check:

arch/s390/kvm/guestdbg.c:__import_wp_info() {
    ...
    wp_info->len = bp_data->len;
    ...
    if (wp_info->len < 0 || wp_info->len > MAX_WP_SIZE)
        return -EINVAL;
    
    wp_info->old_data = kmalloc(bp_data->len, GFP_KERNEL_ACCOUNT);
    ...
}

If a malicious user provides a length of 4GB + 1, the truncated integer
becomes 1, bypassing the bounds check. The code then calls kmalloc() with
the original __u64 size. Since it lacks __GFP_NOWARN, won't this trigger a
memory allocation warning dump in the kernel log?

> +		}
>  	} else {
>  		kvm_s390_clear_cpuflags(vcpu, CPUSTAT_P);
>  		vcpu->arch.guestdbg.last_bp = 0;

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

Re: [PATCH v2 6/7] KVM: s390: Take srcu when importing watchpoint data

[Christian Borntraeger]

Am 04.08.26 um 20:40 schrieb sashiko-bot@kernel.org:
> 
> [Severity: High]
> Could holding the SRCU read lock across this call introduce a userfaultfd
> deadlock?
> 
> Inside kvm_s390_import_bp_data(), the very first operation is:
> 
> arch/s390/kvm/guestdbg.c:kvm_s390_import_bp_data() {
>      ...
>      bp_data = memdup_array_user(dbg->arch.hw_bp, dbg->arch.nr_hw_bp,
>                                  sizeof(*bp_data));
>      ...
> }
> 
> If user space maps this array using userfaultfd, the VCPU thread will block
> in copy_from_user() while holding the kvm->srcu read lock. If the user space
> userfaultfd handler attempts to execute a VM-wide KVM ioctl that modifies
> memslots (such as KVM_SET_USER_MEMORY_REGION), that ioctl will call
> synchronize_srcu(&kvm->srcu). The VCPU thread would wait for the page fault
> to be resolved, and the userfaultfd handler would wait for the SRCU lock to
> be released.


Now I am puzzled. Sashiko basically asked for srcu protection for the guest access
in the same review series. But I want to followup nevertheless.


There are other places in KVM code which do access guest memory under the srcu.

1.
KVM_SET_NESTED_STATE — arch/x86/kvm/x86.c
               idx = srcu_read_lock(&vcpu->kvm->srcu);
               r = kvm_x86_ops.nested_ops->set_state(vcpu, user_kvm_nested_state, &kvm_state);
               srcu_read_unlock(&vcpu->kvm->srcu, idx);
with  vmx_set_nested_state() then copying  the vmcs12 from user


2.
KVM_GET_MSRS / KVM_SET_MSRS — arch/x86/kvm/x86.c

       case KVM_SET_MSRS: {
               int idx = srcu_read_lock(&vcpu->kvm->srcu);
               r = msr_io(vcpu, argp, do_set_msr, 0);
               srcu_read_unlock(&vcpu->kvm->srcu, idx);
               break;
       }
argp goes in as a raw void __user *, and msr_io() does both user copies inside that section:

       if (copy_from_user(&msrs, user_msrs, sizeof(msrs)))


3.
KVM_S390_MEM_OP
kvm_s390_vm_mem_op_abs() has
         scoped_guard(srcu, &kvm->srcu) {
[...]
                 if (acc_mode == GACC_STORE && copy_from_user(tmpbuf, uaddr, mop->size))
                         return -EFAULT;


4.
KVM_PRE_FAULT_MEMORY generic KVM holds srcu across a loop of kvm_arch_vcpu_pre_fault_memory() calls that fault in guest memory,

5.
and last but not least,isnt vcpu_run also holding kvm->srcu when handling page faults?

In the end the cycle seems userspace constructed. It deadlocks only if its own uffd handler
is made to depend on a vcpu blocked on that same handler.The faulting side waits in
TASK_KILLABLE (mm/userfaultfd.c:2673) — SIGKILL to the process releases the srcu section
and lets the other thread finish.  So we can get out of it.


So I think this finding is wrong. Paolo?

[PATCH v2 7/7] KVM: s390: Free guest debug data on vcpu destroy

[Christian Borntraeger]

kvm_s390_clear_bp_data() is only called from
kvm_arch_vcpu_ioctl_set_guest_debug(), i.e. when user space changes or
disables debugging. A vCPU that is destroyed while hardware breakpoints
are still armed - the normal case when the VMM just exits or crashes -
leaks hw_bp_info, hw_wp_info and all old_data buffers, since generic KVM
frees the vCPU right after kvm_arch_vcpu_destroy().

That is bounded by MAX_BP_COUNT entries, so roughly 8 KiB per vCPU, but
it is unbounded over VM lifetimes. The allocations are
GFP_KERNEL_ACCOUNT, so the charge also outlives the exiting process and
pins dying memcgs.

Fix by clearing the debug data on vCPU destruction. Calling it
unconditionally is fine: struct kvm_vcpu is zero allocated, so for a vCPU
that never enabled debugging the counters are 0 and the pointers NULL.

Fixes: 27291e2165b6 ("KVM: s390: hardware support for guest debugging")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index e0548926dc49..ccbdb109e22c 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3409,6 +3409,7 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 	trace_kvm_s390_destroy_vcpu(vcpu->vcpu_id);
 	kvm_s390_clear_local_irqs(vcpu);
 	kvm_clear_async_pf_completion_queue(vcpu);
+	kvm_s390_clear_bp_data(vcpu);
 	if (!kvm_is_ucontrol(vcpu->kvm))
 		sca_del_vcpu(vcpu);
 	kvm_s390_update_topology_change_report(vcpu->kvm, 1);
-- 
2.53.0

Re: [PATCH v2 7/7] KVM: s390: Free guest debug data on vcpu destroy

[sashiko-bot]

> kvm_s390_clear_bp_data() is only called from
> kvm_arch_vcpu_ioctl_set_guest_debug(), i.e. when user space changes or
> disables debugging. A vCPU that is destroyed while hardware breakpoints
> are still armed - the normal case when the VMM just exits or crashes -
> leaks hw_bp_info, hw_wp_info and all old_data buffers, since generic KVM
> frees the vCPU right after kvm_arch_vcpu_destroy().
> 
> That is bounded by MAX_BP_COUNT entries, so roughly 8 KiB per vCPU, but
> it is unbounded over VM lifetimes. The allocations are
> GFP_KERNEL_ACCOUNT, so the charge also outlives the exiting process and
> pins dying memcgs.
> 
> Fix by clearing the debug data on vCPU destruction. Calling it
> unconditionally is fine: struct kvm_vcpu is zero allocated, so for a vCPU
> that never enabled debugging the counters are 0 and the pointers NULL.
> 
> Fixes: 27291e2165b6 ("KVM: s390: hardware support for guest debugging")
> Cc: stable@vger.kernel.org
> 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/20260804162435.28617-1-borntraeger@linux.ibm.com?part=7

Re: [PATCH v2 7/7] KVM: s390: Free guest debug data on vcpu destroy

[Matthew Rosato]

On 8/4/26 12:24 PM, Christian Borntraeger wrote:
> kvm_s390_clear_bp_data() is only called from
> kvm_arch_vcpu_ioctl_set_guest_debug(), i.e. when user space changes or
> disables debugging. A vCPU that is destroyed while hardware breakpoints
> are still armed - the normal case when the VMM just exits or crashes -
> leaks hw_bp_info, hw_wp_info and all old_data buffers, since generic KVM
> frees the vCPU right after kvm_arch_vcpu_destroy().
> 
> That is bounded by MAX_BP_COUNT entries, so roughly 8 KiB per vCPU, but
> it is unbounded over VM lifetimes. The allocations are
> GFP_KERNEL_ACCOUNT, so the charge also outlives the exiting process and
> pins dying memcgs.
> 
> Fix by clearing the debug data on vCPU destruction. Calling it
> unconditionally is fine: struct kvm_vcpu is zero allocated, so for a vCPU
> that never enabled debugging the counters are 0 and the pointers NULL.
> 
> Fixes: 27291e2165b6 ("KVM: s390: hardware support for guest debugging")
> 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 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index e0548926dc49..ccbdb109e22c 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -3409,6 +3409,7 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>  	trace_kvm_s390_destroy_vcpu(vcpu->vcpu_id);
>  	kvm_s390_clear_local_irqs(vcpu);
>  	kvm_clear_async_pf_completion_queue(vcpu);
> +	kvm_s390_clear_bp_data(vcpu);
>  	if (!kvm_is_ucontrol(vcpu->kvm))
>  		sca_del_vcpu(vcpu);
>  	kvm_s390_update_topology_change_report(vcpu->kvm, 1);