* [GIT PULL v1 01/11] KVM: s390: Fix dirty marking in adapter_indicators_set*()
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:15 ` sashiko-bot
2026-09-04 6:06 ` [GIT PULL v1 02/11] KVM: s390: Fix compile warning for kvm_s390_update_cmma_dirty() Claudio Imbrenda
` (10 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
When the indicator and/or summary bits are set in the guest, the
accessed page was only marked dirty in KVM if the access was performed
using the slow path; accesses through the new kvm_arch_set_irq_inatomic
fast inject path would not mark the page as dirty.
Fix by adding/moving the missing calls to mark_page_dirty(). Note that
for the inatomic path set_page_dirty{,_lock}() is not needed as the
page stays pinned; the unpin path correctly marks it as dirty.
Opportunistically reorder the local variables to be in reverse
Christmas tree order and refactor to use guard().
Fixes: 1e95e3bc6b05 ("KVM: s390: Enable adapter_indicators_set to use mapped pages")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260828115439.145885-2-imbrenda@linux.ibm.com>
---
arch/s390/kvm/s390/interrupt.c | 75 +++++++++++++++++-----------------
1 file changed, 38 insertions(+), 37 deletions(-)
diff --git a/arch/s390/kvm/s390/interrupt.c b/arch/s390/kvm/s390/interrupt.c
index 0381ae981703..49b4e233e791 100644
--- a/arch/s390/kvm/s390/interrupt.c
+++ b/arch/s390/kvm/s390/interrupt.c
@@ -2984,61 +2984,58 @@ static int adapter_indicators_set(struct kvm *kvm,
struct s390_io_adapter *adapter,
struct kvm_s390_adapter_int *adapter_int)
{
- unsigned long bit;
- int summary_set, idx;
struct s390_map_info *ind_info, *summary_info;
- void *map;
struct page *ind_page, *summary_page;
- unsigned long flags;
+ unsigned long bit;
+ int summary_set;
+ void *map;
ind_page = NULL;
- spin_lock_irqsave(&adapter->maps_lock, flags);
- ind_info = get_map_info(adapter, adapter_int->ind_addr);
+ scoped_guard(spinlock_irqsave, &adapter->maps_lock) {
+ ind_info = get_map_info(adapter, adapter_int->ind_addr);
+ if (ind_info) {
+ map = page_address(ind_info->page);
+ bit = get_ind_bit(ind_info->addr, adapter_int->ind_offset, adapter->swap);
+ set_bit(bit, map);
+ }
+ }
if (!ind_info) {
- spin_unlock_irqrestore(&adapter->maps_lock, flags);
ind_page = pin_map_page(kvm, adapter_int->ind_addr, 0);
if (!ind_page)
return -1;
- idx = srcu_read_lock(&kvm->srcu);
map = page_address(ind_page);
bit = get_ind_bit(adapter_int->ind_addr,
adapter_int->ind_offset, adapter->swap);
set_bit(bit, map);
- mark_page_dirty(kvm, adapter_int->ind_gaddr >> PAGE_SHIFT);
set_page_dirty_lock(ind_page);
- srcu_read_unlock(&kvm->srcu, idx);
unpin_user_page(ind_page);
- } else {
- map = page_address(ind_info->page);
- bit = get_ind_bit(ind_info->addr, adapter_int->ind_offset, adapter->swap);
- set_bit(bit, map);
- spin_unlock_irqrestore(&adapter->maps_lock, flags);
}
+ scoped_guard(srcu, &kvm->srcu)
+ mark_page_dirty(kvm, gpa_to_gfn(adapter_int->ind_gaddr));
- spin_lock_irqsave(&adapter->maps_lock, flags);
- summary_info = get_map_info(adapter, adapter_int->summary_addr);
+ scoped_guard(spinlock_irqsave, &adapter->maps_lock) {
+ summary_info = get_map_info(adapter, adapter_int->summary_addr);
+ if (summary_info) {
+ map = page_address(summary_info->page);
+ bit = get_ind_bit(summary_info->addr, adapter_int->summary_offset,
+ adapter->swap);
+ summary_set = test_and_set_bit(bit, map);
+ }
+ }
if (!summary_info) {
- spin_unlock_irqrestore(&adapter->maps_lock, flags);
summary_page = pin_map_page(kvm, adapter_int->summary_addr, 0);
if (!summary_page)
return -1;
- idx = srcu_read_lock(&kvm->srcu);
map = page_address(summary_page);
bit = get_ind_bit(adapter_int->summary_addr,
adapter_int->summary_offset, adapter->swap);
summary_set = test_and_set_bit(bit, map);
- mark_page_dirty(kvm, adapter_int->summary_gaddr >> PAGE_SHIFT);
set_page_dirty_lock(summary_page);
- srcu_read_unlock(&kvm->srcu, idx);
unpin_user_page(summary_page);
- } else {
- map = page_address(summary_info->page);
- bit = get_ind_bit(summary_info->addr, adapter_int->summary_offset,
- adapter->swap);
- summary_set = test_and_set_bit(bit, map);
- spin_unlock_irqrestore(&adapter->maps_lock, flags);
}
+ scoped_guard(srcu, &kvm->srcu)
+ mark_page_dirty(kvm, gpa_to_gfn(adapter_int->summary_gaddr));
return summary_set ? 0 : 1;
}
@@ -3048,26 +3045,29 @@ static int adapter_indicators_set_fast(struct kvm *kvm,
struct kvm_s390_adapter_int *adapter_int,
int setbit)
{
+ struct s390_map_info *ind_info, *summary_info;
unsigned long bit;
int summary_set;
- struct s390_map_info *ind_info, *summary_info;
void *map;
- spin_lock(&adapter->maps_lock);
+ guard(srcu)(&kvm->srcu);
+ guard(spinlock)(&adapter->maps_lock);
+
ind_info = get_map_info(adapter, adapter_int->ind_addr);
- if (!ind_info) {
- spin_unlock(&adapter->maps_lock);
+ if (!ind_info)
return -EWOULDBLOCK;
- }
+
map = page_address(ind_info->page);
bit = get_ind_bit(ind_info->addr, adapter_int->ind_offset, adapter->swap);
- if (setbit)
+ if (setbit) {
set_bit(bit, map);
+ mark_page_dirty(kvm, gpa_to_gfn(adapter_int->ind_gaddr));
+ }
+
summary_info = get_map_info(adapter, adapter_int->summary_addr);
- if (!summary_info) {
- spin_unlock(&adapter->maps_lock);
+ if (!summary_info)
return -EWOULDBLOCK;
- }
+
map = page_address(summary_info->page);
bit = get_ind_bit(summary_info->addr, adapter_int->summary_offset,
adapter->swap);
@@ -3077,7 +3077,8 @@ static int adapter_indicators_set_fast(struct kvm *kvm,
summary_set = test_and_set_bit(bit, map);
else
summary_set = test_and_clear_bit(bit, map);
- spin_unlock(&adapter->maps_lock);
+ mark_page_dirty(kvm, gpa_to_gfn(adapter_int->summary_gaddr));
+
return summary_set ? 0 : 1;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [GIT PULL v1 01/11] KVM: s390: Fix dirty marking in adapter_indicators_set*()
2026-09-04 6:06 ` [GIT PULL v1 01/11] KVM: s390: Fix dirty marking in adapter_indicators_set*() Claudio Imbrenda
@ 2026-09-04 6:15 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-04 6:15 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: Christian Borntraeger, Heiko Carstens, Vasily Gorbik, kvm,
Alexander Gordeev, linux-s390
> When the indicator and/or summary bits are set in the guest, the
> accessed page was only marked dirty in KVM if the access was performed
> using the slow path; accesses through the new kvm_arch_set_irq_inatomic
> fast inject path would not mark the page as dirty.
>
> Fix by adding/moving the missing calls to mark_page_dirty(). Note that
> for the inatomic path set_page_dirty{,_lock}() is not needed as the
> page stays pinned; the unpin path correctly marks it as dirty.
>
> Opportunistically reorder the local variables to be in reverse
> Christmas tree order and refactor to use guard().
>
> Fixes: 1e95e3bc6b05 ("KVM: s390: Enable adapter_indicators_set to use mapped pages")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260904060647.562443-1-imbrenda@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [GIT PULL v1 02/11] KVM: s390: Fix compile warning for kvm_s390_update_cmma_dirty()
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
2026-09-04 6:06 ` [GIT PULL v1 01/11] KVM: s390: Fix dirty marking in adapter_indicators_set*() Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:13 ` sashiko-bot
2026-09-04 6:06 ` [GIT PULL v1 03/11] KVM: s390: Fix _gaccess_shadow_fault() Claudio Imbrenda
` (9 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
The parameter "old" should be marked as const, to prevent compile-time
warnings.
Fixes: d487a24041c2 ("KVM: s390: Prepare gmap for a second KVM implementation")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260828115439.145885-3-imbrenda@linux.ibm.com>
---
arch/s390/kvm/s390/s390.c | 2 +-
arch/s390/kvm/s390/s390.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kvm/s390/s390.c b/arch/s390/kvm/s390/s390.c
index b0839e887221..8f7e09d7d049 100644
--- a/arch/s390/kvm/s390/s390.c
+++ b/arch/s390/kvm/s390/s390.c
@@ -5766,7 +5766,7 @@ static long cmma_d_count_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_
return 0;
}
-void kvm_s390_update_cmma_dirty(struct kvm *kvm, struct kvm_memory_slot *old)
+void kvm_s390_update_cmma_dirty(struct kvm *kvm, const struct kvm_memory_slot *old)
{
const struct dat_walk_ops ops = { .pte_entry = cmma_d_count_pte, };
diff --git a/arch/s390/kvm/s390/s390.h b/arch/s390/kvm/s390/s390.h
index d284a263ba70..aa0d1d062f8d 100644
--- a/arch/s390/kvm/s390/s390.h
+++ b/arch/s390/kvm/s390/s390.h
@@ -472,7 +472,7 @@ int __kvm_s390_mprotect_many(struct gmap *gmap, gpa_t gpa, u8 npages, unsigned i
unsigned long bits);
bool kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu);
-void kvm_s390_update_cmma_dirty(struct kvm *kvm, struct kvm_memory_slot *old);
+void kvm_s390_update_cmma_dirty(struct kvm *kvm, const struct kvm_memory_slot *old);
int kvm_s390_vm_stop_migration(struct kvm *kvm);
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [GIT PULL v1 03/11] KVM: s390: Fix _gaccess_shadow_fault()
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
2026-09-04 6:06 ` [GIT PULL v1 01/11] KVM: s390: Fix dirty marking in adapter_indicators_set*() Claudio Imbrenda
2026-09-04 6:06 ` [GIT PULL v1 02/11] KVM: s390: Fix compile warning for kvm_s390_update_cmma_dirty() Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:18 ` sashiko-bot
2026-09-04 6:06 ` [GIT PULL v1 04/11] KVM: s390: Refactor dat_set_slot() Claudio Imbrenda
` (8 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
In some circumstances, it is possible that the page of nested guest
memory that is being shadowed is not present at all in the parent guest
gmap. dat_entry_walk() will not find any leaf entry and return with
-ENOENT, which will erroneously be propagated all the way to userspace.
Fix by manually calling gmap_link() on the memory of the nested guest
that is being shadowed if the mapping was not already present.
Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260828115439.145885-4-imbrenda@linux.ibm.com>
---
arch/s390/kvm/s390/gaccess.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/s390/kvm/s390/gaccess.c b/arch/s390/kvm/s390/gaccess.c
index e5c064f263df..405345ccc4f1 100644
--- a/arch/s390/kvm/s390/gaccess.c
+++ b/arch/s390/kvm/s390/gaccess.c
@@ -1589,12 +1589,25 @@ static inline int ___gaccess_shadow_fault(struct kvm_vcpu *vcpu, struct gmap *sg
parent = READ_ONCE(sg->parent);
if (!parent)
return -EAGAIN;
+retry:
scoped_guard(spinlock, &parent->children_lock) {
if (READ_ONCE(sg->parent) != parent)
return -EAGAIN;
sg->invalidated = false;
rc = _gaccess_do_shadow(vcpu->arch.mc, sg, saddr, walk);
}
+ if (rc == -ENOENT) {
+ struct kvm_memory_slot *slot;
+ struct guest_fault *entries;
+
+ entries = get_entries(walk);
+ slot = kvm_vcpu_gfn_to_memslot(vcpu, entries[LEVEL_MEM].gfn);
+ if (!slot)
+ return PGM_ADDRESSING;
+ rc = gmap_link(vcpu->arch.mc, parent, entries + LEVEL_MEM, slot);
+ if (!rc)
+ goto retry;
+ }
if (!rc)
kvm_s390_release_faultin_array(vcpu->kvm, walk->raw_entries, false);
return rc;
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [GIT PULL v1 04/11] KVM: s390: Refactor dat_set_slot()
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
` (2 preceding siblings ...)
2026-09-04 6:06 ` [GIT PULL v1 03/11] KVM: s390: Fix _gaccess_shadow_fault() Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:23 ` sashiko-bot
2026-09-04 6:06 ` [GIT PULL v1 05/11] KVM: s390: Move all code into s390_kvm_mmu_prepare_memory_region() Claudio Imbrenda
` (7 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
Refactor dat_set_slot(), _dat_slot_pte(), _dat_slot_crste(). Now they
only take a struct kvm_s390_mmu_cache as priv. For dat_delete_slot(),
mc is NULL, as no allocations should take place.
This is needed as a prerequisite to move gmap DAT table setup from
kvm_arch_commit_memory_region() to kvm_arch_prepare_memory_region().
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260828115439.145885-5-imbrenda@linux.ibm.com>
---
arch/s390/kvm/gmap/dat.c | 29 +++++++++--------------------
arch/s390/kvm/gmap/dat.h | 10 ++++------
arch/s390/kvm/gmap/kvm_mmu.c | 4 ++--
3 files changed, 15 insertions(+), 28 deletions(-)
diff --git a/arch/s390/kvm/gmap/dat.c b/arch/s390/kvm/gmap/dat.c
index 24547e39fab2..dcedd5479d82 100644
--- a/arch/s390/kvm/gmap/dat.c
+++ b/arch/s390/kvm/gmap/dat.c
@@ -846,19 +846,12 @@ long dat_reset_skeys(union asce asce, gfn_t start)
}
#endif /* KVM_S390_MANAGES_S390_GUEST */
-struct slot_priv {
- unsigned long token;
- struct kvm_s390_mmu_cache *mc;
-};
-
static long _dat_slot_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_walk *walk)
{
- struct slot_priv *p = walk->priv;
- union crste dummy = { .val = p->token };
union pte new_pte, pte = READ_ONCE(*ptep);
union pgste pgste;
- new_pte = _PTE_TOK(dummy.tok.type, dummy.tok.par);
+ new_pte = walk->priv ? _PTE_EMPTY : _PTE_TOK(_DAT_TOKEN_PIC, PGM_ADDRESSING);
/* Table entry already in the desired state. */
if (pte.val == new_pte.val)
@@ -875,10 +868,9 @@ static long _dat_slot_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_wal
static long _dat_slot_crste(union crste *crstep, gfn_t gfn, gfn_t next, struct dat_walk *walk)
{
union crste new_crste, crste = READ_ONCE(*crstep);
- struct slot_priv *p = walk->priv;
+ struct kvm_s390_mmu_cache *mc = walk->priv;
- new_crste.val = p->token;
- new_crste.h.tt = crste.h.tt;
+ new_crste = mc ? _CRSTE_EMPTY(crste.h.tt) : _CRSTE_HOLE(crste.h.tt);
/* Table entry already in the desired state. */
if (crste.val == new_crste.val)
@@ -902,7 +894,10 @@ static long _dat_slot_crste(union crste *crstep, gfn_t gfn, gfn_t next, struct d
if (!crste.h.fc && !crste.h.i)
return 0;
/* Split (install a lower level table), and handle things there. */
- return dat_split_crste(p->mc, crstep, gfn, walk->asce, false);
+ if (mc)
+ return dat_split_crste(mc, crstep, gfn, walk->asce, false);
+ /* A large page should never cross memslots boundaries */
+ return -EINVAL;
}
static const struct dat_walk_ops dat_slot_ops = {
@@ -910,16 +905,10 @@ static const struct dat_walk_ops dat_slot_ops = {
.crste_ops = { _dat_slot_crste, _dat_slot_crste, _dat_slot_crste, _dat_slot_crste, },
};
-int dat_set_slot(struct kvm_s390_mmu_cache *mc, union asce asce, gfn_t start, gfn_t end,
- u16 type, u16 param)
+int dat_set_slot(struct kvm_s390_mmu_cache *mc, union asce asce, gfn_t start, gfn_t end)
{
- struct slot_priv priv = {
- .token = _CRSTE_TOK(0, type, param).val,
- .mc = mc,
- };
-
return _dat_walk_gfn_range(start, end, asce, &dat_slot_ops,
- DAT_WALK_IGN_HOLES | DAT_WALK_ANY, &priv);
+ DAT_WALK_IGN_HOLES | DAT_WALK_ANY, mc);
}
static void pgste_set_unlock_multiple(union pte *first, int n, union pgste *pgstes)
diff --git a/arch/s390/kvm/gmap/dat.h b/arch/s390/kvm/gmap/dat.h
index e452c141b841..90389d47ba4e 100644
--- a/arch/s390/kvm/gmap/dat.h
+++ b/arch/s390/kvm/gmap/dat.h
@@ -547,8 +547,7 @@ long dat_reset_skeys(union asce asce, gfn_t start);
unsigned long dat_get_ptval(struct page_table *table, struct ptval_param param);
void dat_set_ptval(struct page_table *table, struct ptval_param param, unsigned long val);
-int dat_set_slot(struct kvm_s390_mmu_cache *mc, union asce asce, gfn_t start, gfn_t end,
- u16 type, u16 param);
+int dat_set_slot(struct kvm_s390_mmu_cache *mc, union asce asce, gfn_t start, gfn_t end);
#if KVM_S390_MANAGES_S390_GUEST
int dat_set_prefix_notif_bit(union asce asce, gfn_t gfn);
@@ -973,16 +972,15 @@ static inline int get_level(union crste *crstep, union pte *ptep)
return ptep ? TABLE_TYPE_PAGE_TABLE : crstep->h.tt;
}
-static inline int dat_delete_slot(struct kvm_s390_mmu_cache *mc, union asce asce, gfn_t start,
- unsigned long npages)
+static inline int dat_delete_slot(union asce asce, gfn_t start, unsigned long npages)
{
- return dat_set_slot(mc, asce, start, start + npages, _DAT_TOKEN_PIC, PGM_ADDRESSING);
+ return dat_set_slot(NULL, asce, start, start + npages);
}
static inline int dat_create_slot(struct kvm_s390_mmu_cache *mc, union asce asce, gfn_t start,
unsigned long npages)
{
- return dat_set_slot(mc, asce, start, start + npages, _DAT_TOKEN_NONE, 0);
+ return dat_set_slot(mc, asce, start, start + npages);
}
static inline bool crste_is_ucas(union crste crste)
diff --git a/arch/s390/kvm/gmap/kvm_mmu.c b/arch/s390/kvm/gmap/kvm_mmu.c
index b08b8229bb6f..4c8054e18490 100644
--- a/arch/s390/kvm/gmap/kvm_mmu.c
+++ b/arch/s390/kvm/gmap/kvm_mmu.c
@@ -111,10 +111,10 @@ void s390_kvm_mmu_commit_memory_region(struct kvm *kvm,
kvm_s390_update_cmma_dirty(kvm, old);
switch (change) {
case KVM_MR_DELETE:
- rc = dat_delete_slot(mc, kvm->arch.gmap->asce, old->base_gfn, old->npages);
+ rc = dat_delete_slot(kvm->arch.gmap->asce, old->base_gfn, old->npages);
break;
case KVM_MR_MOVE:
- rc = dat_delete_slot(mc, kvm->arch.gmap->asce, old->base_gfn, old->npages);
+ rc = dat_delete_slot(kvm->arch.gmap->asce, old->base_gfn, old->npages);
if (rc)
break;
fallthrough;
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [GIT PULL v1 04/11] KVM: s390: Refactor dat_set_slot()
2026-09-04 6:06 ` [GIT PULL v1 04/11] KVM: s390: Refactor dat_set_slot() Claudio Imbrenda
@ 2026-09-04 6:23 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-04 6:23 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: Christian Borntraeger, Heiko Carstens, Alexander Gordeev,
Vasily Gorbik, kvm, linux-s390
> Refactor dat_set_slot(), _dat_slot_pte(), _dat_slot_crste(). Now they
> only take a struct kvm_s390_mmu_cache as priv. For dat_delete_slot(),
> mc is NULL, as no allocations should take place.
>
> This is needed as a prerequisite to move gmap DAT table setup from
> kvm_arch_commit_memory_region() to kvm_arch_prepare_memory_region().
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260904060647.562443-1-imbrenda@linux.ibm.com?part=4
^ permalink raw reply [flat|nested] 24+ messages in thread
* [GIT PULL v1 05/11] KVM: s390: Move all code into s390_kvm_mmu_prepare_memory_region()
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
` (3 preceding siblings ...)
2026-09-04 6:06 ` [GIT PULL v1 04/11] KVM: s390: Refactor dat_set_slot() Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:19 ` sashiko-bot
2026-09-04 6:06 ` [GIT PULL v1 06/11] KVM: s390: Add missing srcu in kvm_s390_set_irq_state() Claudio Imbrenda
` (6 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
Move all code from s390_kvm_mmu_commit_memory_region() into
s390_kvm_mmu_prepare_memory_region(). This allows the function to fail
gracefully if needed. The previous behaviour was to print a warning and
continue execution with page tables inconsistent with the memslots.
Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260828115439.145885-6-imbrenda@linux.ibm.com>
---
arch/s390/kvm/gmap/kvm_mmu.c | 89 +++++++++++++++---------------------
arch/s390/kvm/gmap/kvm_mmu.h | 4 --
arch/s390/kvm/s390/s390.c | 1 -
3 files changed, 36 insertions(+), 58 deletions(-)
diff --git a/arch/s390/kvm/gmap/kvm_mmu.c b/arch/s390/kvm/gmap/kvm_mmu.c
index 4c8054e18490..c2ffb5e59ec6 100644
--- a/arch/s390/kvm/gmap/kvm_mmu.c
+++ b/arch/s390/kvm/gmap/kvm_mmu.c
@@ -47,6 +47,9 @@ int s390_kvm_mmu_prepare_memory_region(struct kvm *kvm,
struct kvm_memory_slot *new,
enum kvm_mr_change change)
{
+ struct kvm_s390_mmu_cache *mc __free(kvm_s390_mmu_cache) = NULL;
+ int rc = 0;
+
if (kvm_is_ucontrol(kvm) && new && new->id < KVM_USER_MEM_SLOTS)
return -EINVAL;
@@ -61,6 +64,10 @@ int s390_kvm_mmu_prepare_memory_region(struct kvm *kvm,
* and munmap() stuff in this slot after doing this call at any
* time.
*/
+ if (change != KVM_MR_MOVE && change != KVM_MR_CREATE) {
+ WARN(1, "Unknown KVM MR CHANGE: %d\n", change);
+ return -EINVAL;
+ }
if (new->userspace_addr & ~PAGE_MASK)
return -EINVAL;
if ((new->base_gfn + new->npages) * PAGE_SIZE > kvm->arch.mem_limit)
@@ -69,65 +76,41 @@ int s390_kvm_mmu_prepare_memory_region(struct kvm *kvm,
return -EINVAL;
}
- if (!kvm_s390_is_migration_mode(kvm))
- return 0;
-
- /*
- * Turn off migration mode when:
- * - userspace creates a new memslot with dirty logging off,
- * - userspace modifies an existing memslot (MOVE or FLAGS_ONLY) and
- * dirty logging is turned off.
- * Migration mode expects dirty page logging being enabled to store
- * its dirty bitmap.
- */
- if (change != KVM_MR_DELETE &&
- !(new->flags & KVM_MEM_LOG_DIRTY_PAGES))
- WARN(kvm_s390_vm_stop_migration(kvm),
- "Failed to stop migration mode");
-
- return 0;
-}
-
-void s390_kvm_mmu_commit_memory_region(struct kvm *kvm,
- struct kvm_memory_slot *old,
- const struct kvm_memory_slot *new,
- enum kvm_mr_change change)
-{
- struct kvm_s390_mmu_cache *mc __free(kvm_s390_mmu_cache) = NULL;
- int rc = 0;
-
- guard(mutex)(&kvm->slots_arch_lock);
+ if (kvm->arch.migration_mode) {
+ /*
+ * Turn off migration mode when:
+ * - userspace creates a new memslot with dirty logging off,
+ * - userspace modifies an existing memslot (MOVE or FLAGS_ONLY)
+ * and dirty logging is turned off.
+ * Migration mode expects dirty page logging being enabled to
+ * store its dirty bitmap.
+ */
+ if (change != KVM_MR_DELETE &&
+ !(new->flags & KVM_MEM_LOG_DIRTY_PAGES))
+ WARN(kvm_s390_vm_stop_migration(kvm),
+ "Failed to stop migration mode");
+ }
if (change == KVM_MR_FLAGS_ONLY)
- return;
-
- mc = kvm_s390_new_mmu_cache();
- if (!mc) {
- rc = -ENOMEM;
- goto out;
+ return 0;
+ if (change != KVM_MR_DELETE) {
+ /* Enough capacity to add a new memslot */
+ mc = kvm_s390_new_mmu_cache();
+ if (!mc)
+ return -ENOMEM;
}
-
scoped_guard(write_lock, &kvm->mmu_lock) {
kvm_s390_update_cmma_dirty(kvm, old);
- switch (change) {
- case KVM_MR_DELETE:
- rc = dat_delete_slot(kvm->arch.gmap->asce, old->base_gfn, old->npages);
- break;
- case KVM_MR_MOVE:
+ if (change == KVM_MR_DELETE || change == KVM_MR_MOVE)
rc = dat_delete_slot(kvm->arch.gmap->asce, old->base_gfn, old->npages);
- if (rc)
- break;
- fallthrough;
- case KVM_MR_CREATE:
+ if (!rc && (change == KVM_MR_MOVE || change == KVM_MR_CREATE))
rc = dat_create_slot(mc, kvm->arch.gmap->asce, new->base_gfn, new->npages);
- break;
- case KVM_MR_FLAGS_ONLY:
- break;
- default:
- WARN(1, "Unknown KVM MR CHANGE: %d\n", change);
- }
}
-out:
- if (rc)
- pr_warn("failed to commit memory region\n");
+ /*
+ * Can only be triggered if dat_{create,delete}_slot() found an
+ * internal inconsistency or if the mmu cache ran out of memory;
+ * both should be impossible.
+ */
+ KVM_BUG_ON(rc, kvm);
+ return rc;
}
diff --git a/arch/s390/kvm/gmap/kvm_mmu.h b/arch/s390/kvm/gmap/kvm_mmu.h
index cdbd390bd33c..43cde61bae03 100644
--- a/arch/s390/kvm/gmap/kvm_mmu.h
+++ b/arch/s390/kvm/gmap/kvm_mmu.h
@@ -10,9 +10,5 @@ int s390_kvm_mmu_prepare_memory_region(struct kvm *kvm,
const struct kvm_memory_slot *old,
struct kvm_memory_slot *new,
enum kvm_mr_change change);
-void s390_kvm_mmu_commit_memory_region(struct kvm *kvm,
- struct kvm_memory_slot *old,
- const struct kvm_memory_slot *new,
- enum kvm_mr_change change);
#endif /* ARCH_KVM_GMAP_KVM_MMU_H */
diff --git a/arch/s390/kvm/s390/s390.c b/arch/s390/kvm/s390/s390.c
index 8f7e09d7d049..eca4a4359ab2 100644
--- a/arch/s390/kvm/s390/s390.c
+++ b/arch/s390/kvm/s390/s390.c
@@ -5781,7 +5781,6 @@ void kvm_arch_commit_memory_region(struct kvm *kvm, struct kvm_memory_slot *old,
const struct kvm_memory_slot *new,
enum kvm_mr_change change)
{
- s390_kvm_mmu_commit_memory_region(kvm, old, new, change);
}
/**
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [GIT PULL v1 05/11] KVM: s390: Move all code into s390_kvm_mmu_prepare_memory_region()
2026-09-04 6:06 ` [GIT PULL v1 05/11] KVM: s390: Move all code into s390_kvm_mmu_prepare_memory_region() Claudio Imbrenda
@ 2026-09-04 6:19 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-04 6:19 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, kvm, linux-s390
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Migration mode is permanently stopped if MMU cache allocation fails during memslot prepare phase.
--
commit 946bbe22fec184c4059d86ca088a0d825405309b
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>
KVM: s390: Move all code into s390_kvm_mmu_prepare_memory_region()
This commit moves all code from s390_kvm_mmu_commit_memory_region() into
s390_kvm_mmu_prepare_memory_region() to allow graceful failure. This fixes
issues where execution would continue with inconsistent page tables.
Message-ID: <20260828115439.145885-6-imbrenda@linux.ibm.com>
> diff --git a/arch/s390/kvm/gmap/kvm_mmu.c b/arch/s390/kvm/gmap/kvm_mmu.c
> index 4c8054e184908..c2ffb5e59ec63 100644
> --- a/arch/s390/kvm/gmap/kvm_mmu.c
> +++ b/arch/s390/kvm/gmap/kvm_mmu.c
[ ... ]
> @@ -69,65 +76,41 @@ int s390_kvm_mmu_prepare_memory_region(struct kvm *kvm,
[ ... ]
> + if (kvm->arch.migration_mode) {
> + /*
> + * Turn off migration mode when:
> + * - userspace creates a new memslot with dirty logging off,
> + * - userspace modifies an existing memslot (MOVE or FLAGS_ONLY)
> + * and dirty logging is turned off.
> + * Migration mode expects dirty page logging being enabled to
> + * store its dirty bitmap.
> + */
> + if (change != KVM_MR_DELETE &&
> + !(new->flags & KVM_MEM_LOG_DIRTY_PAGES))
> + WARN(kvm_s390_vm_stop_migration(kvm),
> + "Failed to stop migration mode");
> + }
>
> if (change == KVM_MR_FLAGS_ONLY)
> - return;
> -
> - mc = kvm_s390_new_mmu_cache();
> - if (!mc) {
> - rc = -ENOMEM;
> - goto out;
> + return 0;
> + if (change != KVM_MR_DELETE) {
> + /* Enough capacity to add a new memslot */
> + mc = kvm_s390_new_mmu_cache();
> + if (!mc)
> + return -ENOMEM;
[Severity: Medium]
Does this early return leave the VM with migration permanently stopped if the
mmu cache allocation fails?
In the previous block, s390_kvm_mmu_prepare_memory_region() calls
kvm_s390_vm_stop_migration() when migration mode is active and dirty logging
is off. If kvm_s390_new_mmu_cache() fails here and returns -ENOMEM, the KVM
core will roll back the ioctl and the memslot will retain its previous
configuration.
However, the migration mode stoppage is not rolled back, potentially leaving
the VM in an inconsistent state where global migration mode is stopped despite
the memslot update failing.
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260904060647.562443-1-imbrenda@linux.ibm.com?part=5
^ permalink raw reply [flat|nested] 24+ messages in thread
* [GIT PULL v1 06/11] KVM: s390: Add missing srcu in kvm_s390_set_irq_state()
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
` (4 preceding siblings ...)
2026-09-04 6:06 ` [GIT PULL v1 05/11] KVM: s390: Move all code into s390_kvm_mmu_prepare_memory_region() Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:16 ` sashiko-bot
2026-09-04 6:06 ` [GIT PULL v1 07/11] KVM: s390: Fix potential races in dat skey functions Claudio Imbrenda
` (5 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
Like kvm_s390_inject_vcpu(), kvm_s390_set_irq_state() also needs the
kvm->srcu or the slots lock when performing the Store status operation.
Fix by taking kvm->srcu in kvm_s390_set_irq_state().
Fixes: ba5c1e9b6cee ("KVM: s390: interrupt subsystem, cpu timer, waitpsw")
Fixes: 062e44a9319f ("KVM: s390: Use srcu in kvm_arch_vcpu_unlocked_ioctl()")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260828115439.145885-7-imbrenda@linux.ibm.com>
---
arch/s390/kvm/s390/interrupt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kvm/s390/interrupt.c b/arch/s390/kvm/s390/interrupt.c
index 49b4e233e791..f892f4307883 100644
--- a/arch/s390/kvm/s390/interrupt.c
+++ b/arch/s390/kvm/s390/interrupt.c
@@ -3229,9 +3229,9 @@ int kvm_s390_set_irq_state(struct kvm_vcpu *vcpu, void __user *irqstate, int len
break;
}
}
-
if (storestatus) {
- n = kvm_s390_store_status_unloaded(vcpu, KVM_S390_STORE_STATUS_NOADDR);
+ scoped_guard(srcu, &vcpu->kvm->srcu)
+ n = kvm_s390_store_status_unloaded(vcpu, KVM_S390_STORE_STATUS_NOADDR);
return r ? r : n;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [GIT PULL v1 07/11] KVM: s390: Fix potential races in dat skey functions
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
` (5 preceding siblings ...)
2026-09-04 6:06 ` [GIT PULL v1 06/11] KVM: s390: Add missing srcu in kvm_s390_set_irq_state() Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:20 ` sashiko-bot
2026-09-04 6:06 ` [GIT PULL v1 08/11] KVM: s390: Fix race in _destroy_pages_crste() Claudio Imbrenda
` (4 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
When dat_cond_set_storage_key() finds a large page, it will
conditionally set the storage key in absolute memory using
large_crste_to_phys() to get the absolute address.
There is a race window between dat_entry_walk() and
large_crste_to_phys(): the large page could have been split
concurrently, and large_crste_to_phys() might be called with a crste
that does not designate a large page, leading to crashes.
Similar issues were also present in dat_set_storage_key().
dat_get_storage_key() and dat_reset_reference_bit() did instead check
for a potential concurrent splitting of the large page, but then
handled it incorrectly.
Fix by performing a READ_ONCE on the crste pointer, checking and using
the result, instead of dereferencing the pointer again. In case a race
is detacted, try dat_entry_walk() again.
Fixes: 8e03e8316eb2 ("KVM: s390: KVM page table management functions: storage keys")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260828115439.145885-8-imbrenda@linux.ibm.com>
---
arch/s390/kvm/gmap/dat.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/arch/s390/kvm/gmap/dat.c b/arch/s390/kvm/gmap/dat.c
index dcedd5479d82..ff80d02c9f56 100644
--- a/arch/s390/kvm/gmap/dat.c
+++ b/arch/s390/kvm/gmap/dat.c
@@ -621,17 +621,20 @@ int dat_get_storage_key(union asce asce, gfn_t gfn, union skey *skey)
union pte *ptep;
int rc;
+again:
skey->skey = 0;
rc = dat_entry_walk(NULL, gfn, asce, DAT_WALK_ANY, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep);
if (rc)
return rc;
if (!ptep) {
- union crste crste;
+ union crste crste = READ_ONCE(*crstep);
- crste = READ_ONCE(*crstep);
- if (!crste.h.fc || !crste.s.fc1.pr)
+ if (!crste_leaf(crste) && !crste.h.i)
+ goto again;
+ if (!crste.s.fc1.pr)
return 0;
+
skey->skey = page_get_storage_key(large_crste_to_phys(crste, gfn));
return 0;
}
@@ -662,13 +665,20 @@ int dat_set_storage_key(struct kvm_s390_mmu_cache *mc, union asce asce, gfn_t gf
union pte *ptep;
int rc;
+again:
rc = dat_entry_walk(mc, gfn, asce, DAT_WALK_LEAF_ALLOC, TABLE_TYPE_PAGE_TABLE,
&crstep, &ptep);
if (rc)
return rc;
if (!ptep) {
- page_set_storage_key(large_crste_to_phys(*crstep, gfn), skey.skey, !nq);
+ union crste crste = READ_ONCE(*crstep);
+
+ /* A large page has been split concurrently, try again */
+ if (!crste_leaf(crste))
+ goto again;
+
+ page_set_storage_key(large_crste_to_phys(crste, gfn), skey.skey, !nq);
return 0;
}
@@ -718,15 +728,22 @@ int dat_cond_set_storage_key(struct kvm_s390_mmu_cache *mmc, union asce asce, gf
union pte *ptep;
int rc;
+again:
rc = dat_entry_walk(mmc, gfn, asce, DAT_WALK_LEAF_ALLOC, TABLE_TYPE_PAGE_TABLE,
&crstep, &ptep);
if (rc)
return rc;
if (!ptep) {
+ union crste crste = READ_ONCE(*crstep);
+
+ /* A large page has been split concurrently, try again */
+ if (!crste_leaf(crste))
+ goto again;
if (!oldkey)
oldkey = &prev;
- return page_cond_set_storage_key(large_crste_to_phys(*crstep, gfn), skey, oldkey,
+
+ return page_cond_set_storage_key(large_crste_to_phys(crste, gfn), skey, oldkey,
nq, mr, mc);
}
@@ -768,7 +785,7 @@ int dat_reset_reference_bit(union asce asce, gfn_t gfn, union skey *skey)
int rc;
skey->skey = 0;
-
+again:
rc = dat_entry_walk(NULL, gfn, asce, DAT_WALK_ANY, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep);
if (rc)
return rc;
@@ -776,9 +793,12 @@ int dat_reset_reference_bit(union asce asce, gfn_t gfn, union skey *skey)
if (!ptep) {
union crste crste = READ_ONCE(*crstep);
- if (!crste.h.fc || !crste.s.fc1.pr)
+ /* A large page has been split concurrently, try again */
+ if (!crste_leaf(crste) && !crste.h.i)
+ goto again;
+ if (!crste.s.fc1.pr)
return 0;
- skey->skey = page_reset_referenced(large_crste_to_phys(*crstep, gfn)) << 1;
+ skey->skey = page_reset_referenced(large_crste_to_phys(crste, gfn)) << 1;
return 0;
}
old = pgste_get_lock(ptep);
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [GIT PULL v1 07/11] KVM: s390: Fix potential races in dat skey functions
2026-09-04 6:06 ` [GIT PULL v1 07/11] KVM: s390: Fix potential races in dat skey functions Claudio Imbrenda
@ 2026-09-04 6:20 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-04 6:20 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: Christian Borntraeger, Alexander Gordeev, linux-s390, kvm,
Vasily Gorbik, Heiko Carstens
> When dat_cond_set_storage_key() finds a large page, it will
> conditionally set the storage key in absolute memory using
> large_crste_to_phys() to get the absolute address.
>
> There is a race window between dat_entry_walk() and
> large_crste_to_phys(): the large page could have been split
> concurrently, and large_crste_to_phys() might be called with a crste
> that does not designate a large page, leading to crashes.
>
> Similar issues were also present in dat_set_storage_key().
>
> dat_get_storage_key() and dat_reset_reference_bit() did instead check
> for a potential concurrent splitting of the large page, but then
> handled it incorrectly.
>
> Fix by performing a READ_ONCE on the crste pointer, checking and using
> the result, instead of dereferencing the pointer again. In case a race
> is detacted, try dat_entry_walk() again.
>
> Fixes: 8e03e8316eb2 ("KVM: s390: KVM page table management functions: storage keys")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260904060647.562443-1-imbrenda@linux.ibm.com?part=7
^ permalink raw reply [flat|nested] 24+ messages in thread
* [GIT PULL v1 08/11] KVM: s390: Fix race in _destroy_pages_crste()
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
` (6 preceding siblings ...)
2026-09-04 6:06 ` [GIT PULL v1 07/11] KVM: s390: Fix potential races in dat skey functions Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:22 ` sashiko-bot
2026-09-04 6:06 ` [GIT PULL v1 09/11] s390/vfio-ap: fix KVM GISC and page leak when queue removed from host config Claudio Imbrenda
` (3 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
Use READ_ONCE() in _destroy_pages_crste() to read the crste, avoid
dereferencing the pointer multiple times.
Fixes: a2c17f9270cc ("KVM: s390: New gmap code")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260828115439.145885-9-imbrenda@linux.ibm.com>
---
arch/s390/kvm/gmap/gmap.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kvm/gmap/gmap.c b/arch/s390/kvm/gmap/gmap.c
index 4968330e9553..3f3fa864cc36 100644
--- a/arch/s390/kvm/gmap/gmap.c
+++ b/arch/s390/kvm/gmap/gmap.c
@@ -994,11 +994,13 @@ static long _destroy_pages_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct da
static long _destroy_pages_crste(union crste *crstep, gfn_t gfn, gfn_t next, struct dat_walk *walk)
{
phys_addr_t origin, cur, end;
+ union crste crste;
- if (!crstep->h.fc || !crstep->s.fc1.pr)
+ crste = READ_ONCE(*crstep);
+ if (!crste.h.fc || !crste.s.fc1.pr)
return 0;
- origin = crste_origin_large(*crstep);
+ origin = crste_origin_large(crste);
cur = ((max(gfn, walk->start) - gfn) << PAGE_SHIFT) + origin;
end = ((min(next, walk->end) - gfn) << PAGE_SHIFT) + origin;
for ( ; cur < end; cur += PAGE_SIZE)
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [GIT PULL v1 08/11] KVM: s390: Fix race in _destroy_pages_crste()
2026-09-04 6:06 ` [GIT PULL v1 08/11] KVM: s390: Fix race in _destroy_pages_crste() Claudio Imbrenda
@ 2026-09-04 6:22 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-04 6:22 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: linux-s390, Heiko Carstens, Alexander Gordeev,
Christian Borntraeger, kvm, Vasily Gorbik
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Time-of-check to time-of-use (TOCTOU) vulnerability in `_destroy_pages_pte()` allows arbitrary physical page destruction.
- [High] _destroy_pages_crste loops over up to 524,288 pages for a 2GB large page without yielding, causing massive latency and soft lockups.
- [High] Stale PTE value used to construct new PTE in `gmap_clear_young_pte()` overwrites concurrent changes.
--
commit beff0ee5655a939df38ade64a4fe7014bdc87cd5
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>
KVM: s390: Fix race in _destroy_pages_crste()
This commit uses READ_ONCE() in _destroy_pages_crste() to read the crste,
avoiding dereferencing the pointer multiple times.
> diff --git a/arch/s390/kvm/gmap/gmap.c b/arch/s390/kvm/gmap/gmap.c
> index 4968330e9553b..3f3fa864cc36b 100644
> --- a/arch/s390/kvm/gmap/gmap.c
> +++ b/arch/s390/kvm/gmap/gmap.c
> @@ -994,11 +994,13 @@ static long _destroy_pages_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct da
> static long _destroy_pages_crste(union crste *crstep, gfn_t gfn, gfn_t next, struct dat_walk *walk)
> {
> phys_addr_t origin, cur, end;
> + union crste crste;
>
> - if (!crstep->h.fc || !crstep->s.fc1.pr)
> + crste = READ_ONCE(*crstep);
> + if (!crste.h.fc || !crste.s.fc1.pr)
> return 0;
>
> - origin = crste_origin_large(*crstep);
> + origin = crste_origin_large(crste);
> cur = ((max(gfn, walk->start) - gfn) << PAGE_SHIFT) + origin;
> end = ((min(next, walk->end) - gfn) << PAGE_SHIFT) + origin;
> for ( ; cur < end; cur += PAGE_SIZE)
[Severity: High]
This is a pre-existing issue, but does this loop process the entirety of the
large page before checking if preemption is needed?
If a PV guest is backed by 2GB large pages (Region 3 / PUD), could the loop
execute up to 524,288 Ultravisor calls without yielding, potentially causing
massive latency or a host soft lockup?
[Severity: Critical]
While looking at this file, I also noticed a pre-existing potential TOCTOU
vulnerability in _destroy_pages_pte():
arch/s390/kvm/gmap/gmap.c:_destroy_pages_pte() {
if (!ptep->s.pr)
return 0;
__kvm_s390_pv_destroy_page(phys_to_page(pte_origin(*ptep)));
}
Can a concurrent modification to the PTE (e.g., by another thread handling a
page fault or unmapping memory) occur between the lockless ptep->s.pr check
and the subsequent pte_origin(*ptep) fetch?
If the PTE is cleared concurrently, could pte_origin evaluate to 0, causing
phys_to_page(0) to be targeted by the Ultravisor hypercall? Could this allow
an untrusted guest vCPU to cause destruction of an arbitrary host physical
page?
[Severity: High]
Additionally, there is a pre-existing issue where a stale PTE value appears
to be used to construct a new PTE in gmap_clear_young_pte(), which could
overwrite concurrent changes:
arch/s390/kvm/gmap/gmap.c:gmap_clear_young_pte() {
pte = READ_ONCE(*ptep);
if (!pte.s.pr || (!pte.s.y && pte.h.i))
return 0;
pgste = pgste_get_lock(ptep);
if (!pgste.prefix_notif || gmap_mkold_prefix(p->gmap, gfn, end)) {
new = pte;
new.h.i = 1;
new.s.y = 0;
...
pgste = __dat_ptep_xchg(ptep, pgste, new, gfn, walk->asce,
uses_skeys(p->gmap));
...
}
Since kvm_age_gfn() takes the KVM MMU lock for read, concurrent KVM fault
handlers can also modify the PTE. If a concurrent fault modifies the PTE
after gmap_clear_young_pte locklessly reads it, but before it acquires
pgste_get_lock, wouldn't the concurrent modification (such as setting the
dirty bit) be silently overwritten because the stale read value is used to
construct the new PTE?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260904060647.562443-1-imbrenda@linux.ibm.com?part=8
^ permalink raw reply [flat|nested] 24+ messages in thread
* [GIT PULL v1 09/11] s390/vfio-ap: fix KVM GISC and page leak when queue removed from host config
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
` (7 preceding siblings ...)
2026-09-04 6:06 ` [GIT PULL v1 08/11] KVM: s390: Fix race in _destroy_pages_crste() Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:22 ` sashiko-bot
2026-09-04 6:06 ` [GIT PULL v1 10/11] s390/uv: Fix loop condition in uv_find_secrets Claudio Imbrenda
` (2 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
From: Anthony Krowiak <akrowiak@linux.ibm.com>
Three related problems exist in the handling of KVM interrupt and page
resources when a queue is removed from the host's AP configuration
while assigned to a mediated device (mdev).
Problem 1:
~~~~~~~~~
AP_RESPONSE_Q_NOT_AVAIL not handled in vfio_ap_mdev_reset_queue()
When the AP bus removes a queue device whose adapter or domain has
been removed from the host's AP configuration,
vfio_ap_mdev_remove_queue() is called. If the queue is still in the
host's AP configuration at that point, it calls
vfio_ap_mdev_reset_queue(), which issues a PQAP(ZAPQ). Since the
adapter is already gone from the host configuration, ap_zapq() returns
AP_RESPONSE_Q_NOT_AVAIL (0x01). This response code is not handled in
vfio_ap_mdev_reset_queue()'s switch statement and falls through to
the default case, which issues a WARN but does not call
vfio_ap_free_aqic_resources(). As a result, if IRQ handling was
enabled for the queue by the guest, the KVM GISC registration and
the pinned guest page holding the notification indicator byte (NIB)
are both leaked.
This is fixed by adding AP_RESPONSE_Q_NOT_AVAIL to the same case as
AP_RESPONSE_DECONFIGURED and AP_RESPONSE_CHECKSTOPPED in
vfio_ap_mdev_reset_queue(). Like those response codes, Q_NOT_AVAIL
indicates the queue is not operational and no further reset attempts
are possible; the correct action is to free the IRQ resources
immediately.
Problem 2:
~~~~~~~~~
AP_RESPONSE_Q_NOT_AVAIL not handled in apq_status_check()
In vfio_ap_mdev_reset_queue(), there are four cases that indicate a queue
reset has not yet completed, in which case apq_reset_check() is queued to
a work queue to verify completion of the reset operation. This function
uses the PQAP(TAPQ) function to get the queue's status and calls
apq_status_check() to verify whether the reset has completed, failed or
needs to be executed again. As described in Problem #1 above,
apq_reset_check() does not specifically check for AP_RESPONSE_Q_NOT_AVAIL,
thereby potentially leaking KVM GISC registration and the pinned guest page
holding the NIB.
This is fixed by adding a case statement for AP_RESPONSE_Q_NOT_AVAIL to
apq_status_check() and returning -ENODEV for that case. The caller,
apq_reset_check() will then check for this return code and call
vfio_ap_free_aqic_resources() to prevent the leak.
Problem 3:
~~~~~~~~~
vfio_ap_free_aqic_resources() leaks saved_isc when kvm is NULL
vfio_ap_free_aqic_resources() guards the call to
kvm_s390_gisc_unregister() with:
if (q->saved_isc != VFIO_AP_ISC_INVALID &&
!WARN_ON(!(q->matrix_mdev && q->matrix_mdev->kvm)))
If matrix_mdev->kvm is NULL -- which can happen when
vfio_ap_mdev_unset_kvm() has already run and cleared kvm before a
subsequent cleanup path reaches this function -- the WARN_ON fires
and the entire block is skipped. This leaves q->saved_isc set to a
non-invalid value, creating a potential double-free on any subsequent
call to this function.
When kvm is NULL the KVM guest is already torn down, so
kvm_s390_gisc_unregister() need not and cannot be called; however,
q->saved_isc must always be cleared. Fix this by separating the
kvm_s390_gisc_unregister() call from the q->saved_isc reset. The
WARN_ON now guards only the genuinely impossible case of matrix_mdev
being NULL. A NULL kvm is handled gracefully by skipping only the
unregister call, and q->saved_isc = VFIO_AP_ISC_INVALID is set
unconditionally whenever saved_isc was not already invalid.
Additionally, add an else clause to the host-config check in
vfio_ap_mdev_remove_queue() to call vfio_ap_free_aqic_resources()
directly when the queue is not in the host's AP configuration. This
serves as a backstop: when the AP bus fires the driver .remove
callback after an adapter is removed from the host config, the queue
is by definition no longer addressable, so vfio_ap_mdev_reset_queue()
would always return Q_NOT_AVAIL. The else clause handles this case
directly without the unnecessary ap_zapq() call, and ensures cleanup
occurs even if kvm has already been set to NULL by a prior call to
vfio_ap_mdev_unset_kvm().
Fixes: b9bd10c43456d ("s390/vfio-ap: do not reset queue removed from host config")
Cc: stable@vger.kernel.org
Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Acked-by: Halil Pasic <pasic@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260818193349.1877940-2-akrowiak@linux.ibm.com>
---
drivers/s390/crypto/vfio_ap_ops.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 940c0ff668be..4db878c18f41 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -277,9 +277,9 @@ static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q)
{
if (!q)
return;
- if (q->saved_isc != VFIO_AP_ISC_INVALID &&
- !WARN_ON(!(q->matrix_mdev && q->matrix_mdev->kvm))) {
- kvm_s390_gisc_unregister(q->matrix_mdev->kvm, q->saved_isc);
+ if (q->saved_isc != VFIO_AP_ISC_INVALID) {
+ if (!WARN_ON(!q->matrix_mdev) && q->matrix_mdev->kvm)
+ kvm_s390_gisc_unregister(q->matrix_mdev->kvm, q->saved_isc);
q->saved_isc = VFIO_AP_ISC_INVALID;
}
if (q->saved_iova && !WARN_ON(!q->matrix_mdev)) {
@@ -1935,6 +1935,8 @@ static int apq_status_check(int apqn, struct ap_queue_status *status)
* a value indicating a reset needs to be performed again.
*/
return -EAGAIN;
+ case AP_RESPONSE_Q_NOT_AVAIL:
+ return -ENODEV;
default:
WARN(true,
"failed to verify reset of queue %02x.%04x: TAPQ rc=%u\n",
@@ -1961,6 +1963,10 @@ static void apq_reset_check(struct work_struct *reset_work)
ret = apq_status_check(q->apqn, &status);
if (ret == -EIO)
return;
+ if (ret == -ENODEV) {
+ vfio_ap_free_aqic_resources(q);
+ return;
+ }
if (ret == -EBUSY) {
pr_notice_ratelimited(WAIT_MSG, elapsed,
AP_QID_CARD(q->apqn),
@@ -2004,6 +2010,7 @@ static void vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q)
break;
case AP_RESPONSE_DECONFIGURED:
case AP_RESPONSE_CHECKSTOPPED:
+ case AP_RESPONSE_Q_NOT_AVAIL:
vfio_ap_free_aqic_resources(q);
break;
default:
@@ -2528,12 +2535,15 @@ void vfio_ap_mdev_remove_queue(struct ap_device *apdev)
/*
* If the queue is not in the host's AP configuration, then resetting
* it will fail with response code 01, (APQN not valid); so, let's make
- * sure it is in the host's config.
+ * sure it is in the host's config. If it is not, free the KVM GISC
+ * resources.
*/
if (test_bit_inv(apid, (unsigned long *)matrix_dev->info.apm) &&
test_bit_inv(apqi, (unsigned long *)matrix_dev->info.aqm)) {
vfio_ap_mdev_reset_queue(q);
flush_work(&q->reset_work);
+ } else {
+ vfio_ap_free_aqic_resources(q);
}
done:
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [GIT PULL v1 09/11] s390/vfio-ap: fix KVM GISC and page leak when queue removed from host config
2026-09-04 6:06 ` [GIT PULL v1 09/11] s390/vfio-ap: fix KVM GISC and page leak when queue removed from host config Claudio Imbrenda
@ 2026-09-04 6:22 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-04 6:22 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: linux-s390, kvm, Alexander Gordeev, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger
> From: Anthony Krowiak <akrowiak@linux.ibm.com>
>
> Three related problems exist in the handling of KVM interrupt and page
> resources when a queue is removed from the host's AP configuration
> while assigned to a mediated device (mdev).
>
> Problem 1:
> ~~~~~~~~~
> AP_RESPONSE_Q_NOT_AVAIL not handled in vfio_ap_mdev_reset_queue()
>
> When the AP bus removes a queue device whose adapter or domain has
> been removed from the host's AP configuration,
> vfio_ap_mdev_remove_queue() is called. If the queue is still in the
> host's AP configuration at that point, it calls
> vfio_ap_mdev_reset_queue(), which issues a PQAP(ZAPQ). Since the
> [ ... ]
> Cc: stable@vger.kernel.org
> Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Acked-by: Halil Pasic <pasic@linux.ibm.com>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260904060647.562443-1-imbrenda@linux.ibm.com?part=9
^ permalink raw reply [flat|nested] 24+ messages in thread
* [GIT PULL v1 10/11] s390/uv: Fix loop condition in uv_find_secrets
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
` (8 preceding siblings ...)
2026-09-04 6:06 ` [GIT PULL v1 09/11] s390/vfio-ap: fix KVM GISC and page leak when queue removed from host config Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:20 ` sashiko-bot
2026-09-04 6:06 ` [GIT PULL v1 11/11] s390/uv: Prevent potential out-of-bounds read Claudio Imbrenda
2026-09-04 15:36 ` [GIT PULL v1 00/11] KVM: s390: Misc fixes Paolo Bonzini
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
From: Steffen Eiden <seiden@linux.ibm.com>
Systems with more than 85 UV secrets got -ENOENT for any secret past the
first page.
Fix this by setting the start index at the beginning of the loop in
uv_find_secret() and not at the end. First test if there are more
secrets left by comparing start_idx with list->next_secret_idx, and then
set the start index to the next secret index.
Fixes: 7c9137af2042 ("s390/uv: Retrieve UV secrets support")
Acked-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260812-uv_secrets_fix-v3-1-a85bd29e0666@linux.ibm.com>
---
arch/s390/kernel/uv.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index dc14ebc0105b..52003c463fad 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -846,11 +846,14 @@ int uv_find_secret(const u8 secret_id[UV_SECRET_ID_LEN],
struct uv_secret_list *list,
struct uv_secret_list_item_hdr *secret)
{
- u16 start_idx = 0;
+ u16 start_idx;
u16 list_rc;
int ret;
+ list->next_secret_idx = 0;
+
do {
+ start_idx = list->next_secret_idx;
uv_list_secrets(list, start_idx, &list_rc, NULL);
if (list_rc != UVC_RC_EXECUTED && list_rc != UVC_RC_MORE_DATA) {
if (list_rc == UVC_RC_INV_CMD)
@@ -861,7 +864,6 @@ int uv_find_secret(const u8 secret_id[UV_SECRET_ID_LEN],
ret = find_secret_in_page(secret_id, list, secret);
if (ret == 0)
return ret;
- start_idx = list->next_secret_idx;
} while (list_rc == UVC_RC_MORE_DATA && start_idx < list->next_secret_idx);
return -ENOENT;
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [GIT PULL v1 11/11] s390/uv: Prevent potential out-of-bounds read
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
` (9 preceding siblings ...)
2026-09-04 6:06 ` [GIT PULL v1 10/11] s390/uv: Fix loop condition in uv_find_secrets Claudio Imbrenda
@ 2026-09-04 6:06 ` Claudio Imbrenda
2026-09-04 6:27 ` sashiko-bot
2026-09-04 15:36 ` [GIT PULL v1 00/11] KVM: s390: Misc fixes Paolo Bonzini
11 siblings, 1 reply; 24+ messages in thread
From: Claudio Imbrenda @ 2026-09-04 6:06 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, linux-s390, frankja, borntraeger
From: Steffen Eiden <seiden@linux.ibm.com>
When the system has more than 85 secrets, the uv_secret_list struct
array only holds up to 85 items per page, resulting in an out of bounds
read in find_secret_in_page if the targeted secret is in the next page
or not stored at all.
Fix this by looping over the number of stored secrets which is the
per sub-list count of stored secrets and not the overall count.
Fixes: 7c9137af2042 ("s390/uv: Retrieve UV secrets support")
Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
Reviewed-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260812-uv_secrets_fix-v3-2-a85bd29e0666@linux.ibm.com>
---
arch/s390/kernel/uv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index 52003c463fad..8ea9dd7704ff 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -825,7 +825,7 @@ static int find_secret_in_page(const u8 secret_id[UV_SECRET_ID_LEN],
{
u16 i;
- for (i = 0; i < list->total_num_secrets; i++) {
+ for (i = 0; i < list->num_secr_stored; i++) {
if (memcmp(secret_id, list->secrets[i].id, UV_SECRET_ID_LEN) == 0) {
*secret = list->secrets[i].hdr;
return 0;
--
2.55.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [GIT PULL v1 11/11] s390/uv: Prevent potential out-of-bounds read
2026-09-04 6:06 ` [GIT PULL v1 11/11] s390/uv: Prevent potential out-of-bounds read Claudio Imbrenda
@ 2026-09-04 6:27 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-09-04 6:27 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: Alexander Gordeev, Vasily Gorbik, Christian Borntraeger,
linux-s390, kvm, Heiko Carstens
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Potential infinite loop in `uvio_get_list` if UVC hardware returns `UVC_RC_MORE_DATA` but fails to advance `next_secret_idx`.
- [High] Missing bounds check on `num_secr_stored` against array size 85, leading to potential out-of-bounds reads and information leaks.
--
commit 3bddf93bb1450f001c28919a0ec81368564fe49c
Author: Steffen Eiden <seiden@linux.ibm.com>
s390/uv: Prevent potential out-of-bounds read
When the system has more than 85 secrets, the uv_secret_list struct array
only holds up to 85 items per page, resulting in an out of bounds read in
find_secret_in_page if the targeted secret is in the next page or not
stored at all. Fix this by looping over the per sub-list count of stored
secrets instead of the overall count.
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index 52003c463fad0..8ea9dd7704ffc 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -825,7 +825,7 @@ static int find_secret_in_page(const u8 secret_id[UV_SECRET_ID_LEN],
> {
> u16 i;
>
> - for (i = 0; i < list->total_num_secrets; i++) {
> + for (i = 0; i < list->num_secr_stored; i++) {
[Severity: High]
This is a pre-existing issue, but does list->num_secr_stored need to be
bounds checked against the size of the secrets[] array (85) before being
used as a loop limit?
If the Ultravisor firmware returns a num_secr_stored value greater than 85,
could this still result in an out-of-bounds read in find_secret_in_page()?
This same missing bounds check appears in uvio_get_list():
drivers/s390/char/uvdevice.c:uvio_get_list() {
...
copy_len = sizeof(list->secrets[0]) * list->num_secr_stored;
if (copy_to_user(user_buf + user_off, list->secrets, copy_len))
return -EFAULT;
...
}
Could an anomalously large num_secr_stored here cause copy_to_user() to copy
kernel memory beyond the allocated page out to user-space?
[Severity: High]
This is also a pre-existing issue, but looking at uvio_get_list() in
drivers/s390/char/uvdevice.c, is there a potential infinite loop?
drivers/s390/char/uvdevice.c:uvio_get_list() {
...
do {
uv_list_secrets(list, list->next_secret_idx, &uv_ioctl->uv_rc,
&uv_ioctl->uv_rrc);
if (uv_ioctl->uv_rc != UVC_RC_EXECUTED &&
uv_ioctl->uv_rc != UVC_RC_MORE_DATA)
break;
copy_len = sizeof(list->secrets[0]) * list->num_secr_stored;
...
user_off += copy_len;
num_secrets_stored += list->num_secr_stored;
} while (uv_ioctl->uv_rc == UVC_RC_MORE_DATA &&
user_off + sizeof(*list) <= uv_ioctl->argument_len);
...
}
If the hardware/firmware returns UVC_RC_MORE_DATA but yields
list->num_secr_stored == 0, user_off does not increment. If
list->next_secret_idx is also not advanced by the hardware, could this loop
indefinitely and cause a CPU stall?
> if (memcmp(secret_id, list->secrets[i].id, UV_SECRET_ID_LEN) == 0) {
> *secret = list->secrets[i].hdr;
> return 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260904060647.562443-1-imbrenda@linux.ibm.com?part=11
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [GIT PULL v1 00/11] KVM: s390: Misc fixes
2026-09-04 6:06 [GIT PULL v1 00/11] KVM: s390: Misc fixes Claudio Imbrenda
` (10 preceding siblings ...)
2026-09-04 6:06 ` [GIT PULL v1 11/11] s390/uv: Prevent potential out-of-bounds read Claudio Imbrenda
@ 2026-09-04 15:36 ` Paolo Bonzini
11 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2026-09-04 15:36 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: kvm, linux-s390, frankja, borntraeger
On Fri, Sep 4, 2026 at 8:07 AM Claudio Imbrenda <imbrenda@linux.ibm.com> wrote:
>
> Ciao Paolo,
>
> Here you have a bunch of fixes to bugs found in the past few weeks.
Pulled, thanks.
Paolo
>
>
> Anthony Krowiak (1):
> s390/vfio-ap: fix KVM GISC and page leak when queue removed from host
> config
>
> Claudio Imbrenda (8):
> KVM: s390: Fix dirty marking in adapter_indicators_set*()
> KVM: s390: Fix compile warning for kvm_s390_update_cmma_dirty()
> KVM: s390: Fix _gaccess_shadow_fault()
> KVM: s390: Refactor dat_set_slot()
> KVM: s390: Move all code into s390_kvm_mmu_prepare_memory_region()
> KVM: s390: Add missing srcu in kvm_s390_set_irq_state()
> KVM: s390: Fix potential races in dat skey functions
> KVM: s390: Fix race in _destroy_pages_crste()
>
> Steffen Eiden (2):
> s390/uv: Fix loop condition in uv_find_secrets
> s390/uv: Prevent potential out-of-bounds read
>
> arch/s390/kernel/uv.c | 8 ++-
> arch/s390/kvm/gmap/dat.c | 65 ++++++++++++----------
> arch/s390/kvm/gmap/dat.h | 10 ++--
> arch/s390/kvm/gmap/gmap.c | 6 +-
> arch/s390/kvm/gmap/kvm_mmu.c | 91 +++++++++++++------------------
> arch/s390/kvm/gmap/kvm_mmu.h | 4 --
> arch/s390/kvm/s390/gaccess.c | 13 +++++
> arch/s390/kvm/s390/interrupt.c | 79 ++++++++++++++-------------
> arch/s390/kvm/s390/s390.c | 3 +-
> arch/s390/kvm/s390/s390.h | 2 +-
> drivers/s390/crypto/vfio_ap_ops.c | 18 ++++--
> 11 files changed, 156 insertions(+), 143 deletions(-)
>
> --
> 2.55.0
>
> The following changes since commit 786262be6048deab760f68c8acc2c85607165894:
>
> Merge tag 'edac_updates_for_v7.3_rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras (2026-08-31 20:10:39 -0700)
>
> are available in the Git repository at:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git tags/kvm-s390-master-7.3-1
>
> for you to fetch changes up to f47190b08b71e8482072978373ee88cb2dfbdaf4:
>
> s390/uv: Prevent potential out-of-bounds read (2026-09-02 18:50:42 +0200)
>
> ----------------------------------------------------------------
> KVM: s390: Misc fixes
>
> A bunch of fixes to bugs found in the past weeks.
>
> ----------------------------------------------------------------
> Anthony Krowiak (1):
> s390/vfio-ap: fix KVM GISC and page leak when queue removed from host config
>
> Claudio Imbrenda (8):
> KVM: s390: Fix dirty marking in adapter_indicators_set*()
> KVM: s390: Fix compile warning for kvm_s390_update_cmma_dirty()
> KVM: s390: Fix _gaccess_shadow_fault()
> KVM: s390: Refactor dat_set_slot()
> KVM: s390: Move all code into s390_kvm_mmu_prepare_memory_region()
> KVM: s390: Add missing srcu in kvm_s390_set_irq_state()
> KVM: s390: Fix potential races in dat skey functions
> KVM: s390: Fix race in _destroy_pages_crste()
>
> Steffen Eiden (2):
> s390/uv: Fix loop condition in uv_find_secrets
> s390/uv: Prevent potential out-of-bounds read
>
> arch/s390/kernel/uv.c | 8 ++--
> arch/s390/kvm/gmap/dat.c | 65 ++++++++++++++++------------
> arch/s390/kvm/gmap/dat.h | 10 ++---
> arch/s390/kvm/gmap/gmap.c | 6 ++-
> arch/s390/kvm/gmap/kvm_mmu.c | 91 ++++++++++++++++-----------------------
> arch/s390/kvm/gmap/kvm_mmu.h | 4 --
> arch/s390/kvm/s390/gaccess.c | 13 ++++++
> arch/s390/kvm/s390/interrupt.c | 79 ++++++++++++++++-----------------
> arch/s390/kvm/s390/s390.c | 3 +-
> arch/s390/kvm/s390/s390.h | 2 +-
> drivers/s390/crypto/vfio_ap_ops.c | 18 ++++++--
> 11 files changed, 156 insertions(+), 143 deletions(-)
>
^ permalink raw reply [flat|nested] 24+ messages in thread