[PATCH v2 6/6] KVM: s390: Fix potential races in dat skey functions
Claudio Imbrenda <[email protected]>
| Newsgroups | org.kernel.vger.linux-s390,org.kernel.vger.kvm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>
---
arch/s390/kvm/dat.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
index 7e5dd5a1eb1e..b4c318ebd91e 100644
--- a/arch/s390/kvm/dat.c
+++ b/arch/s390/kvm/dat.c
@@ -620,17 +620,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))
+ goto again;
+ if (!crste.s.fc1.pr)
return 0;
+
skey->skey = page_get_storage_key(large_crste_to_phys(crste, gfn));
return 0;
}
@@ -661,13 +664,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;
}
@@ -717,15 +727,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);
}
@@ -767,7 +784,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;
@@ -775,9 +792,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))
+ 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